Skip to content

Commit bdc97d1

Browse files
borsbors
committed
Auto merge of rust-lang#136575 - scottmcm:nsuw-math, r=nikic
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](https://github.com/llvm/llvm-project/blob/f0b8ff12519270adcfef93410abff76ab073476a/llvm/include/llvm-c/Core.h#L4403-L4408), we might as well. And it's been there since what looks like LLVM 17 llvm/llvm-project@de9b6aa so doesn't even need to be conditional. (There's other places, like `RawVecInner` or `Layout`, that might want to do things like this too, but I'll leave those for a future PR.)
2 parents d88ffcd + 9ad6839 commit bdc97d1

File tree

7 files changed

+81
-44
lines changed

7 files changed

+81
-44
lines changed

Diff for: ‎compiler/rustc_codegen_gcc/src/builder.rs

+1-25
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
665665
a + b
666666
}
667667

668+
// TODO(antoyo): should we also override the `unchecked_` versions?
668669
fn sub(&mut self, a: RValue<'gcc>, b: RValue<'gcc>) -> RValue<'gcc> {
669670
self.gcc_sub(a, b)
670671
}
@@ -832,31 +833,6 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
832833
set_rvalue_location(self, self.gcc_not(a))
833834
}
834835

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

Diff for: ‎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);

Diff for: ‎compiler/rustc_codegen_llvm/src/llvm/ffi.rs

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

14311431
// Extra flags on arithmetic
14321432
pub(crate) fn LLVMSetIsDisjoint(Instr: &Value, IsDisjoint: Bool);
1433+
pub(crate) fn LLVMSetNUW(ArithInst: &Value, HasNUW: Bool);
1434+
pub(crate) fn LLVMSetNSW(ArithInst: &Value, HasNSW: Bool);
14331435

14341436
// Memory
14351437
pub(crate) fn LLVMBuildAlloca<'a>(

Diff for: ‎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
}

Diff for: ‎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

Diff for: ‎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 nsw {{i16|i32|i64}} %x.1, 1
36+
// CHECK: = add nuw {{i16|i32|i64}} [[BYTES]], 2
37+
core::mem::size_of_val(x) + 2
38+
}

Diff for: ‎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)