Skip to content

Commit 7f7b8ef

Browse files
committed
Auto merge of #145144 - scottmcm:unsigned_overflow_intr, r=nikic
Stop using uadd.with.overflow As discussed in [#t-compiler/llvm > `uadd.with.overflow` (again) @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/187780-t-compiler.2Fllvm/topic/.60uadd.2Ewith.2Eoverflow.60.20.28again.29/near/533041085), stop emitting `uadd.with.overflow` in favour of `add`+`icmp` instead. r? nikic
2 parents 8712e45 + 8831c5b commit 7f7b8ef

File tree

3 files changed

+41
-23
lines changed

3 files changed

+41
-23
lines changed

compiler/rustc_codegen_llvm/src/builder.rs

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -557,13 +557,25 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
557557
let (size, signed) = ty.int_size_and_signed(self.tcx);
558558
let width = size.bits();
559559

560-
if oop == OverflowOp::Sub && !signed {
561-
// Emit sub and icmp instead of llvm.usub.with.overflow. LLVM considers these
562-
// to be the canonical form. It will attempt to reform llvm.usub.with.overflow
563-
// in the backend if profitable.
564-
let sub = self.sub(lhs, rhs);
565-
let cmp = self.icmp(IntPredicate::IntULT, lhs, rhs);
566-
return (sub, cmp);
560+
if !signed {
561+
match oop {
562+
OverflowOp::Sub => {
563+
// Emit sub and icmp instead of llvm.usub.with.overflow. LLVM considers these
564+
// to be the canonical form. It will attempt to reform llvm.usub.with.overflow
565+
// in the backend if profitable.
566+
let sub = self.sub(lhs, rhs);
567+
let cmp = self.icmp(IntPredicate::IntULT, lhs, rhs);
568+
return (sub, cmp);
569+
}
570+
OverflowOp::Add => {
571+
// Like with sub above, using icmp is the preferred form. See
572+
// <https://rust-lang.zulipchat.com/#narrow/channel/187780-t-compiler.2Fllvm/topic/.60uadd.2Ewith.2Eoverflow.60.20.28again.29/near/533041085>
573+
let add = self.add(lhs, rhs);
574+
let cmp = self.icmp(IntPredicate::IntULT, add, lhs);
575+
return (add, cmp);
576+
}
577+
OverflowOp::Mul => {}
578+
}
567579
}
568580

569581
let oop_str = match oop {

tests/assembly-llvm/x86_64-bigint-helpers.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22
//@ assembly-output: emit-asm
33
//@ compile-flags: --crate-type=lib -Copt-level=3 -C target-cpu=x86-64-v4
44
//@ compile-flags: -C llvm-args=-x86-asm-syntax=intel
5-
//@ revisions: llvm-pre-20 llvm-20
6-
//@ [llvm-20] min-llvm-version: 20
7-
//@ [llvm-pre-20] max-llvm-major-version: 19
5+
//@ min-llvm-version: 20
86

97
#![no_std]
108
#![feature(bigint_helper_methods)]
@@ -23,16 +21,15 @@ pub unsafe extern "sysv64" fn bigint_chain_carrying_add(
2321
n: usize,
2422
mut carry: bool,
2523
) -> bool {
26-
// llvm-pre-20: mov [[TEMP:r..]], qword ptr [rsi + 8*[[IND:r..]] + 8]
27-
// llvm-pre-20: adc [[TEMP]], qword ptr [rdx + 8*[[IND]] + 8]
28-
// llvm-pre-20: mov qword ptr [rdi + 8*[[IND]] + 8], [[TEMP]]
29-
// llvm-pre-20: mov [[TEMP]], qword ptr [rsi + 8*[[IND]] + 16]
30-
// llvm-pre-20: adc [[TEMP]], qword ptr [rdx + 8*[[IND]] + 16]
31-
// llvm-pre-20: mov qword ptr [rdi + 8*[[IND]] + 16], [[TEMP]]
32-
// llvm-20: adc [[TEMP:r..]], qword ptr [rdx + 8*[[IND:r..]]]
33-
// llvm-20: mov qword ptr [rdi + 8*[[IND]]], [[TEMP]]
34-
// llvm-20: mov [[TEMP]], qword ptr [rsi + 8*[[IND]] + 8]
35-
// llvm-20: adc [[TEMP]], qword ptr [rdx + 8*[[IND]] + 8]
24+
// Even if we emit A+B, LLVM will sometimes reorder that to B+A, so this
25+
// test doesn't actually check which register is mov vs which is adc.
26+
27+
// CHECK: mov [[TEMP1:.+]], qword ptr [{{rdx|rsi}} + 8*[[IND:.+]] + 8]
28+
// CHECK: adc [[TEMP1]], qword ptr [{{rdx|rsi}} + 8*[[IND]] + 8]
29+
// CHECK: mov qword ptr [rdi + 8*[[IND]] + 8], [[TEMP1]]
30+
// CHECK: mov [[TEMP2:.+]], qword ptr [{{rdx|rsi}} + 8*[[IND]] + 16]
31+
// CHECK: adc [[TEMP2]], qword ptr [{{rdx|rsi}} + 8*[[IND]] + 16]
32+
// CHECK: mov qword ptr [rdi + 8*[[IND]] + 16], [[TEMP2]]
3633
for i in 0..n {
3734
(*dest.add(i), carry) = u64::carrying_add(*src1.add(i), *src2.add(i), carry);
3835
}

tests/codegen-llvm/bigint-helpers.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,20 @@
33
#![crate_type = "lib"]
44
#![feature(bigint_helper_methods)]
55

6+
// Note that there's also an assembly test for this, which is what checks for
7+
// the `ADC` (Add with Carry) instruction on x86 now that the IR we emit uses
8+
// the preferred instruction phrasing instead of the intrinsic.
9+
610
// CHECK-LABEL: @u32_carrying_add
711
#[no_mangle]
812
pub fn u32_carrying_add(a: u32, b: u32, c: bool) -> (u32, bool) {
9-
// CHECK: @llvm.uadd.with.overflow.i32
10-
// CHECK: @llvm.uadd.with.overflow.i32
11-
// CHECK: or disjoint i1
13+
// CHECK: %[[AB:.+]] = add i32 {{%a, %b|%b, %a}}
14+
// CHECK: %[[O1:.+]] = icmp ult i32 %[[AB]], %a
15+
// CHECK: %[[CEXT:.+]] = zext i1 %c to i32
16+
// CHECK: %[[ABC:.+]] = add i32 %[[AB]], %[[CEXT]]
17+
// CHECK: %[[O2:.+]] = icmp ult i32 %[[ABC]], %[[AB]]
18+
// CHECK: %[[O:.+]] = or disjoint i1 %[[O1]], %[[O2]]
19+
// CHECK: insertvalue {{.+}}, i32 %[[ABC]], 0
20+
// CHECK: insertvalue {{.+}}, i1 %[[O]], 1
1221
u32::carrying_add(a, b, c)
1322
}

0 commit comments

Comments
 (0)