Skip to content

Commit 53e3939

Browse files
committed
Emit fewer assumes now that we have range metadata on parameters
We still need the `assume` for the *target* type's range, but we no longer need it for the *source* type's range. Admittedly there's one test not properly handled by LLVM today, but it's synthetic, so I'd still be fine doing this and just updating the test once LLVM fixes the bug. All the other optimization tests still pass. Hopefully this means less crud for LLVM to churn through in `opt` builds...
1 parent 76a030a commit 53e3939

File tree

6 files changed

+92
-105
lines changed

6 files changed

+92
-105
lines changed

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

+16-18
Original file line numberDiff line numberDiff line change
@@ -257,13 +257,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
257257
if let OperandValueKind::Immediate(to_scalar) = cast_kind
258258
&& from_scalar.size(self.cx) == to_scalar.size(self.cx)
259259
{
260-
let from_backend_ty = bx.backend_type(operand.layout);
261260
let to_backend_ty = bx.backend_type(cast);
262261
Some(OperandValue::Immediate(self.transmute_immediate(
263262
bx,
264263
imm,
265264
from_scalar,
266-
from_backend_ty,
267265
to_scalar,
268266
to_backend_ty,
269267
)))
@@ -279,13 +277,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
279277
&& in_a.size(self.cx) == out_a.size(self.cx)
280278
&& in_b.size(self.cx) == out_b.size(self.cx)
281279
{
282-
let in_a_ibty = bx.scalar_pair_element_backend_type(operand.layout, 0, false);
283-
let in_b_ibty = bx.scalar_pair_element_backend_type(operand.layout, 1, false);
284280
let out_a_ibty = bx.scalar_pair_element_backend_type(cast, 0, false);
285281
let out_b_ibty = bx.scalar_pair_element_backend_type(cast, 1, false);
286282
Some(OperandValue::Pair(
287-
self.transmute_immediate(bx, imm_a, in_a, in_a_ibty, out_a, out_a_ibty),
288-
self.transmute_immediate(bx, imm_b, in_b, in_b_ibty, out_b, out_b_ibty),
283+
self.transmute_immediate(bx, imm_a, in_a, out_a, out_a_ibty),
284+
self.transmute_immediate(bx, imm_b, in_b, out_b, out_b_ibty),
289285
))
290286
} else {
291287
None
@@ -309,12 +305,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
309305
) -> Option<Bx::Value> {
310306
use abi::Primitive::*;
311307

312-
// When scalars are passed by value, there's no metadata recording their
313-
// valid ranges. For example, `char`s are passed as just `i32`, with no
314-
// way for LLVM to know that they're 0x10FFFF at most. Thus we assume
315-
// the range of the input value too, not just the output range.
316-
self.assume_scalar_range(bx, imm, from_scalar, from_backend_ty);
317-
318308
imm = match (from_scalar.primitive(), to_scalar.primitive()) {
319309
(Int(_, is_signed), Int(..)) => bx.intcast(imm, to_backend_ty, is_signed),
320310
(Float(_), Float(_)) => {
@@ -356,7 +346,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
356346
bx: &mut Bx,
357347
mut imm: Bx::Value,
358348
from_scalar: abi::Scalar,
359-
from_backend_ty: Bx::Type,
360349
to_scalar: abi::Scalar,
361350
to_backend_ty: Bx::Type,
362351
) -> Bx::Value {
@@ -365,11 +354,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
365354
use abi::Primitive::*;
366355
imm = bx.from_immediate(imm);
367356

368-
// When scalars are passed by value, there's no metadata recording their
369-
// valid ranges. For example, `char`s are passed as just `i32`, with no
370-
// way for LLVM to know that they're 0x10FFFF at most. Thus we assume
371-
// the range of the input value too, not just the output range.
372-
self.assume_scalar_range(bx, imm, from_scalar, from_backend_ty);
357+
// We used to `assume` the `from_scalar` here too, but that's no longer needed
358+
// because if we have a scalar, we must already know its range. Either
359+
// 1) It's a parameter with `range` parameter metadata,
360+
// 2) It's something we `load`ed with `!range` metadata, or
361+
// 3) It's something we transmuted and already `assume`d the range.
362+
// And thus in all those cases another `assume` is just wasteful.
363+
// (Case 1 didn't used to be covered, and thus the `assume` was needed.)
373364

374365
imm = match (from_scalar.primitive(), to_scalar.primitive()) {
375366
(Int(..) | Float(_), Int(..) | Float(_)) => bx.bitcast(imm, to_backend_ty),
@@ -389,7 +380,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
389380
bx.bitcast(int_imm, to_backend_ty)
390381
}
391382
};
383+
384+
// This `assume` remains important for cases like
385+
// transmute::<u32, NonZeroU32>(x) == 0
386+
// since it's never passed to something with parameter metadata (especially
387+
// after MIR inlining) so the only way to tell the backend about the
388+
// constraint that the `transmute` introduced is to `assume` it.
392389
self.assume_scalar_range(bx, imm, to_scalar, to_backend_ty);
390+
393391
imm = bx.to_immediate_scalar(imm, to_scalar);
394392
imm
395393
}

tests/codegen/cast-optimized.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//@ compile-flags: -O -Z merge-functions=disabled
2+
//@ min-llvm-version: 19 (these optimizations depend on range parameter metadata)
23
#![crate_type = "lib"]
34

45
// This tests that LLVM can optimize based on the niches in the source or

tests/codegen/intrinsics/transmute-niched.rs

+55-56
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//@ revisions: OPT DBG
22
//@ [OPT] compile-flags: -C opt-level=3 -C no-prepopulate-passes
3+
//@ [OPT] min-llvm-version: 19 (for range parameter metadata)
34
//@ [DBG] compile-flags: -C opt-level=0 -C no-prepopulate-passes
45
//@ only-64bit (so I don't need to worry about usize)
56
#![crate_type = "lib"]
@@ -17,26 +18,25 @@ pub enum SmallEnum {
1718
// CHECK-LABEL: @check_to_enum(
1819
#[no_mangle]
1920
pub unsafe fn check_to_enum(x: i8) -> SmallEnum {
21+
// CHECK-NOT: icmp
22+
// CHECK-NOT: assume
2023
// OPT: %0 = icmp uge i8 %x, 10
2124
// OPT: call void @llvm.assume(i1 %0)
2225
// OPT: %1 = icmp ule i8 %x, 12
2326
// OPT: call void @llvm.assume(i1 %1)
24-
// DBG-NOT: icmp
25-
// DBG-NOT: assume
27+
// CHECK-NOT: icmp
28+
// CHECK-NOT: assume
2629
// CHECK: ret i8 %x
2730

2831
transmute(x)
2932
}
3033

3134
// CHECK-LABEL: @check_from_enum(
35+
// OPT-SAME: range(i8 10, 13){{.+}}%x
3236
#[no_mangle]
3337
pub unsafe fn check_from_enum(x: SmallEnum) -> i8 {
34-
// OPT: %0 = icmp uge i8 %x, 10
35-
// OPT: call void @llvm.assume(i1 %0)
36-
// OPT: %1 = icmp ule i8 %x, 12
37-
// OPT: call void @llvm.assume(i1 %1)
38-
// DBG-NOT: icmp
39-
// DBG-NOT: assume
38+
// CHECK-NOT: icmp
39+
// CHECK-NOT: assume
4040
// CHECK: ret i8 %x
4141

4242
transmute(x)
@@ -45,26 +45,25 @@ pub unsafe fn check_from_enum(x: SmallEnum) -> i8 {
4545
// CHECK-LABEL: @check_to_ordering(
4646
#[no_mangle]
4747
pub unsafe fn check_to_ordering(x: u8) -> std::cmp::Ordering {
48+
// CHECK-NOT: icmp
49+
// CHECK-NOT: assume
4850
// OPT: %0 = icmp uge i8 %x, -1
4951
// OPT: %1 = icmp ule i8 %x, 1
5052
// OPT: %2 = or i1 %0, %1
5153
// OPT: call void @llvm.assume(i1 %2)
52-
// DBG-NOT: icmp
53-
// DBG-NOT: assume
54+
// CHECK-NOT: icmp
55+
// CHECK-NOT: assume
5456
// CHECK: ret i8 %x
5557

5658
transmute(x)
5759
}
5860

5961
// CHECK-LABEL: @check_from_ordering(
62+
// OPT-SAME: range(i8 -1, 2){{.+}}%x
6063
#[no_mangle]
6164
pub unsafe fn check_from_ordering(x: std::cmp::Ordering) -> u8 {
62-
// OPT: %0 = icmp uge i8 %x, -1
63-
// OPT: %1 = icmp ule i8 %x, 1
64-
// OPT: %2 = or i1 %0, %1
65-
// OPT: call void @llvm.assume(i1 %2)
66-
// DBG-NOT: icmp
67-
// DBG-NOT: assume
65+
// CHECK-NOT: icmp
66+
// CHECK-NOT: assume
6867
// CHECK: ret i8 %x
6968

7069
transmute(x)
@@ -96,50 +95,50 @@ pub enum Minus100ToPlus100 {
9695
}
9796

9897
// CHECK-LABEL: @check_enum_from_char(
98+
// OPT-SAME: range(i32 0, 1114112){{.+}}%x
9999
#[no_mangle]
100100
pub unsafe fn check_enum_from_char(x: char) -> Minus100ToPlus100 {
101-
// OPT: %0 = icmp ule i32 %x, 1114111
102-
// OPT: call void @llvm.assume(i1 %0)
103-
// OPT: %1 = icmp uge i32 %x, -100
104-
// OPT: %2 = icmp ule i32 %x, 100
105-
// OPT: %3 = or i1 %1, %2
106-
// OPT: call void @llvm.assume(i1 %3)
107-
// DBG-NOT: icmp
108-
// DBG-NOT: assume
101+
// CHECK-NOT: icmp
102+
// 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)
107+
// CHECK-NOT: icmp
108+
// CHECK-NOT: assume
109109
// CHECK: ret i32 %x
110110

111111
transmute(x)
112112
}
113113

114114
// CHECK-LABEL: @check_enum_to_char(
115+
// OPT-SAME: range(i32 -100, 101){{.+}}%x
115116
#[no_mangle]
116117
pub unsafe fn check_enum_to_char(x: Minus100ToPlus100) -> char {
117-
// OPT: %0 = icmp uge i32 %x, -100
118-
// OPT: %1 = icmp ule i32 %x, 100
119-
// OPT: %2 = or i1 %0, %1
120-
// OPT: call void @llvm.assume(i1 %2)
121-
// OPT: %3 = icmp ule i32 %x, 1114111
122-
// OPT: call void @llvm.assume(i1 %3)
123-
// DBG-NOT: icmp
124-
// DBG-NOT: assume
118+
// CHECK-NOT: icmp
119+
// CHECK-NOT: assume
120+
// OPT: %0 = icmp ule i32 %x, 1114111
121+
// OPT: call void @llvm.assume(i1 %0)
122+
// CHECK-NOT: icmp
123+
// CHECK-NOT: assume
125124
// CHECK: ret i32 %x
126125

127126
transmute(x)
128127
}
129128

130129
// CHECK-LABEL: @check_swap_pair(
130+
// OPT-SAME: range(i32 0, 1114112){{.+}}%x.0
131+
// OPT-SAME: range(i32 1, 0){{.+}}%x.1
131132
#[no_mangle]
132133
pub unsafe fn check_swap_pair(x: (char, NonZero<u32>)) -> (NonZero<u32>, char) {
133-
// OPT: %0 = icmp ule i32 %x.0, 1114111
134+
// CHECK-NOT: icmp
135+
// CHECK-NOT: assume
136+
// OPT: %0 = icmp uge i32 %x.0, 1
134137
// OPT: call void @llvm.assume(i1 %0)
135-
// OPT: %1 = icmp uge i32 %x.0, 1
138+
// OPT: %1 = icmp ule i32 %x.1, 1114111
136139
// OPT: call void @llvm.assume(i1 %1)
137-
// OPT: %2 = icmp uge i32 %x.1, 1
138-
// OPT: call void @llvm.assume(i1 %2)
139-
// OPT: %3 = icmp ule i32 %x.1, 1114111
140-
// OPT: call void @llvm.assume(i1 %3)
141-
// DBG-NOT: icmp
142-
// DBG-NOT: assume
140+
// CHECK-NOT: icmp
141+
// CHECK-NOT: assume
143142
// CHECK: %[[P1:.+]] = insertvalue { i32, i32 } poison, i32 %x.0, 0
144143
// CHECK: %[[P2:.+]] = insertvalue { i32, i32 } %[[P1]], i32 %x.1, 1
145144
// CHECK: ret { i32, i32 } %[[P2]]
@@ -148,34 +147,34 @@ pub unsafe fn check_swap_pair(x: (char, NonZero<u32>)) -> (NonZero<u32>, char) {
148147
}
149148

150149
// CHECK-LABEL: @check_bool_from_ordering(
150+
// OPT-SAME: range(i8 -1, 2){{.+}}%x
151151
#[no_mangle]
152152
pub unsafe fn check_bool_from_ordering(x: std::cmp::Ordering) -> bool {
153-
// OPT: %0 = icmp uge i8 %x, -1
154-
// OPT: %1 = icmp ule i8 %x, 1
155-
// OPT: %2 = or i1 %0, %1
156-
// OPT: call void @llvm.assume(i1 %2)
157-
// OPT: %3 = icmp ule i8 %x, 1
158-
// OPT: call void @llvm.assume(i1 %3)
159-
// DBG-NOT: icmp
160-
// DBG-NOT: assume
153+
// CHECK-NOT: icmp
154+
// CHECK-NOT: assume
155+
// OPT: %0 = icmp ule i8 %x, 1
156+
// OPT: call void @llvm.assume(i1 %0)
157+
// CHECK-NOT: icmp
158+
// CHECK-NOT: assume
161159
// CHECK: %[[R:.+]] = trunc i8 %x to i1
162160
// CHECK: ret i1 %[[R]]
163161

164162
transmute(x)
165163
}
166164

167165
// CHECK-LABEL: @check_bool_to_ordering(
166+
// OPT-SAME: i1 {{.+}} %x
168167
#[no_mangle]
169168
pub unsafe fn check_bool_to_ordering(x: bool) -> std::cmp::Ordering {
170169
// CHECK: %_0 = zext i1 %x to i8
171-
// OPT: %0 = icmp ule i8 %_0, 1
172-
// OPT: call void @llvm.assume(i1 %0)
173-
// OPT: %1 = icmp uge i8 %_0, -1
174-
// OPT: %2 = icmp ule i8 %_0, 1
175-
// OPT: %3 = or i1 %1, %2
176-
// OPT: call void @llvm.assume(i1 %3)
177-
// DBG-NOT: icmp
178-
// DBG-NOT: assume
170+
// CHECK-NOT: icmp
171+
// 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)
176+
// CHECK-NOT: icmp
177+
// CHECK-NOT: assume
179178
// CHECK: ret i8 %_0
180179

181180
transmute(x)

tests/codegen/issues/issue-119422.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//! for `NonZero` integer types.
33
//!
44
//@ compile-flags: -O --edition=2021 -Zmerge-functions=disabled
5+
//@ min-llvm-version: 19 (for range parameter metadata)
56
//@ only-64bit (because the LLVM type of i64 for usize shows up)
67
#![crate_type = "lib"]
78

tests/codegen/transmute-optimized.rs

+19-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//@ compile-flags: -O -Z merge-functions=disabled
2+
//@ min-llvm-version: 19 (for range parameter metadata)
23
#![crate_type = "lib"]
34

45
// This tests that LLVM can optimize based on the niches in the source or
@@ -90,9 +91,18 @@ pub enum OneTwoThree {
9091
}
9192

9293
// CHECK-LABEL: i8 @ordering_transmute_onetwothree(i8
94+
// CHECK-SAME: range(i8 -1, 2){{.+}}%x
9395
#[no_mangle]
9496
pub unsafe fn ordering_transmute_onetwothree(x: std::cmp::Ordering) -> OneTwoThree {
95-
// CHECK: ret i8 1
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
103+
// CHECK: tail call void @llvm.assume(i1 %[[TEMP2]])
104+
105+
// CHECK: ret i8 %x
96106
std::mem::transmute(x)
97107
}
98108

@@ -110,3 +120,11 @@ pub fn char_is_negative(c: char) -> bool {
110120
let x: i32 = unsafe { std::mem::transmute(c) };
111121
x < 0
112122
}
123+
124+
// CHECK-LABEL: i1 @transmute_to_char_is_negative(i32
125+
#[no_mangle]
126+
pub fn transmute_to_char_is_negative(x: i32) -> bool {
127+
// CHECK: ret i1 false
128+
let _c: char = unsafe { std::mem::transmute(x) };
129+
x < 0
130+
}

0 commit comments

Comments
 (0)