[llvm] d8e0e12 - [InstCombine] Simplify addends reordering logic

[Andrew Savonichev via llvm-commits]

Author: Daniil Kovalev
Date: 2022-01-18T16:00:47+03:00
New Revision: d8e0e125a2ff03335838039f5d33ecd61dfec654

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

LOG: [InstCombine] Simplify addends reordering logic

Previously some constants were not pushed to the top of the resulting
expression tree as intended by the algorithm. We can remove the logic
from simplifyFAdd and rely on SimplifyAssociativeOrCommutative to do
that.

Differential Revision: https://reviews.llvm.org/D117302

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
    llvm/test/Transforms/Reassociate/fast-basictest.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 7f4f5d880e690..22dac42cd9da2 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -519,13 +519,6 @@ Value *FAddCombine::simplifyFAdd(AddendVect& Addends, unsigned InstrQuota) {
   unsigned NextTmpIdx = 0;
   FAddend TmpResult[3];
 
-  // Points to the constant addend of the resulting simplified expression.
-  // If the resulting expr has constant-addend, this constant-addend is
-  // desirable to reside at the top of the resulting expression tree. Placing
-  // constant close to supper-expr(s) will potentially reveal some optimization
-  // opportunities in super-expr(s).
-  const FAddend *ConstAdd = nullptr;
-
   // Simplified addends are placed <SimpVect>.
   AddendVect SimpVect;
 
@@ -541,6 +534,14 @@ Value *FAddCombine::simplifyFAdd(AddendVect& Addends, unsigned InstrQuota) {
     }
 
     Value *Val = ThisAddend->getSymVal();
+
+    // If the resulting expr has constant-addend, this constant-addend is
+    // desirable to reside at the top of the resulting expression tree. Placing
+    // constant close to super-expr(s) will potentially reveal some
+    // optimization opportunities in super-expr(s). Here we do not implement
+    // this logic intentionally and rely on SimplifyAssociativeOrCommutative
+    // call later.
+
     unsigned StartIdx = SimpVect.size();
     SimpVect.push_back(ThisAddend);
 
@@ -569,14 +570,8 @@ Value *FAddCombine::simplifyFAdd(AddendVect& Addends, unsigned InstrQuota) {
 
       // Pop all addends being folded and push the resulting folded addend.
       SimpVect.resize(StartIdx);
-      if (Val) {
-        if (!R.isZero()) {
-          SimpVect.push_back(&R);
-        }
-      } else {
-        // Don't push constant addend at this time. It will be the last element
-        // of <SimpVect>.
-        ConstAdd = &R;
+      if (!R.isZero()) {
+        SimpVect.push_back(&R);
       }
     }
   }
@@ -584,9 +579,6 @@ Value *FAddCombine::simplifyFAdd(AddendVect& Addends, unsigned InstrQuota) {
   assert((NextTmpIdx <= array_lengthof(TmpResult) + 1) &&
          "out-of-bound access");
 
-  if (ConstAdd)
-    SimpVect.push_back(ConstAdd);
-
   Value *Result;
   if (!SimpVect.empty())
     Result = createNaryFAdd(SimpVect, InstrQuota);

diff  --git a/llvm/test/Transforms/Reassociate/fast-basictest.ll b/llvm/test/Transforms/Reassociate/fast-basictest.ll
index 9b38f3b88c39e..561a406daa2c6 100644
--- a/llvm/test/Transforms/Reassociate/fast-basictest.ll
+++ b/llvm/test/Transforms/Reassociate/fast-basictest.ll
@@ -353,8 +353,8 @@ define float @test12(float %X) {
 ; Check again with 'reassoc' and 'nsz' ('nsz' not technically required).
 define float @test12_reassoc_nsz(float %X) {
 ; CHECK-LABEL: @test12_reassoc_nsz(
-; CHECK-NEXT:    [[TMP1:%.*]] = fmul reassoc nsz float [[X:%.*]], 3.000000e+00
-; CHECK-NEXT:    [[TMP2:%.*]] = fsub reassoc nsz float 6.000000e+00, [[TMP1]]
+; CHECK-NEXT:    [[TMP1:%.*]] = fmul reassoc nsz float [[X:%.*]], -3.000000e+00
+; CHECK-NEXT:    [[TMP2:%.*]] = fadd reassoc nsz float [[TMP1]], 6.000000e+00
 ; CHECK-NEXT:    ret float [[TMP2]]
 ;
   %A = fsub reassoc nsz float 1.000000e+00, %X