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

Wed Mar 29 05:37:53 PDT 2023

dmgreen added a comment.

Hello. This sounds like a nice idea. I sometimes worry about practice not matching theory, but I hope it should an improvement for the most part. Do you have any benchmark data to back it up?

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5596
+    InstructionCost &Cost, const BasicBlock *BB) const {
+  assert(Cost.isValid() && "Can't scale invalid cost");
+
----------------
Could invalid cost handling be pushed into this function. It would help not needing it at all the call sites.

================
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);
----------------
Is BFI intended to be required now? Some of the other code makes it look like it is still optional.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147114