[PATCH] D69069: [LoopExtractor] Convert LoopExtractor from LoopPass to ModulePass

Ehud Katz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 15 20:34:07 PST 2020


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


================
Comment at: llvm/lib/Transforms/IPO/LoopExtractor.cpp:149
+  // function is more than a minimal wrapper around the loop, extract the loop.
+  Loop *TLL = *LI.begin();
+
----------------
fhahn wrote:
> ekatz wrote:
> > fhahn wrote:
> > > Do we actually need this special case when it's a module pass? IIUC, it was only needed with the LoopPass to avoid extracting loops we already extracted into new functions earlier right?
> > This particular line was added in SVN commit r86175 (Git commit rGa83ac2d9e7b990df6978598d9dec860b5ef13a9a). Personally, I do not see a good reason not to extract loops in a non simplify-form, but the change (that added it) does not explain much, so I decided to keep it (maybe there was a related bug?).
> > If you are positive that it is not needed, then I'll remove it.
> I think the position of my original comment got messed up. I was referring to the distinction between functions with a single loop and functions with multiple loops. I *think* this was only needed with the LoopPass to avoid extracting loops we already extracted into new functions earlier which might result in an infinite loop.
> 
> I.e. just have the following starting at line 140.
> 
> ```  
> DominatorTree &DT = getAnalysis<DominatorTreeWrapperPass>(F).getDomTree();
> return extractLoops(LI.begin(), LI.end(), LI, DT);
> ```
> 
> The test coverage isn't great, but it seems to pass all tests ;)
Not sure the position got messed up - it is probably my fault. I do not sleep much these days, with my 1 year old teething or whatever he decides the reason to wake up every  hour at night...

Anyway, this code is from SVN commit rL12403 (Git commit rG2f155d87). It is from the time that the pass was a FunctionPass. It tries to distinguish between a function that has more than a single loop with a simple function wrapper around a loop (which can be created by other means than the extractor).


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

https://reviews.llvm.org/D69069





More information about the llvm-commits mailing list