[SelectionDAG] Removing FABS folding from DAGCombiner

rotateright · rotateright · commit e09b7dcf3d64 · 2018-03-30T15:42:52.000Z
The code has bugs dealing with -0.0. Since D44550 introduced FABS pattern folding in InstCombine, this patch removes the now-redundant code that causes https://bugs.llvm.org/show_bug.cgi?id=36600. Patch by Mikhail Dvoretckii! Differential Revision: https://reviews.llvm.org/D44683 llvm-svn: 328872
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -16854,24 +16854,6 @@ SDValue DAGCombiner::SimplifySelectCC(const SDLoc &DL, SDValue N0, SDValue N1,
     return !SCCC->isNullValue() ? N2 : N3;
   }
 
-  // Check to see if we can simplify the select into an fabs node
-  if (ConstantFPSDNode *CFP = dyn_cast<ConstantFPSDNode>(N1)) {
-    // Allow either -0.0 or 0.0
-    if (CFP->isZero()) {
-      // select (setg[te] X, +/-0.0), X, fneg(X) -> fabs
-      if ((CC == ISD::SETGE || CC == ISD::SETGT) &&
-          N0 == N2 && N3.getOpcode() == ISD::FNEG &&
-          N2 == N3.getOperand(0))
-        return DAG.getNode(ISD::FABS, DL, VT, N0);
-
-      // select (setl[te] X, +/-0.0), fneg(X), X -> fabs
-      if ((CC == ISD::SETLT || CC == ISD::SETLE) &&
-          N0 == N3 && N2.getOpcode() == ISD::FNEG &&
-          N2.getOperand(0) == N3)
-        return DAG.getNode(ISD::FABS, DL, VT, N3);
-    }
-  }
-
   // Turn "(a cond b) ? 1.0f : 2.0f" into "load (tmp + ((a cond b) ? 0 : 4)"
   // where "tmp" is a constant pool entry containing an array with 1.0 and 2.0
   // in it.  This is a win when the constant is not otherwise available because
diff --git a/llvm/test/CodeGen/AArch64/fabs.ll b/llvm/test/CodeGen/AArch64/fabs.ll
@@ -1,14 +1,15 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=aarch64-unknown-unknown | FileCheck %s
 
-; FIXME:
-; PR36600: https://bugs.llvm.org/show_bug.cgi?id=36600
+; Test against PR36600: https://bugs.llvm.org/show_bug.cgi?id=36600
 ; This is not fabs. If X = -0.0, it should return -0.0 not 0.0.
 
 define double @not_fabs(double %x) #0 {
 ; CHECK-LABEL: not_fabs:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    fabs d0, d0
+; CHECK-NEXT:    fneg d1, d0
+; CHECK-NEXT:    fcmp d0, #0.0
+; CHECK-NEXT:    fcsel d0, d1, d0, le
 ; CHECK-NEXT:    ret
   %cmp = fcmp nnan ole double %x, 0.0
   %sub = fsub nnan double -0.0, %x
@@ -21,7 +22,11 @@ define double @not_fabs(double %x) #0 {
 define float @still_not_fabs(float %x) #0 {
 ; CHECK-LABEL: still_not_fabs:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    fabs s0, s0
+; CHECK-NEXT:    adrp x8, .LCPI1_0
+; CHECK-NEXT:    ldr s1, [x8, :lo12:.LCPI1_0]
+; CHECK-NEXT:    fneg s2, s0
+; CHECK-NEXT:    fcmp s0, s1
+; CHECK-NEXT:    fcsel s0, s0, s2, ge
 ; CHECK-NEXT:    ret
   %cmp = fcmp nnan oge float %x, -0.0
   %sub = fsub nnan float -0.0, %x
diff --git a/llvm/test/CodeGen/X86/fabs.ll b/llvm/test/CodeGen/X86/fabs.ll
@@ -27,67 +27,20 @@ define float @test1(float %X) {
   ret float %Y
 }
 
-define double @test2(double %X) {
+define x86_fp80 @test2(x86_fp80 %X) {
 ; X87-LABEL: test2:
 ; X87:       # %bb.0:
-; X87-NEXT:    fldl {{[0-9]+}}(%esp)
-; X87-NEXT:    fldz
-; X87-NEXT:    fchs
-; X87-NEXT:    fxch %st(1)
-; X87-NEXT:    fucom %st(1)
-; X87-NEXT:    fstp %st(1)
-; X87-NEXT:    fnstsw %ax
-; X87-NEXT:    # kill: def $ah killed $ah killed $ax
-; X87-NEXT:    sahf
-; X87-NEXT:    fld %st(0)
-; X87-NEXT:    fchs
-; X87-NEXT:    jae .LBB1_2
-; X87-NEXT:  # %bb.1:
-; X87-NEXT:    fstp %st(1)
-; X87-NEXT:    fldz
-; X87-NEXT:  .LBB1_2:
-; X87-NEXT:    fstp %st(0)
-; X87-NEXT:    retl
-;
-; X87UNSAFE-LABEL: test2:
-; X87UNSAFE:       # %bb.0:
-; X87UNSAFE-NEXT:    fldl {{[0-9]+}}(%esp)
-; X87UNSAFE-NEXT:    fabs
-; X87UNSAFE-NEXT:    retl
-;
-; X64-LABEL: test2:
-; X64:       # %bb.0:
-; X64-NEXT:    movsd {{.*#+}} xmm1 = mem[0],zero
-; X64-NEXT:    movapd %xmm1, %xmm2
-; X64-NEXT:    unpcklpd {{.*#+}} xmm2 = xmm2[0],xmm1[0]
-; X64-NEXT:    xorpd %xmm0, %xmm2
-; X64-NEXT:    cmplesd %xmm0, %xmm1
-; X64-NEXT:    movapd %xmm1, %xmm3
-; X64-NEXT:    andnpd %xmm2, %xmm3
-; X64-NEXT:    andpd %xmm0, %xmm1
-; X64-NEXT:    orpd %xmm3, %xmm1
-; X64-NEXT:    movapd %xmm1, %xmm0
-; X64-NEXT:    retq
-  %Y = fcmp oge double %X, -0.0
-  %Z = fsub double -0.0, %X
-  %Q = select i1 %Y, double %X, double %Z
-  ret double %Q
-}
-
-define x86_fp80 @test3(x86_fp80 %X) {
-; X87-LABEL: test3:
-; X87:       # %bb.0:
 ; X87-NEXT:    fldt {{[0-9]+}}(%esp)
 ; X87-NEXT:    fabs
 ; X87-NEXT:    retl
 ;
-; X87UNSAFE-LABEL: test3:
+; X87UNSAFE-LABEL: test2:
 ; X87UNSAFE:       # %bb.0:
 ; X87UNSAFE-NEXT:    fldt {{[0-9]+}}(%esp)
 ; X87UNSAFE-NEXT:    fabs
 ; X87UNSAFE-NEXT:    retl
 ;
-; X64-LABEL: test3:
+; X64-LABEL: test2:
 ; X64:       # %bb.0:
 ; X64-NEXT:    fldt {{[0-9]+}}(%rsp)
 ; X64-NEXT:    fabs
