[llvm] 598721f - [InstCombine] try harder to propagate 'nsz' through fneg-of-select

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2022-03-15T11:05:29-04:00
New Revision: 598721f866156ba1259af37d9b30b1ec95983678

URL: https://github.com/llvm/llvm-project/commit/598721f866156ba1259af37d9b30b1ec95983678
DIFF: https://github.com/llvm/llvm-project/commit/598721f866156ba1259af37d9b30b1ec95983678.diff

LOG: [InstCombine] try harder to propagate 'nsz' through fneg-of-select

This can be viewed as swapping the select arms:
https://alive2.llvm.org/ce/z/jUvFMJ
...so we don't have the 'nsz' problem with the more general fold.

This unlocks other folds for the motivating fabs example.
This was discussed in issue #38828.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
    llvm/test/Transforms/InstCombine/fneg.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index ff186d1f3e7eb..de79fe0ec507d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -2268,10 +2268,10 @@ Instruction *InstCombinerImpl::visitFNeg(UnaryOperator &I) {
     // Unlike most transforms, this one is not safe to propagate nsz unless
     // it is present on the original select. (We are conservatively intersecting
     // the nsz flags from the select and root fneg instruction.)
-    auto propagateSelectFMF = [&](SelectInst *S) {
+    auto propagateSelectFMF = [&](SelectInst *S, bool CommonOperand) {
       S->copyFastMathFlags(&I);
       if (auto *OldSel = dyn_cast<SelectInst>(Op))
-        if (!OldSel->hasNoSignedZeros() &&
+        if (!OldSel->hasNoSignedZeros() && !CommonOperand &&
             !isGuaranteedNotToBeUndefOrPoison(OldSel->getCondition()))
           S->setHasNoSignedZeros(false);
     };
@@ -2280,14 +2280,14 @@ Instruction *InstCombinerImpl::visitFNeg(UnaryOperator &I) {
     if (match(X, m_FNeg(m_Value(P)))) {
       Value *NegY = Builder.CreateFNegFMF(Y, &I, Y->getName() + ".neg");
       SelectInst *NewSel = SelectInst::Create(Cond, P, NegY);
-      propagateSelectFMF(NewSel);
+      propagateSelectFMF(NewSel, P == Y);
       return NewSel;
     }
     // -(Cond ? X : -P) --> Cond ? -X : P
     if (match(Y, m_FNeg(m_Value(P)))) {
       Value *NegX = Builder.CreateFNegFMF(X, &I, X->getName() + ".neg");
       SelectInst *NewSel = SelectInst::Create(Cond, NegX, P);
-      propagateSelectFMF(NewSel);
+      propagateSelectFMF(NewSel, P == X);
       return NewSel;
     }
   }

diff  --git a/llvm/test/Transforms/InstCombine/fneg.ll b/llvm/test/Transforms/InstCombine/fneg.ll
index 2af1e6ea26b5b..c111b0080ee0f 100644
--- a/llvm/test/Transforms/InstCombine/fneg.ll
+++ b/llvm/test/Transforms/InstCombine/fneg.ll
@@ -674,10 +674,12 @@ define float @select_fneg_false_nsz(float %x, float %y, i1 %b) {
   ret float %r
 }
 
+; Special-case for propagating nsz: it's ok when selecting between an operand and its negation.
+
 define float @select_common_op_fneg_true(float %x, i1 %b) {
 ; CHECK-LABEL: @select_common_op_fneg_true(
 ; CHECK-NEXT:    [[X_NEG:%.*]] = fneg nnan ninf nsz float [[X:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = select nnan ninf i1 [[B:%.*]], float [[X_NEG]], float [[X]]
+; CHECK-NEXT:    [[R:%.*]] = select nnan ninf nsz i1 [[B:%.*]], float [[X_NEG]], float [[X]]
 ; CHECK-NEXT:    ret float [[R]]
 ;
   %nx = fneg float %x
@@ -689,7 +691,7 @@ define float @select_common_op_fneg_true(float %x, i1 %b) {
 define float @select_common_op_fneg_false(float %x, i1 %b) {
 ; CHECK-LABEL: @select_common_op_fneg_false(
 ; CHECK-NEXT:    [[X_NEG:%.*]] = fneg nnan ninf nsz float [[X:%.*]]
-; CHECK-NEXT:    [[R:%.*]] = select nnan ninf i1 [[B:%.*]], float [[X_NEG]], float [[X]]
+; CHECK-NEXT:    [[R:%.*]] = select nnan ninf nsz i1 [[B:%.*]], float [[X_NEG]], float [[X]]
 ; CHECK-NEXT:    ret float [[R]]
 ;
   %nx = fneg float %x
@@ -698,12 +700,12 @@ define float @select_common_op_fneg_false(float %x, i1 %b) {
   ret float %r
 }
 
+; The transform above allows follow-on folds to convert to fabs.
+
 define float @fabs(float %a) {
 ; CHECK-LABEL: @fabs(
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp ogt float [[A:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[A_NEG:%.*]] = fneg nnan ninf nsz float [[A]]
-; CHECK-NEXT:    [[FNEG1:%.*]] = select nnan ninf i1 [[CMP]], float [[A]], float [[A_NEG]]
-; CHECK-NEXT:    ret float [[FNEG1]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call nnan ninf nsz float @llvm.fabs.f32(float [[A:%.*]])
+; CHECK-NEXT:    ret float [[TMP1]]
 ;
   %fneg = fneg float %a
   %cmp = fcmp ogt float %a, %fneg
@@ -712,11 +714,13 @@ define float @fabs(float %a) {
   ret float %fneg1
 }
 
+; TODO: This should reduce to fneg-of-fabs.
+
 define float @fnabs(float %a) {
 ; CHECK-LABEL: @fnabs(
 ; CHECK-NEXT:    [[CMP:%.*]] = fcmp olt float [[A:%.*]], 0.000000e+00
 ; CHECK-NEXT:    [[A_NEG:%.*]] = fneg fast float [[A]]
-; CHECK-NEXT:    [[FNEG1:%.*]] = select reassoc nnan ninf arcp contract afn i1 [[CMP]], float [[A]], float [[A_NEG]]
+; CHECK-NEXT:    [[FNEG1:%.*]] = select fast i1 [[CMP]], float [[A]], float [[A_NEG]]
 ; CHECK-NEXT:    ret float [[FNEG1]]
 ;
   %fneg = fneg float %a