Auto merge of #135674 - scottmcm:assume-better, r=estebank

Update our range `assume`s to the format that LLVM prefers
I found out in llvm/llvm-project#123278 (comment) that the way I started emitting the `assume`s in #109993 was suboptimal, and as seen in that LLVM issue the way we're doing it -- with two `assume`s sometimes -- can at times lead to CVP/SCCP not realize what's happening because one of them turns into a `ne` instead of conveying a range.
So this updates how it's emitted from
```
assume( x >= LOW );
assume( x <= HIGH );
```
or
```
// (for ranges that wrap the range)
assume( (x <= LOW) | (x >= HIGH) );
```
to
```
assume( (x - LOW) <= (HIGH - LOW) );
```
so that we don't need multiple `icmp`s nor multiple `assume`s for a single value, and both wrappping and non-wrapping ranges emit the same shape.
(And we don't bother emitting the subtraction if `LOW` is zero, since that's trivial for us to check too.)

Author: bors

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -387,10 +387,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 use abi::Primitive::*;
  imm = bx.from_immediate(imm);
 
-// When scalars are passed by value, there's no metadata recording their
-// valid ranges. For example, `char`s are passed as just `i32`, with no
-// way for LLVM to know that they're 0x10FFFF at most. Thus we assume
-// the range of the input value too, not just the output range.
+// If we have a scalar, we must already know its range. Either
+//
+// 1) It's a parameter with `range` parameter metadata,
+// 2) It's something we `load`ed with `!range` metadata, or
+// 3) After a transmute we `assume`d the range (see below).
+//
+// That said, last time we tried removing this, it didn't actually help
+// the rustc-perf results, so might as well keep doing it
+// <https://github.com/rust-lang/rust/pull/135610#issuecomment-2599275182>
 self.assume_scalar_range(bx, imm, from_scalar, from_backend_ty);
 
  imm = match(from_scalar.primitive(), to_scalar.primitive()){
@@ -411,7 +416,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
  bx.bitcast(int_imm, to_backend_ty)
 }
 };
+
+// This `assume` remains important for cases like (a conceptual)
+// transmute::<u32, NonZeroU32>(x) == 0
+// since it's never passed to something with parameter metadata (especially
+// after MIR inlining) so the only way to tell the backend about the
+// constraint that the `transmute` introduced is to `assume` it.
 self.assume_scalar_range(bx, imm, to_scalar, to_backend_ty);
+
  imm = bx.to_immediate_scalar(imm, to_scalar);
  imm
 }
@@ -433,31 +445,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 return;
 }
 
-let abi::WrappingRange{ start, end } = scalar.valid_range(self.cx);
-
-if start <= end {
-if start > 0{
-let low = bx.const_uint_big(backend_ty, start);
-let cmp = bx.icmp(IntPredicate::IntUGE, imm, low);
- bx.assume(cmp);
-}
-
-let type_max = scalar.size(self.cx).unsigned_int_max();
-if end < type_max {
-let high = bx.const_uint_big(backend_ty, end);
-let cmp = bx.icmp(IntPredicate::IntULE, imm, high);
- bx.assume(cmp);
-}
-}else{
-let low = bx.const_uint_big(backend_ty, start);
-let cmp_low = bx.icmp(IntPredicate::IntUGE, imm, low);
-
-let high = bx.const_uint_big(backend_ty, end);
-let cmp_high = bx.icmp(IntPredicate::IntULE, imm, high);
-
-let or = bx.or(cmp_low, cmp_high);
- bx.assume(or);
-}
+let range = scalar.valid_range(self.cx);
+ bx.assume_integer_range(imm, backend_ty, range);
 }
 
 pub(crate)fncodegen_rvalue_unsized(

compiler/rustc_codegen_ssa/src/traits/builder.rs

@@ -217,6 +217,27 @@ pub trait BuilderMethods<'a, 'tcx>:
 dest:PlaceRef<'tcx,Self::Value>,
 );
 
+/// Emits an `assume` that the integer value `imm` of type `ty` is contained in `range`.
+///
+/// This *always* emits the assumption, so you probably want to check the
+/// optimization level and `Scalar::is_always_valid` before calling it.
+fnassume_integer_range(&mutself,imm:Self::Value,ty:Self::Type,range:WrappingRange){
+letWrappingRange{ start, end } = range;
+
+// Perhaps one day we'll be able to use assume operand bundles for this,
+// but for now this encoding with a single icmp+assume is best per
+// <https://github.com/llvm/llvm-project/issues/123278#issuecomment-2597440158>
+let shifted = if start == 0{
+ imm
+}else{
+let low = self.const_uint_big(ty, start);
+self.sub(imm, low)
+};
+let width = self.const_uint_big(ty, u128::wrapping_sub(end, start));
+let cmp = self.icmp(IntPredicate::IntULE, shifted, width);
+self.assume(cmp);
+}
+
 fnrange_metadata(&mutself,load:Self::Value,range:WrappingRange);
 fnnonnull_metadata(&mutself,load:Self::Value);
 

tests/codegen/intrinsics/transmute-niched.rs

@@ -17,12 +17,13 @@ pub enum SmallEnum {
 // CHECK-LABEL: @check_to_enum(
 #[no_mangle]
 pubunsafefncheck_to_enum(x:i8) -> SmallEnum{
-// OPT: %0 = icmp uge i8 %x, 10
-// OPT: call void @llvm.assume(i1 %0)
-// OPT: %1 = icmp ule i8 %x, 12
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
+// OPT: %0 = sub i8 %x, 10
+// OPT: %1 = icmp ule i8 %0, 2
 // OPT: call void @llvm.assume(i1 %1)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: ret i8 %x
 
 transmute(x)
@@ -31,12 +32,13 @@ pub unsafe fn check_to_enum(x: i8) -> SmallEnum {
 // CHECK-LABEL: @check_from_enum(
 #[no_mangle]
 pubunsafefncheck_from_enum(x:SmallEnum) -> i8{
-// OPT: %0 = icmp uge i8 %x, 10
-// OPT: call void @llvm.assume(i1 %0)
-// OPT: %1 = icmp ule i8 %x, 12
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
+// OPT: %0 = sub i8 %x, 10
+// OPT: %1 = icmp ule i8 %0, 2
 // OPT: call void @llvm.assume(i1 %1)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: ret i8 %x
 
 transmute(x)
@@ -45,12 +47,13 @@ pub unsafe fn check_from_enum(x: SmallEnum) -> i8 {
 // CHECK-LABEL: @check_to_ordering(
 #[no_mangle]
 pubunsafefncheck_to_ordering(x:u8) -> std::cmp::Ordering{
-// OPT: %0 = icmp uge i8 %x, -1
-// OPT: %1 = icmp ule i8 %x, 1
-// OPT: %2 = or i1 %0, %1
-// OPT: call void @llvm.assume(i1 %2)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
+// OPT: %0 = sub i8 %x, -1
+// OPT: %1 = icmp ule i8 %0, 2
+// OPT: call void @llvm.assume(i1 %1)
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: ret i8 %x
 
 transmute(x)
@@ -59,12 +62,13 @@ pub unsafe fn check_to_ordering(x: u8) -> std::cmp::Ordering {
 // CHECK-LABEL: @check_from_ordering(
 #[no_mangle]
 pubunsafefncheck_from_ordering(x: std::cmp::Ordering) -> u8{
-// OPT: %0 = icmp uge i8 %x, -1
-// OPT: %1 = icmp ule i8 %x, 1
-// OPT: %2 = or i1 %0, %1
-// OPT: call void @llvm.assume(i1 %2)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
+// OPT: %0 = sub i8 %x, -1
+// OPT: %1 = icmp ule i8 %0, 2
+// OPT: call void @llvm.assume(i1 %1)
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: ret i8 %x
 
 transmute(x)
@@ -98,14 +102,15 @@ pub enum Minus100ToPlus100 {
 // CHECK-LABEL: @check_enum_from_char(
 #[no_mangle]
 pubunsafefncheck_enum_from_char(x:char) -> Minus100ToPlus100{
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // OPT: %0 = icmp ule i32 %x, 1114111
 // OPT: call void @llvm.assume(i1 %0)
-// OPT: %1 = icmp uge i32 %x, -100
-// OPT: %2 = icmp ule i32 %x, 100
-// OPT: %3 = or i1 %1, %2
-// OPT: call void @llvm.assume(i1 %3)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// OPT: %1 = sub i32 %x, -100
+// OPT: %2 = icmp ule i32 %1, 200
+// OPT: call void @llvm.assume(i1 %2)
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: ret i32 %x
 
 transmute(x)
@@ -114,14 +119,15 @@ pub unsafe fn check_enum_from_char(x: char) -> Minus100ToPlus100 {
 // CHECK-LABEL: @check_enum_to_char(
 #[no_mangle]
 pubunsafefncheck_enum_to_char(x:Minus100ToPlus100) -> char{
-// OPT: %0 = icmp uge i32 %x, -100
-// OPT: %1 = icmp ule i32 %x, 100
-// OPT: %2 = or i1 %0, %1
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
+// OPT: %0 = sub i32 %x, -100
+// OPT: %1 = icmp ule i32 %0, 200
+// OPT: call void @llvm.assume(i1 %1)
+// OPT: %2 = icmp ule i32 %x, 1114111
 // OPT: call void @llvm.assume(i1 %2)
-// OPT: %3 = icmp ule i32 %x, 1114111
-// OPT: call void @llvm.assume(i1 %3)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: ret i32 %x
 
 transmute(x)
@@ -130,16 +136,20 @@ pub unsafe fn check_enum_to_char(x: Minus100ToPlus100) -> char {
 // CHECK-LABEL: @check_swap_pair(
 #[no_mangle]
 pubunsafefncheck_swap_pair(x:(char,NonZero<u32>)) -> (NonZero<u32>,char){
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // OPT: %0 = icmp ule i32 %x.0, 1114111
 // OPT: call void @llvm.assume(i1 %0)
-// OPT: %1 = icmp uge i32 %x.0, 1
-// OPT: call void @llvm.assume(i1 %1)
-// OPT: %2 = icmp uge i32 %x.1, 1
+// OPT: %1 = sub i32 %x.0, 1
+// OPT: %2 = icmp ule i32 %1, -2
 // OPT: call void @llvm.assume(i1 %2)
-// OPT: %3 = icmp ule i32 %x.1, 1114111
-// OPT: call void @llvm.assume(i1 %3)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// OPT: %3 = sub i32 %x.1, 1
+// OPT: %4 = icmp ule i32 %3, -2
+// OPT: call void @llvm.assume(i1 %4)
+// OPT: %5 = icmp ule i32 %x.1, 1114111
+// OPT: call void @llvm.assume(i1 %5)
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: %[[P1:.+]] = insertvalue { i32, i32 } poison, i32 %x.0, 0
 // CHECK: %[[P2:.+]] = insertvalue { i32, i32 } %[[P1]], i32 %x.1, 1
 // CHECK: ret { i32, i32 } %[[P2]]
@@ -150,14 +160,15 @@ pub unsafe fn check_swap_pair(x: (char, NonZero<u32>)) -> (NonZero<u32>, char) {
 // CHECK-LABEL: @check_bool_from_ordering(
 #[no_mangle]
 pubunsafefncheck_bool_from_ordering(x: std::cmp::Ordering) -> bool{
-// OPT: %0 = icmp uge i8 %x, -1
-// OPT: %1 = icmp ule i8 %x, 1
-// OPT: %2 = or i1 %0, %1
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
+// OPT: %0 = sub i8 %x, -1
+// OPT: %1 = icmp ule i8 %0, 2
+// OPT: call void @llvm.assume(i1 %1)
+// OPT: %2 = icmp ule i8 %x, 1
 // OPT: call void @llvm.assume(i1 %2)
-// OPT: %3 = icmp ule i8 %x, 1
-// OPT: call void @llvm.assume(i1 %3)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: %[[R:.+]] = trunc i8 %x to i1
 // CHECK: ret i1 %[[R]]
 
@@ -168,14 +179,15 @@ pub unsafe fn check_bool_from_ordering(x: std::cmp::Ordering) -> bool {
 #[no_mangle]
 pubunsafefncheck_bool_to_ordering(x:bool) -> std::cmp::Ordering{
 // CHECK: %_0 = zext i1 %x to i8
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // OPT: %0 = icmp ule i8 %_0, 1
 // OPT: call void @llvm.assume(i1 %0)
-// OPT: %1 = icmp uge i8 %_0, -1
-// OPT: %2 = icmp ule i8 %_0, 1
-// OPT: %3 = or i1 %1, %2
-// OPT: call void @llvm.assume(i1 %3)
-// DBG-NOT: icmp
-// DBG-NOT: assume
+// OPT: %1 = sub i8 %_0, -1
+// OPT: %2 = icmp ule i8 %1, 2
+// OPT: call void @llvm.assume(i1 %2)
+// CHECK-NOT: icmp
+// CHECK-NOT: assume
 // CHECK: ret i8 %_0
 
 transmute(x)

tests/codegen/transmute-optimized.rs

@@ -110,3 +110,11 @@ pub fn char_is_negative(c: char) -> bool {
 let x:i32 = unsafe{ std::mem::transmute(c)};
  x < 0
 }
+
+// CHECK-LABEL: i1 @transmute_to_char_is_negative(i32
+#[no_mangle]
+pubfntransmute_to_char_is_negative(x:i32) -> bool{
+// CHECK: ret i1 false
+let _c:char = unsafe{ std::mem::transmute(x)};
+ x < 0
+}