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

[Nikita Popov via Phabricator via llvm-commits]

nikic 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);
----------------
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?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147114