[PATCH] D18155: Instcombine: try to avoid wasted work in ConstantFold

[escha via llvm-commits]

escha created this revision.
escha added reviewers: resistor, bogner.
escha added a subscriber: llvm-commits.
escha set the repository for this revision to rL LLVM.

This is a bit tricky, and if someone has a better idea, please feel free to suggest it!

InstCombine currently does the following:
1. Call ConstantFoldInstruction on the instruction, and replace it with a constant if appropriate. This involves (internally) iterating over its operands and optimizing each ConstantExpr operand with ConstantFoldConstantExpression, then calling ConstantFold on the instruction with operands.
2. If that didn't work, optimize each ConstantExpr operand with ConstantFoldConstantExpression.

However, if you have lots of instructions with ConstantExpr operands but which cannot be folded (ex: loads from constant locations in memory -- in our case, uniform buffers are a common case), this means you call ConstantFoldConstantExpression twice on every operand, every time, because the constant folding fails.

One possible solution (as written here) is to provide an option to ConstantFoldInstruction to tell it not to try to optimize the ConstantExpr arguments, and then to reorder 1) and 2) so that we optimize the ConstantExprs first. I don't know if this is the best option, but it saves ~3-4% of time in instcombine for us.

Repository:
  rL LLVM

http://reviews.llvm.org/D18155

Files:
  include/llvm/Analysis/ConstantFolding.h
  lib/Analysis/ConstantFolding.cpp
  lib/Transforms/InstCombine/InstructionCombining.cpp

Index: lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- lib/Transforms/InstCombine/InstructionCombining.cpp
+++ lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2872,18 +2872,6 @@
         continue;
       }
 
-      // ConstantProp instruction if trivially constant.
-      if (!Inst->use_empty() &&
-          (Inst->getNumOperands() == 0 || isa<Constant>(Inst->getOperand(0))))
-        if (Constant *C = ConstantFoldInstruction(Inst, DL, TLI)) {
-          DEBUG(dbgs() << "IC: ConstFold to: " << *C << " from: "
-                       << *Inst << '\n');
-          Inst->replaceAllUsesWith(C);
-          ++NumConstProp;
-          Inst->eraseFromParent();
-          continue;
-        }
-
       // See if we can constant fold its operands.
       for (User::op_iterator i = Inst->op_begin(), e = Inst->op_end(); i != e;
            ++i) {
@@ -2903,6 +2891,18 @@
         }
       }
 
+      // ConstantProp instruction if trivially constant.
+      if (!Inst->use_empty() &&
+          (Inst->getNumOperands() == 0 || isa<Constant>(Inst->getOperand(0))))
+        if (Constant *C = ConstantFoldInstruction(Inst, DL, TLI, false  )) {
+          DEBUG(dbgs() << "IC: ConstFold to: " << *C << " from: "
+                       << *Inst << '\n');
+          Inst->replaceAllUsesWith(C);
+          ++NumConstProp;
+          Inst->eraseFromParent();
+          continue;
+        }
+
       InstrsForInstCombineWorklist.push_back(Inst);
     }
 
Index: lib/Analysis/ConstantFolding.cpp
===================================================================
--- lib/Analysis/ConstantFolding.cpp
+++ lib/Analysis/ConstantFolding.cpp
@@ -894,7 +894,8 @@
 /// this function can only fail when attempting to fold instructions like loads
 /// and stores, which have no constant expression form.
 Constant *llvm::ConstantFoldInstruction(Instruction *I, const DataLayout &DL,
-                                        const TargetLibraryInfo *TLI) {
+                                        const TargetLibraryInfo *TLI,
+                                        bool FoldOperands) {
   // Handle PHI nodes quickly here...
   if (PHINode *PN = dyn_cast<PHINode>(I)) {
     Constant *CommonValue = nullptr;
@@ -911,7 +912,8 @@
       if (!C)
         return nullptr;
       // Fold the PHI's operands.
-      if (ConstantExpr *NewC = dyn_cast<ConstantExpr>(C))
+      ConstantExpr *NewC = dyn_cast<ConstantExpr>(C);
+      if (NewC && FoldOperands)
         C = ConstantFoldConstantExpression(NewC, DL, TLI);
       // If the incoming value is a different constant to
       // the one we saw previously, then give up.
@@ -932,7 +934,8 @@
   for (User::op_iterator i = I->op_begin(), e = I->op_end(); i != e; ++i) {
     Constant *Op = cast<Constant>(*i);
     // Fold the Instruction's operands.
-    if (ConstantExpr *NewCE = dyn_cast<ConstantExpr>(Op))
+    ConstantExpr *NewCE = dyn_cast<ConstantExpr>(Op);
+    if (NewCE && FoldOperands)
       Op = ConstantFoldConstantExpression(NewCE, DL, TLI);
 
     Ops.push_back(Op);
Index: include/llvm/Analysis/ConstantFolding.h
===================================================================
--- include/llvm/Analysis/ConstantFolding.h
+++ include/llvm/Analysis/ConstantFolding.h
@@ -36,8 +36,11 @@
 /// Note that this fails if not all of the operands are constant.  Otherwise,
 /// this function can only fail when attempting to fold instructions like loads
 /// and stores, which have no constant expression form.
+/// Only attempt to constant-fold ConstantExpr operands if FoldOperands
+/// is set to true; set this to false if they are already known to be folded.
   Constant *ConstantFoldInstruction(Instruction *I, const DataLayout &DL,
-                                    const TargetLibraryInfo *TLI = nullptr);
+                                    const TargetLibraryInfo *TLI = nullptr,
+                                    bool FoldOperands = true);
 
 /// ConstantFoldConstantExpression - Attempt to fold the constant expression
 /// using the specified DataLayout.  If successful, the constant result is


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D18155.50624.patch
Type: text/x-patch
Size: 4143 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160314/90128b9f/attachment.bin>