[PATCH] D71538: [SCEVExpander] Preserve LCSSA directly.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:1791
+    if (auto *Inst = dyn_cast<Instruction>(V)) {
+      // Create a temporary instruction to at the current insertion point, so we
+      // can hand it off to the helper to create LCSSA PHIs if required for the
----------------
mkazantsev wrote:
> fhahn wrote:
> > mkazantsev wrote:
> > > I don't quite get this logic. If all you need it for is an insertion point, why not just take next/prev iterator if `Inst` in its block?
> > This code here is a preparation step so we can use `formLCSSAForInstructions` to fix the LCSSA form. `formLCSSAForInstructions` will insert LCSSA phi nodes as required for all uses of a given instruction. 
> > 
> > The problem here is that we do not have a use at the insertion point yet, as the caller will insert the use. So to conveniently use the existing  `formLCSSAForInstructions`, it is easiest to just create a temporary user at the insertion point, so LCSSA phis are inserted for uses at insertion point, if required. Does that make sense?
> I get it, thanks. Looks fishy, I'm certain there should be a cleaner way to do it, but I cannot come up with one on the top of my head. Please add a TODO to consider a better way to do it.
Agreed, I added a comment. I will look into this as follow-up, unless some other fundamental issues are exposed by the patches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71538/new/

https://reviews.llvm.org/D71538