[PATCH] D28534: [LCSSA] Don't let SSAUpdater to break LCSSA during LCSSA construction.

[Michael Zolotukhin via Phabricator via llvm-commits]

mzolotukhin added inline comments.


================
Comment at: lib/Transforms/Utils/LCSSA.cpp:83-84
 
+  SmallVector<Instruction *, 4> Worklist;
+  Worklist.push_back(I);
+
----------------
chandlerc wrote:
> The only hesitation I really have here is that this makes us do a lot more memory allocations when we're rewriting a lot of instructions with a lot of uses.
> 
> Is it worthwhile to try and share the worklists in some way? Have you looked at whether this worklist is just always small and so it doesn't matter in practice?
Frankly, I haven't thought about that -- thanks for pointing this out!
We can have a single worklist object, which we'll pass to this function, and that should get rid of allocations you are concerned about. We cannot use the same worklist as in `formLCSSAForInstructions` though - we clear SSAUpdater when we leave `formLCSSAForOneInstruction`, and we leave it when the worklist is empty. If we want to reuse worklist from the upper function, we need to somehow introduce boundaries at which we need to clear SSAUpdater, and I don't see a good way to do it.

If that makes sense to you, I'll update the patch so that we don't create a new worklist for every invocation of `formLCSSAForOneInstruction`.


https://reviews.llvm.org/D28534