[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:
> > 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? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147114