[PATCH] D117302: [InstCombine] Simplify addends reordering logic

[Daniil Kovalev via Phabricator via llvm-commits]

kovdan01 created this revision.
kovdan01 added reviewers: spatel, lebedev.ri.
Herald added a subscriber: hiraditya.
kovdan01 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117302

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


Index: llvm/test/Transforms/Reassociate/fast-basictest.ll
===================================================================
--- llvm/test/Transforms/Reassociate/fast-basictest.ll
+++ llvm/test/Transforms/Reassociate/fast-basictest.ll
@@ -353,8 +353,8 @@
 ; 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
Index: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -519,13 +519,6 @@
   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,13 @@
     }
 
     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 supper-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 +569,8 @@
 
       // 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 +578,6 @@
   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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117302.399954.patch
Type: text/x-patch
Size: 2873 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220114/2f15367b/attachment.bin>