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

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Transforms/Utils/LCSSA.cpp:83-84
 
+  SmallVector<Instruction *, 4> Worklist;
+  Worklist.push_back(I);
+
----------------
mzolotukhin wrote:
> 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`.
Yeah, that makes sense.

Do we ever really want formLCSSForOneInstruction? I wonder if we should just leave the single function and scope the SSAUpdater more tightly. Happy to leave the code structuring question up to you though.


https://reviews.llvm.org/D28534