Skip to content

Commit 8ab474a

Browse files
scottmcmscottmcm
committed
Emit range assumes as a single icmp
Thank you dtcxzyw in LLVM 123278 for pointing out that we were doing this in a suboptimal way.
1 parent c1234e2 commit 8ab474a

File tree

4 files changed

+44
-59
lines changed

4 files changed

+44
-59
lines changed

‎compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+4-33
Original file line numberDiff line numberDiff line change
@@ -386,19 +386,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
386386
// since it's never passed to something with parameter metadata (especially
387387
// after MIR inlining) so the only way to tell the backend about the
388388
// constraint that the `transmute` introduced is to `assume` it.
389-
self.assume_scalar_range(bx, imm, to_scalar, to_backend_ty);
389+
self.assume_scalar_range(bx, imm, to_scalar);
390390

391391
imm = bx.to_immediate_scalar(imm, to_scalar);
392392
imm
393393
}
394394

395-
fn assume_scalar_range(
396-
&self,
397-
bx: &mut Bx,
398-
imm: Bx::Value,
399-
scalar: abi::Scalar,
400-
backend_ty: Bx::Type,
401-
) {
395+
fn assume_scalar_range(&self, bx: &mut Bx, imm: Bx::Value, scalar: abi::Scalar) {
402396
if matches!(self.cx.sess().opts.optimize, OptLevel::No)
403397
// For now, the critical niches are all over `Int`eger values.
404398
// Should floating-point values or pointers ever get more complex
@@ -409,31 +403,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
409403
return;
410404
}
411405

412-
let abi::WrappingRange { start, end } = scalar.valid_range(self.cx);
413-
414-
if start <= end {
415-
if start > 0 {
416-
let low = bx.const_uint_big(backend_ty, start);
417-
let cmp = bx.icmp(IntPredicate::IntUGE, imm, low);
418-
bx.assume(cmp);
419-
}
420-
421-
let type_max = scalar.size(self.cx).unsigned_int_max();
422-
if end < type_max {
423-
let high = bx.const_uint_big(backend_ty, end);
424-
let cmp = bx.icmp(IntPredicate::IntULE, imm, high);
425-
bx.assume(cmp);
426-
}
427-
} else {
428-
let low = bx.const_uint_big(backend_ty, start);
429-
let cmp_low = bx.icmp(IntPredicate::IntUGE, imm, low);
430-
431-
let high = bx.const_uint_big(backend_ty, end);
432-
let cmp_high = bx.icmp(IntPredicate::IntULE, imm, high);
433-
434-
let or = bx.or(cmp_low, cmp_high);
435-
bx.assume(or);
436-
}
406+
let range = scalar.valid_range(self.cx);
407+
bx.assume_integer_range(imm, range);
437408
}
438409

439410
pub(crate) fn codegen_rvalue_unsized(

‎compiler/rustc_codegen_ssa/src/traits/builder.rs

+22
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,28 @@ pub trait BuilderMethods<'a, 'tcx>:
217217
dest: PlaceRef<'tcx, Self::Value>,
218218
);
219219

220+
/// Emits an `assume` that the integer value `imm` is contained in `range`.
221+
///
222+
/// This *always* emits the assumption, so you probably want to check the
223+
/// optimization level and `Scalar::is_always_valid` before calling it.
224+
fn assume_integer_range(&mut self, imm: Self::Value, range: WrappingRange) {
225+
let WrappingRange { start, end } = range;
226+
let backend_ty = self.cx().val_ty(imm);
227+
228+
// Perhaps one day we'll be able to use assume operand bundles for this,
229+
// but for now this encoding with a single icmp+assume is best per
230+
// <https://github.com/llvm/llvm-project/issues/123278#issuecomment-2597440158>
231+
let shifted = if start == 0 {
232+
imm
233+
} else {
234+
let low = self.const_uint_big(backend_ty, start);
235+
self.sub(imm, low)
236+
};
237+
let width = self.const_uint_big(backend_ty, u128::wrapping_sub(end, start));
238+
let cmp = self.icmp(IntPredicate::IntULE, shifted, width);
239+
self.assume(cmp);
240+
}
241+
220242
fn range_metadata(&mut self, load: Self::Value, range: WrappingRange);
221243
fn nonnull_metadata(&mut self, load: Self::Value);
222244

‎tests/codegen/intrinsics/transmute-niched.rs

+15-18
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,8 @@ pub enum SmallEnum {
2020
pub unsafe fn check_to_enum(x: i8) -> SmallEnum {
2121
// CHECK-NOT: icmp
2222
// CHECK-NOT: assume
23-
// OPT: %0 = icmp uge i8 %x, 10
24-
// OPT: call void @llvm.assume(i1 %0)
25-
// OPT: %1 = icmp ule i8 %x, 12
23+
// OPT: %0 = sub i8 %x, 10
24+
// OPT: %1 = icmp ule i8 %0, 2
2625
// OPT: call void @llvm.assume(i1 %1)
2726
// CHECK-NOT: icmp
2827
// CHECK-NOT: assume
@@ -47,10 +46,9 @@ pub unsafe fn check_from_enum(x: SmallEnum) -> i8 {
4746
pub unsafe fn check_to_ordering(x: u8) -> std::cmp::Ordering {
4847
// CHECK-NOT: icmp
4948
// CHECK-NOT: assume
50-
// OPT: %0 = icmp uge i8 %x, -1
51-
// OPT: %1 = icmp ule i8 %x, 1
52-
// OPT: %2 = or i1 %0, %1
53-
// OPT: call void @llvm.assume(i1 %2)
49+
// OPT: %0 = sub i8 %x, -1
50+
// OPT: %1 = icmp ule i8 %0, 2
51+
// OPT: call void @llvm.assume(i1 %1)
5452
// CHECK-NOT: icmp
5553
// CHECK-NOT: assume
5654
// CHECK: ret i8 %x
@@ -100,10 +98,9 @@ pub enum Minus100ToPlus100 {
10098
pub unsafe fn check_enum_from_char(x: char) -> Minus100ToPlus100 {
10199
// CHECK-NOT: icmp
102100
// CHECK-NOT: assume
103-
// OPT: %0 = icmp uge i32 %x, -100
104-
// OPT: %1 = icmp ule i32 %x, 100
105-
// OPT: %2 = or i1 %0, %1
106-
// OPT: call void @llvm.assume(i1 %2)
101+
// OPT: %0 = sub i32 %x, -100
102+
// OPT: %1 = icmp ule i32 %0, 200
103+
// OPT: call void @llvm.assume(i1 %1)
107104
// CHECK-NOT: icmp
108105
// CHECK-NOT: assume
109106
// CHECK: ret i32 %x
@@ -133,10 +130,11 @@ pub unsafe fn check_enum_to_char(x: Minus100ToPlus100) -> char {
133130
pub unsafe fn check_swap_pair(x: (char, NonZero<u32>)) -> (NonZero<u32>, char) {
134131
// CHECK-NOT: icmp
135132
// CHECK-NOT: assume
136-
// OPT: %0 = icmp uge i32 %x.0, 1
137-
// OPT: call void @llvm.assume(i1 %0)
138-
// OPT: %1 = icmp ule i32 %x.1, 1114111
133+
// OPT: %0 = sub i32 %x.0, 1
134+
// OPT: %1 = icmp ule i32 %0, -2
139135
// OPT: call void @llvm.assume(i1 %1)
136+
// OPT: %2 = icmp ule i32 %x.1, 1114111
137+
// OPT: call void @llvm.assume(i1 %2)
140138
// CHECK-NOT: icmp
141139
// CHECK-NOT: assume
142140
// CHECK: %[[P1:.+]] = insertvalue { i32, i32 } poison, i32 %x.0, 0
@@ -169,10 +167,9 @@ pub unsafe fn check_bool_to_ordering(x: bool) -> std::cmp::Ordering {
169167
// CHECK: %_0 = zext i1 %x to i8
170168
// CHECK-NOT: icmp
171169
// CHECK-NOT: assume
172-
// OPT: %0 = icmp uge i8 %_0, -1
173-
// OPT: %1 = icmp ule i8 %_0, 1
174-
// OPT: %2 = or i1 %0, %1
175-
// OPT: call void @llvm.assume(i1 %2)
170+
// OPT: %0 = sub i8 %_0, -1
171+
// OPT: %1 = icmp ule i8 %0, 2
172+
// OPT: call void @llvm.assume(i1 %1)
176173
// CHECK-NOT: icmp
177174
// CHECK-NOT: assume
178175
// CHECK: ret i8 %_0

‎tests/codegen/transmute-optimized.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,10 @@ pub enum OneTwoThree {
9494
// CHECK-SAME: range(i8 -1, 2){{.+}}%x
9595
#[no_mangle]
9696
pub unsafe fn ordering_transmute_onetwothree(x: std::cmp::Ordering) -> OneTwoThree {
97-
// FIXME: this *should* just be `ret i8 1`, but that's not happening today.
98-
// cc <https://github.com/llvm/llvm-project/issues/123278>
99-
100-
// CHECK: %[[TEMP1:.+]] = icmp ne i8 %x, 0
101-
// CHECK: tail call void @llvm.assume(i1 %[[TEMP1]])
102-
// CHECK: %[[TEMP2:.+]] = icmp ult i8 %x, 4
97+
// CHECK: %[[TEMP1:.+]] = add nsw i8 %x, -1
98+
// CHECK: %[[TEMP2:.+]] = icmp ult i8 %[[TEMP1]], 3
10399
// CHECK: tail call void @llvm.assume(i1 %[[TEMP2]])
104-
105-
// CHECK: ret i8 %x
100+
// CHECK: ret i8 1
106101
std::mem::transmute(x)
107102
}
108103

0 commit comments

Comments
 (0)