[PATCH] D81236: Improve LegacyPassManager API to correctly report modified status

[serge via Phabricator via llvm-commits]

serge-sans-paille marked an inline comment as done.
serge-sans-paille added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/LoopExtractor.cpp:135
+  bool Changed = false;
+  LoopInfo &LI = getAnalysis<LoopInfoWrapperPass>(F, &Changed).getLoopInfo();
 
----------------
ekatz wrote:
> mehdi_amini wrote:
> > ekatz wrote:
> > > serge-sans-paille wrote:
> > > > ekatz wrote:
> > > > > jdoerfert wrote:
> > > > > > ekatz wrote:
> > > > > > > Maybe I am missing something, but isn't `LoopExtractor` dependent on `BreakCriticalEdges`? I mean, what are the transformations that the `LoopInfo` analysis depends on?
> > > > > > > what are the transformations that the LoopInfo analysis depends on?
> > > > > > 
> > > > > > No transformations:
> > > > > > 
> > > > > > ```
> > > > > >   void LoopInfoWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const {                                                                                                                                              
> > > > > >     AU.setPreservesAll();
> > > > > >     AU.addRequiredTransitive<DominatorTreeWrapperPass>();
> > > > > >   }
> > > > > >   
> > > > > > ```
> > > > > So why would the `Changed` ever be set to `true`?
> > > > It's set to True because of the on the fly pass evaluation: asking for `LoopInfo` triggers the whole dependencies, including `BreakCriticalEdges`. That's why the API change I propose only impacts on the fly passes.
> > > I see.
> > What is "on the fl pass evaluation" in this context? I'm missing why `BreakCriticalEdges` isn't triggered before `LoopExtractor::runOnFunction` invocation?
> To add to your question, just to make sure I understand it correctly- if `getAnalysis` was never called, then `BreakCriticalEdges` wasn't either?
That's what I observed, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81236