[PATCH] D22378: Make processInstruction from LCSSA.cpp externally available.

[Chandler Carruth via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM with a tweak below. Also see my note about the exit blocks.


================
Comment at: lib/Transforms/Utils/LCSSA.cpp:69-70
@@ -76,1 +68,4 @@
+  while (!Worklist.empty()) {
+    SmallVector<Use *, 16> UsesToRewrite;
+    SmallVector<BasicBlock *, 8> ExitBlocks;
 
----------------
Hoist these outside the loop and clear them on each iteration?

================
Comment at: lib/Transforms/Utils/LCSSA.cpp:74-75
@@ -82,1 +73,4 @@
+    BasicBlock *InstBB = I->getParent();
+    Loop *L = LI.getLoopFor(InstBB);
+    L->getExitBlocks(ExitBlocks);
 
----------------
Wow, I really didn't think about how costly this'll end up being...

I think your patch is probably fine as-is because the exit blocks lists are probably pretty small in practice.

But I think in a follow-up patch we should change this to use a better API. The fact that the LoopInfo analysis just populates a small vector is pretty horrible. We should add a nice iterator that just walks the exit blocks and use that so there is no copying or churn here. Then I think the worklist becomes a complete win. It should also mean you can do stuff across multiple loops if useful here without really stressing about it.

Does that make sense to you?


https://reviews.llvm.org/D22378