[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

Thu Aug 27 13:42:19 PDT 2020

modimo added a comment.

In D86156#2242872 <https://reviews.llvm.org/D86156#2242872>, @asbirlea wrote:

> As a general note, it may make sense to include BFI in the set of loop passes always preserved (`getLoopPassPreservedAnalyses()`), if its nature is to always be preserved (with some potential info loss) due to the callbacks deleting blocks. But since we're only looking at LICM effect for now, this can be a follow up when/if needed.

Certainly, that makes a lot of sense and makes it easier for any other loop optimization to take advantage of this data. I definitely want to make that happen, process-wise agreed that following up makes sense instead of tacking this on in addition.

================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:220
                           &getAnalysis<DominatorTreeWrapperPass>().getDomTree(),
+                          &getAnalysis<LazyBlockFrequencyInfoPass>().getBFI(),
                           &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(
----------------
nikic wrote:
> I believe that to make this actually lazy the getBFI() call needs to be guarded by some other check for presence of profiling data, otherwise it will be computed unconditionally at this point. Typically something like F.hasProfileData() or PSI.hasProfileSummary().
Appreciate the diligence here-your website is great! 

Your assessment is correct, the ".getBFI()" call for lazy will calculate BFI. I've guarded it under hasProfileData now.

I see in the "about" section that I can use your setup to test TP changes myself, here's my fork of llvm-project if that option is still available: https://github.com/modiking/llvm-project

================
Comment at: llvm/test/Other/opt-O2-pipeline.ll:281
 ; CHECK-NEXT:         Loop Invariant Code Motion
 ; CHECK-NEXT:       Lazy Branch Probability Analysis
 ; CHECK-NEXT:       Lazy Block Frequency Analysis
----------------
asbirlea wrote:
> Mark LICM to preserve these passes so they get moved above LICM rather than recomputed here (same as they are preserved in unswitch).
Marked, these are no longer calculated again.

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

https://reviews.llvm.org/D86156