Skip to content

Commit fa6b2c9

Browse files
committed
[DAGCombiner] don't try to partially reduce add-with-overflow ops
This transform was added with D58874, but there were no tests for overflow ops. We need to change this one way or another because it can crash as shown in: https://llvm.org/PR51238 Note that if there are no uses of an overflow op's bool overflow result, we reduce it to a regular math op, so we continue to fold that case either way. If we have uses of both the math and the overflow bool, then we are likely not saving anything by creating an independent sub instruction as seen in the test diffs here. This patch makes the behavior in SDAG consistent with what we do in instcombine AFAICT. Differential Revision: https://reviews.llvm.org/D106983
1 parent 0589351 commit fa6b2c9

File tree

3 files changed

+35
-22
lines changed

3 files changed

+35
-22
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2439,9 +2439,7 @@ SDValue DAGCombiner::visitADDLike(SDNode *N) {
24392439
N0.getOperand(0));
24402440

24412441
// fold (add (add (xor a, -1), b), 1) -> (sub b, a)
2442-
if (N0.getOpcode() == ISD::ADD ||
2443-
N0.getOpcode() == ISD::UADDO ||
2444-
N0.getOpcode() == ISD::SADDO) {
2442+
if (N0.getOpcode() == ISD::ADD) {
24452443
SDValue A, Xor;
24462444

24472445
if (isBitwiseNot(N0.getOperand(0))) {

llvm/test/CodeGen/AArch64/addsub.ll

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,10 @@ define i1 @sadd_add(i32 %a, i32 %b, i32* %p) {
230230
; CHECK-LABEL: sadd_add:
231231
; CHECK: // %bb.0:
232232
; CHECK-NEXT: mvn w8, w0
233-
; CHECK-NEXT: cmn w8, w1
234-
; CHECK-NEXT: cset w8, vs
235-
; CHECK-NEXT: sub w9, w1, w0
236-
; CHECK-NEXT: mov w0, w8
237-
; CHECK-NEXT: str w9, [x2]
233+
; CHECK-NEXT: adds w8, w8, w1
234+
; CHECK-NEXT: cset w0, vs
235+
; CHECK-NEXT: add w8, w8, #1 // =1
236+
; CHECK-NEXT: str w8, [x2]
238237
; CHECK-NEXT: ret
239238
%nota = xor i32 %a, -1
240239
%a0 = call {i32, i1} @llvm.sadd.with.overflow.i32(i32 %nota, i32 %b)
@@ -253,10 +252,9 @@ define i1 @uadd_add(i8 %a, i8 %b, i8* %p) {
253252
; CHECK-NEXT: mvn w8, w0
254253
; CHECK-NEXT: and w8, w8, #0xff
255254
; CHECK-NEXT: add w8, w8, w1, uxtb
256-
; CHECK-NEXT: lsr w8, w8, #8
257-
; CHECK-NEXT: sub w9, w1, w0
258-
; CHECK-NEXT: mov w0, w8
259-
; CHECK-NEXT: strb w9, [x2]
255+
; CHECK-NEXT: lsr w0, w8, #8
256+
; CHECK-NEXT: add w8, w8, #1 // =1
257+
; CHECK-NEXT: strb w8, [x2]
260258
; CHECK-NEXT: ret
261259
%nota = xor i8 %a, -1
262260
%a0 = call {i8, i1} @llvm.uadd.with.overflow.i8(i8 %nota, i8 %b)

llvm/test/CodeGen/X86/combine-add.ll

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -381,12 +381,11 @@ declare {i32, i1} @llvm.sadd.with.overflow.i32(i32 %a, i32 %b)
381381
define i1 @sadd_add(i32 %a, i32 %b, i32* %p) {
382382
; CHECK-LABEL: sadd_add:
383383
; CHECK: # %bb.0:
384-
; CHECK-NEXT: movl %edi, %eax
385-
; CHECK-NEXT: notl %eax
386-
; CHECK-NEXT: addl %esi, %eax
384+
; CHECK-NEXT: notl %edi
385+
; CHECK-NEXT: addl %esi, %edi
387386
; CHECK-NEXT: seto %al
388-
; CHECK-NEXT: subl %edi, %esi
389-
; CHECK-NEXT: movl %esi, (%rdx)
387+
; CHECK-NEXT: incl %edi
388+
; CHECK-NEXT: movl %edi, (%rdx)
390389
; CHECK-NEXT: retq
391390
%nota = xor i32 %a, -1
392391
%a0 = call {i32, i1} @llvm.sadd.with.overflow.i32(i32 %nota, i32 %b)
@@ -402,12 +401,11 @@ declare {i8, i1} @llvm.uadd.with.overflow.i8(i8 %a, i8 %b)
402401
define i1 @uadd_add(i8 %a, i8 %b, i8* %p) {
403402
; CHECK-LABEL: uadd_add:
404403
; CHECK: # %bb.0:
405-
; CHECK-NEXT: movl %edi, %eax
406-
; CHECK-NEXT: notb %al
407-
; CHECK-NEXT: addb %sil, %al
404+
; CHECK-NEXT: notb %dil
405+
; CHECK-NEXT: addb %sil, %dil
408406
; CHECK-NEXT: setb %al
409-
; CHECK-NEXT: subb %dil, %sil
410-
; CHECK-NEXT: movb %sil, (%rdx)
407+
; CHECK-NEXT: incb %dil
408+
; CHECK-NEXT: movb %dil, (%rdx)
411409
; CHECK-NEXT: retq
412410
%nota = xor i8 %a, -1
413411
%a0 = call {i8, i1} @llvm.uadd.with.overflow.i8(i8 %nota, i8 %b)
@@ -417,3 +415,22 @@ define i1 @uadd_add(i8 %a, i8 %b, i8* %p) {
417415
store i8 %res, i8* %p
418416
ret i1 %e1
419417
}
418+
419+
; This would crash because we tried to transform an add-with-overflow
420+
; based on the wrong result value.
421+
422+
define i1 @PR51238(i1 %b, i8 %x, i8 %y, i8 %z) {
423+
; CHECK-LABEL: PR51238:
424+
; CHECK: # %bb.0:
425+
; CHECK-NEXT: notb %cl
426+
; CHECK-NEXT: addb %dl, %cl
427+
; CHECK-NEXT: movb $1, %al
428+
; CHECK-NEXT: adcb $0, %al
429+
; CHECK-NEXT: retq
430+
%ny = xor i8 %y, -1
431+
%nz = xor i8 %z, -1
432+
%minxz = select i1 %b, i8 %x, i8 %nz
433+
%cmpyz = icmp ult i8 %ny, %nz
434+
%r = add i1 %cmpyz, true
435+
ret i1 %r
436+
}

0 commit comments

Comments
 (0)