Skip to content

Commit 8df1418

Browse files
committed
Update the FIXME comments in the generic three_way_compare
1 parent 62663e7 commit 8df1418

File tree

1 file changed

+11
-8
lines changed

1 file changed

+11
-8
lines changed

compiler/rustc_codegen_ssa/src/traits/builder.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -412,23 +412,26 @@ pub trait BuilderMethods<'a, 'tcx>:
412412
lhs: Self::Value,
413413
rhs: Self::Value,
414414
) -> Self::Value {
415+
// FIXME: This implementation was designed around LLVM's ability to optimize, but `cg_llvm`
416+
// overrides this to just use `@llvm.scmp`/`ucmp` since LLVM 20. This default impl should be
417+
// reevaluated with respect to the remaining backends like cg_gcc, whether they might use
418+
// specialized implementations as well, or continue to use a generic implementation here.
415419
use std::cmp::Ordering;
416420
let pred = |op| crate::base::bin_op_to_icmp_predicate(op, ty.is_signed());
417421
if self.cx().sess().opts.optimize == OptLevel::No {
418-
// FIXME: This actually generates tighter assembly, and is a classic trick
419-
// <https://graphics.stanford.edu/~seander/bithacks.html#CopyIntegerSign>
420-
// However, as of 2023-11 it optimizes worse in things like derived
421-
// `PartialOrd`, so only use it in debug for now. Once LLVM can handle it
422-
// better (see <https://github.com/llvm/llvm-project/issues/73417>), it'll
423-
// be worth trying it in optimized builds as well.
422+
// This actually generates tighter assembly, and is a classic trick:
423+
// <https://graphics.stanford.edu/~seander/bithacks.html#CopyIntegerSign>.
424+
// However, as of 2023-11 it optimized worse in LLVM in things like derived
425+
// `PartialOrd`, so we were only using it in debug. Since LLVM now uses its own
426+
// intrinsics, it may be be worth trying it in optimized builds for other backends.
424427
let is_gt = self.icmp(pred(mir::BinOp::Gt), lhs, rhs);
425428
let gtext = self.zext(is_gt, self.type_i8());
426429
let is_lt = self.icmp(pred(mir::BinOp::Lt), lhs, rhs);
427430
let ltext = self.zext(is_lt, self.type_i8());
428431
self.unchecked_ssub(gtext, ltext)
429432
} else {
430-
// These operations are those expected by `tests/codegen-llvm/integer-cmp.rs`,
431-
// from <https://github.com/rust-lang/rust/pull/63767>.
433+
// These operations were better optimized by LLVM, before `@llvm.scmp`/`ucmp` in 20.
434+
// See <https://github.com/rust-lang/rust/pull/63767>.
432435
let is_lt = self.icmp(pred(mir::BinOp::Lt), lhs, rhs);
433436
let is_ne = self.icmp(pred(mir::BinOp::Ne), lhs, rhs);
434437
let ge = self.select(

0 commit comments

Comments
 (0)