Skip to content

Commit fe5bdac

Browse files
scottmcmscottmcm
committed
Set both nuw and nsw in slice size calculation
There's an old note in the code to do this, and now that LLVM-C has an API for it, we might as well.
1 parent 07179d5 commit fe5bdac

File tree

7 files changed

+81
-44
lines changed

7 files changed

+81
-44
lines changed

‎compiler/rustc_codegen_gcc/src/builder.rs

+1-25
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
668668
a + b
669669
}
670670

671+
// TODO(antoyo): should we also override the `unchecked_` versions?
671672
fn sub(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
672673
self.gcc_sub(a, b)
673674
}
@@ -835,31 +836,6 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
835836
set_rvalue_location(self, self.gcc_not(a))
836837
}
837838

838-
fn unchecked_sadd(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
839-
set_rvalue_location(self, self.gcc_add(a, b))
840-
}
841-
842-
fn unchecked_uadd(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
843-
set_rvalue_location(self, self.gcc_add(a, b))
844-
}
845-
846-
fn unchecked_ssub(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
847-
set_rvalue_location(self, self.gcc_sub(a, b))
848-
}
849-
850-
fn unchecked_usub(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
851-
// TODO(antoyo): should generate poison value?
852-
set_rvalue_location(self, self.gcc_sub(a, b))
853-
}
854-
855-
fn unchecked_smul(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
856-
set_rvalue_location(self, self.gcc_mul(a, b))
857-
}
858-
859-
fn unchecked_umul(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
860-
set_rvalue_location(self, self.gcc_mul(a, b))
861-
}
862-
863839
fn fadd_fast(&mut self, lhs: RValue<'gcc>, rhs: RValue<'gcc>) -> RValue<'gcc> {
864840
// NOTE: it seems like we cannot enable fast-mode for a single operation in GCC.
865841
set_rvalue_location(self, lhs + rhs)

‎compiler/rustc_codegen_llvm/src/builder.rs

+31
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,37 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
421421
unchecked_umul(x, y) => LLVMBuildNUWMul,
422422
}
423423

424+
fn unchecked_suadd(&mut self, a: &'ll Value, b: &'ll Value) -> &'ll Value {
425+
unsafe {
426+
let add = llvm::LLVMBuildAdd(self.llbuilder, a, b, UNNAMED);
427+
if llvm::LLVMIsAInstruction(add).is_some() {
428+
llvm::LLVMSetNUW(add, True);
429+
llvm::LLVMSetNSW(add, True);
430+
}
431+
add
432+
}
433+
}
434+
fn unchecked_susub(&mut self, a: &'ll Value, b: &'ll Value) -> &'ll Value {
435+
unsafe {
436+
let sub = llvm::LLVMBuildSub(self.llbuilder, a, b, UNNAMED);
437+
if llvm::LLVMIsAInstruction(sub).is_some() {
438+
llvm::LLVMSetNUW(sub, True);
439+
llvm::LLVMSetNSW(sub, True);
440+
}
441+
sub
442+
}
443+
}
444+
fn unchecked_sumul(&mut self, a: &'ll Value, b: &'ll Value) -> &'ll Value {
445+
unsafe {
446+
let mul = llvm::LLVMBuildMul(self.llbuilder, a, b, UNNAMED);
447+
if llvm::LLVMIsAInstruction(mul).is_some() {
448+
llvm::LLVMSetNUW(mul, True);
449+
llvm::LLVMSetNSW(mul, True);
450+
}
451+
mul
452+
}
453+
}
454+
424455
fn or_disjoint(&mut self, a: &'ll Value, b: &'ll Value) -> &'ll Value {
425456
unsafe {
426457
let or = llvm::LLVMBuildOr(self.llbuilder, a, b, UNNAMED);

‎compiler/rustc_codegen_llvm/src/llvm/ffi.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1432,6 +1432,8 @@ unsafe extern "C" {
14321432

14331433
// Extra flags on arithmetic
14341434
pub fn LLVMSetIsDisjoint(Instr: &Value, IsDisjoint: Bool);
1435+
pub fn LLVMSetNUW(ArithInst: &Value, HasNUW: Bool);
1436+
pub fn LLVMSetNSW(ArithInst: &Value, HasNSW: Bool);
14351437

14361438
// Memory
14371439
pub fn LLVMBuildAlloca<'a>(B: &Builder<'a>, Ty: &'a Type, Name: *const c_char) -> &'a Value;

‎compiler/rustc_codegen_ssa/src/size_of_val.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,9 @@ pub fn size_and_align_of_dst<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
4545
// The info in this case is the length of the str, so the size is that
4646
// times the unit size.
4747
(
48-
// All slice sizes must fit into `isize`, so this multiplication cannot (signed)
49-
// wrap.
50-
// NOTE: ideally, we want the effects of both `unchecked_smul` and `unchecked_umul`
51-
// (resulting in `mul nsw nuw` in LLVM IR), since we know that the multiplication
52-
// cannot signed wrap, and that both operands are non-negative. But at the time of
53-
// writing, the `LLVM-C` binding can't do this, and it doesn't seem to enable any
54-
// further optimizations.
55-
bx.unchecked_smul(info.unwrap(), bx.const_usize(unit.size.bytes())),
48+
// All slice sizes must fit into `isize`, so this multiplication cannot
49+
// wrap -- neither signed nor unsigned.
50+
bx.unchecked_sumul(info.unwrap(), bx.const_usize(unit.size.bytes())),
5651
bx.const_usize(unit.align.abi.bytes()),
5752
)
5853
}

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

+29-6
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,35 @@ pub trait BuilderMethods<'a, 'tcx>:
159159
/// must be interpreted as unsigned and can be assumed to be less than the size of the left
160160
/// operand.
161161
fn ashr(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
162-
fn unchecked_sadd(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
163-
fn unchecked_uadd(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
164-
fn unchecked_ssub(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
165-
fn unchecked_usub(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
166-
fn unchecked_smul(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
167-
fn unchecked_umul(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
162+
fn unchecked_sadd(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
163+
self.add(lhs, rhs)
164+
}
165+
fn unchecked_uadd(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
166+
self.add(lhs, rhs)
167+
}
168+
fn unchecked_suadd(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
169+
self.unchecked_sadd(lhs, rhs)
170+
}
171+
fn unchecked_ssub(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
172+
self.sub(lhs, rhs)
173+
}
174+
fn unchecked_usub(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
175+
self.sub(lhs, rhs)
176+
}
177+
fn unchecked_susub(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
178+
self.unchecked_ssub(lhs, rhs)
179+
}
180+
fn unchecked_smul(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
181+
self.mul(lhs, rhs)
182+
}
183+
fn unchecked_umul(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
184+
self.mul(lhs, rhs)
185+
}
186+
fn unchecked_sumul(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value {
187+
// Which to default to is a fairly arbitrary choice,
188+
// but this is what slice layout was using before.
189+
self.unchecked_smul(lhs, rhs)
190+
}
168191
fn and(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
169192
fn or(&mut self, lhs: Self::Value, rhs: Self::Value) -> Self::Value;
170193
/// Defaults to [`Self::or`], but guarantees `(lhs & rhs) == 0` so some backends

‎tests/codegen/issues/issue-96497-slice-size-nowrap.rs

+11-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
pub fn simple_size_of_nowrap(x: &[u32]) -> usize {
1212
// Make sure the shift used to compute the size has a nowrap flag.
1313

14-
// CHECK: [[A:%.*]] = shl nsw {{.*}}, 2
14+
// CHECK: [[A:%.*]] = shl nuw nsw {{.*}}, 2
1515
// CHECK-NEXT: ret {{.*}} [[A]]
1616
core::mem::size_of_val(x)
1717
}
@@ -26,3 +26,13 @@ pub fn drop_write(mut x: Box<[u32]>) {
2626
// CHECK-NOT: store i32 42
2727
x[1] = 42;
2828
}
29+
30+
// CHECK-LABEL: @slice_size_plus_2
31+
#[no_mangle]
32+
pub fn slice_size_plus_2(x: &[u16]) -> usize {
33+
// Before #136575 this didn't get the `nuw` in the add.
34+
35+
// CHECK: [[BYTES:%.+]] = shl nuw nws %x.1, 1
36+
// CHECK: = add nuw {{i16|i32|i64}} [[BYTES]], 2
37+
core::mem::size_of_val(x) + 2
38+
}

‎tests/codegen/slice-ref-equality.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub fn is_zero_array(data: &[u8; 4]) -> bool {
4747
#[no_mangle]
4848
fn eq_slice_of_nested_u8(x: &[[u8; 3]], y: &[[u8; 3]]) -> bool {
4949
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
50-
// CHECK: %[[BYTES:.+]] = mul nsw [[USIZE]] {{%x.1|%y.1}}, 3
50+
// CHECK: %[[BYTES:.+]] = mul nuw nsw [[USIZE]] {{%x.1|%y.1}}, 3
5151
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
5252
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
5353
x == y
@@ -59,7 +59,7 @@ fn eq_slice_of_nested_u8(x: &[[u8; 3]], y: &[[u8; 3]]) -> bool {
5959
#[no_mangle]
6060
fn eq_slice_of_i32(x: &[i32], y: &[i32]) -> bool {
6161
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
62-
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] {{%x.1|%y.1}}, 2
62+
// CHECK: %[[BYTES:.+]] = shl nuw nsw [[USIZE]] {{%x.1|%y.1}}, 2
6363
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
6464
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
6565
x == y
@@ -71,7 +71,7 @@ fn eq_slice_of_i32(x: &[i32], y: &[i32]) -> bool {
7171
#[no_mangle]
7272
fn eq_slice_of_nonzero(x: &[NonZero<i32>], y: &[NonZero<i32>]) -> bool {
7373
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
74-
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] {{%x.1|%y.1}}, 2
74+
// CHECK: %[[BYTES:.+]] = shl nuw nsw [[USIZE]] {{%x.1|%y.1}}, 2
7575
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
7676
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
7777
x == y
@@ -83,7 +83,7 @@ fn eq_slice_of_nonzero(x: &[NonZero<i32>], y: &[NonZero<i32>]) -> bool {
8383
#[no_mangle]
8484
fn eq_slice_of_option_of_nonzero(x: &[Option<NonZero<i16>>], y: &[Option<NonZero<i16>>]) -> bool {
8585
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
86-
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] {{%x.1|%y.1}}, 1
86+
// CHECK: %[[BYTES:.+]] = shl nuw nsw [[USIZE]] {{%x.1|%y.1}}, 1
8787
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
8888
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
8989
x == y

0 commit comments

Comments
 (0)