[llvm-branch-commits] [llvm] 3bce613 - [DAGCombiner] don't try to partially reduce add-with-overflow ops

[Tom Stellard via llvm-branch-commits]

Author: Sanjay Patel
Date: 2021-08-02T13:52:48-07:00
New Revision: 3bce61312d4636ef6fd9e46c8d84e3fefba0569a

URL: https://github.com/llvm/llvm-project/commit/3bce61312d4636ef6fd9e46c8d84e3fefba0569a
DIFF: https://github.com/llvm/llvm-project/commit/3bce61312d4636ef6fd9e46c8d84e3fefba0569a.diff

LOG: [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

(cherry picked from commit fa6b2c9915ba27e1e97f8901ea4aa877f331fb9f)

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/test/CodeGen/AArch64/addsub.ll
    llvm/test/CodeGen/X86/combine-add.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index b104e995019fe..1bba7232eb14d 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -2439,9 +2439,7 @@ SDValue DAGCombiner::visitADDLike(SDNode *N) {
                          N0.getOperand(0));
 
     // fold (add (add (xor a, -1), b), 1) -> (sub b, a)
-    if (N0.getOpcode() == ISD::ADD ||
-        N0.getOpcode() == ISD::UADDO ||
-        N0.getOpcode() == ISD::SADDO) {
+    if (N0.getOpcode() == ISD::ADD) {
       SDValue A, Xor;
 
       if (isBitwiseNot(N0.getOperand(0))) {

diff  --git a/llvm/test/CodeGen/AArch64/addsub.ll b/llvm/test/CodeGen/AArch64/addsub.ll
index 880325c3584c1..5800676d012e8 100644
--- a/llvm/test/CodeGen/AArch64/addsub.ll
+++ b/llvm/test/CodeGen/AArch64/addsub.ll
@@ -230,11 +230,10 @@ define i1 @sadd_add(i32 %a, i32 %b, i32* %p) {
 ; CHECK-LABEL: sadd_add:
 ; CHECK:       // %bb.0:
 ; CHECK-NEXT:    mvn w8, w0
-; CHECK-NEXT:    cmn w8, w1
-; CHECK-NEXT:    cset w8, vs
-; CHECK-NEXT:    sub w9, w1, w0
-; CHECK-NEXT:    mov w0, w8
-; CHECK-NEXT:    str w9, [x2]
+; CHECK-NEXT:    adds w8, w8, w1
+; CHECK-NEXT:    cset w0, vs
+; CHECK-NEXT:    add w8, w8, #1 // =1
+; CHECK-NEXT:    str w8, [x2]
 ; CHECK-NEXT:    ret
   %nota = xor i32 %a, -1
   %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) {
 ; CHECK-NEXT:    mvn w8, w0
 ; CHECK-NEXT:    and w8, w8, #0xff
 ; CHECK-NEXT:    add w8, w8, w1, uxtb
-; CHECK-NEXT:    lsr w8, w8, #8
-; CHECK-NEXT:    sub w9, w1, w0
-; CHECK-NEXT:    mov w0, w8
-; CHECK-NEXT:    strb w9, [x2]
+; CHECK-NEXT:    lsr w0, w8, #8
+; CHECK-NEXT:    add w8, w8, #1 // =1
+; CHECK-NEXT:    strb w8, [x2]
 ; CHECK-NEXT:    ret
   %nota = xor i8 %a, -1
   %a0 = call {i8, i1} @llvm.uadd.with.overflow.i8(i8 %nota, i8 %b)

diff  --git a/llvm/test/CodeGen/X86/combine-add.ll b/llvm/test/CodeGen/X86/combine-add.ll
index 3bff9ea8b8469..0c38d41190e09 100644
--- a/llvm/test/CodeGen/X86/combine-add.ll
+++ b/llvm/test/CodeGen/X86/combine-add.ll
@@ -381,12 +381,11 @@ declare {i32, i1} @llvm.sadd.with.overflow.i32(i32 %a, i32 %b)
 define i1 @sadd_add(i32 %a, i32 %b, i32* %p) {
 ; CHECK-LABEL: sadd_add:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl %edi, %eax
-; CHECK-NEXT:    notl %eax
-; CHECK-NEXT:    addl %esi, %eax
+; CHECK-NEXT:    notl %edi
+; CHECK-NEXT:    addl %esi, %edi
 ; CHECK-NEXT:    seto %al
-; CHECK-NEXT:    subl %edi, %esi
-; CHECK-NEXT:    movl %esi, (%rdx)
+; CHECK-NEXT:    incl %edi
+; CHECK-NEXT:    movl %edi, (%rdx)
 ; CHECK-NEXT:    retq
   %nota = xor i32 %a, -1
   %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)
 define i1 @uadd_add(i8 %a, i8 %b, i8* %p) {
 ; CHECK-LABEL: uadd_add:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl %edi, %eax
-; CHECK-NEXT:    notb %al
-; CHECK-NEXT:    addb %sil, %al
+; CHECK-NEXT:    notb %dil
+; CHECK-NEXT:    addb %sil, %dil
 ; CHECK-NEXT:    setb %al
-; CHECK-NEXT:    subb %dil, %sil
-; CHECK-NEXT:    movb %sil, (%rdx)
+; CHECK-NEXT:    incb %dil
+; CHECK-NEXT:    movb %dil, (%rdx)
 ; CHECK-NEXT:    retq
   %nota = xor i8 %a, -1
   %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) {
   store i8 %res, i8* %p
   ret i1 %e1
 }
+
+; This would crash because we tried to transform an add-with-overflow
+; based on the wrong result value.
+
+define i1 @PR51238(i1 %b, i8 %x, i8 %y, i8 %z) {
+; CHECK-LABEL: PR51238:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    notb %cl
+; CHECK-NEXT:    addb %dl, %cl
+; CHECK-NEXT:    movb $1, %al
+; CHECK-NEXT:    adcb $0, %al
+; CHECK-NEXT:    retq
+   %ny = xor i8 %y, -1
+   %nz = xor i8 %z, -1
+   %minxz = select i1 %b, i8 %x, i8 %nz
+   %cmpyz = icmp ult i8 %ny, %nz
+   %r = add i1 %cmpyz, true
+   ret i1 %r
+}