[PATCH] D147114: [LV] Use BFI to adjust cost of predicated instructions

[Evgeniy via Phabricator via llvm-commits]

ebrevnov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:10586
     BlockFrequencyInfo *BFI = nullptr;
-    if (PSI && PSI->hasProfileSummary())
+    //if (PSI && PSI->hasProfileSummary())
       BFI = &AM.getResult<BlockFrequencyAnalysis>(F);
----------------
nikic wrote:
> ebrevnov wrote:
> > nikic wrote:
> > > ebrevnov wrote:
> > > > dmgreen wrote:
> > > > > Is BFI intended to be required now? Some of the other code makes it look like it is still optional.
> > > > > Is BFI intended to be required now? 
> > > > Yes. In fact, it should look like:
> > > > BlockFrequencyInfo *BFI = &AM.getResult<BlockFrequencyAnalysis>(F);
> > > > Will fix.
> > > > 
> > > > > Some of the other code makes it look like it is still optional.
> > > > Right. Before this change there was exactly one use of BFI guarded by PSI. I would suggest not to touch this place (at least now) because if compile time regression shows up we may want to conditionally request BFI for predicated loops only.
> > > This will definitely regress compile-time. You are only allowed to query BFI in PGO builds, which implies presence of PSI.
> > But that depends on BPI availability from previous passes and needs of  BPI by following passes, right? 
> Not sure what you mean here. BPI is a dependency of BFI, so it will be calculated if not already available?
> 
> 
I meant BFI (not BPI). Sorry for confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147114