[PATCH] D22599: GVN-hoist: improve code generation for recursive GEPs

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:578-603
@@ +577,28 @@
+  // Make all operands of the GEP available.
+  bool makeGepOperandsAvailable(GetElementPtrInst *Gep, BasicBlock *HoistPt,
+                                Instruction *Repl) const {
+    for (unsigned i = 0, e = Gep->getNumOperands(); i != e; ++i)
+      if (Instruction *Op = dyn_cast<Instruction>(Gep->getOperand(i))) {
+
+        // Check whether the operand is already available.
+        if (DT->dominates(Op->getParent(), HoistPt))
+          continue;
+
+        // As a GEP can refer to other GEPs, recursively check whether all the
+        // operands of this GEP can be computed at HoistPt.
+        if (GetElementPtrInst *GepOp = dyn_cast<GetElementPtrInst>(Op))
+          if (!makeGepOperandsAvailable(GepOp, HoistPt, Gep))
+            return false;
+
+        // Failed to make Op available.
+        return false;
+      }
+
+    // Copy Gep and replace its uses in Repl with ClonedGep.
+    Instruction *ClonedGep = Gep->clone();
+    ClonedGep->insertBefore(HoistPt->getTerminator());
+    Repl->replaceUsesOfWith(Gep, ClonedGep);
+
+    return true;
+  }
+
----------------
IIUC, this can mutate the IR while still returning false.

I'd change this to be a worklist algorithm instead of a recursive one.  Inside the worklist loop, I'd push GEP candidates onto a vector for later processing (cloning).


Repository:
  rL LLVM

https://reviews.llvm.org/D22599