[PATCH] D56848: [LCSSA] Skip blocks in sub-loops when scanning for uses.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LCSSA.cpp:328
   for (BasicBlock *BB : BlocksDominatingExits) {
+    // Skip blocks that are part of any sub-loops, as we already scanned them
+    // previously.
----------------
davide wrote:
> fhahn wrote:
> > efriedma wrote:
> > > The relevant property here is that the sub-loops are already in LCSSA form, right?  The documentation for formLCSSA doesn't actually say that explicitly... granted, it's probably true in the cases we care about.
> > That's a better way of putting it, yes! The formLCSSA uses in LCSSA.cpp guarantee that already and I checked the other 3 uses in LoopSimplify.cpp and SimpleLoopUnswitch.cpp and they either call formLCSSA depth first for their sub-loops or assert that the subloops are in LCSSA form.
> > 
> > 
> > 
> This property of sub loops should always be true, maybe there's a way of verifying this?
Yes, I think we can just check isRecursivelyLCSSAForm on all sub-loops. This is quite expensive, which is why I didn't put it in this patch. But I'll create a follow up, where it is under EXPENSIVE_CHECKS


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

https://reviews.llvm.org/D56848