[PATCH] D74809: [MBP][X86] Include static prof data when collecting loop BBs

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers accepted this revision.
nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

LGTM; thanks Bill!



================
Comment at: llvm/lib/CodeGen/MachineBlockPlacement.cpp:2517
+        const BasicBlock *BB = MBB->getBasicBlock();
+        return BB && BB->getTerminator()->hasMetadata(LLVMContext::MD_prof);
+      })) {
----------------
void wrote:
> nickdesaulniers wrote:
> > I really wish this was a method on `MachineBlockPlacement` or `MachineLoop` rather than a lambda in an `if` statement.
> Is it that it's cleaner to do it that way? The logic is going to be basically the same.
Yes, it's much nicer IMO to have short concise well named functions.  It helps make this code more readable. Thank you.


================
Comment at: llvm/test/CodeGen/Hexagon/prof-early-if.ll:5
 ; CHECK-NOT: if{{.*}}memd
+; CHECK: b5
 
----------------
nickdesaulniers wrote:
> LGTM; b5 is only branched to from b4, but b4's has higher branch weight to b2. Though it would be nice to just list all the blocks in order; otherwise I'm just guessing where the rest land.
Looks like the intent of the test is `that "if.then" was not predicated.` not necessarily anything to do with the block placement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74809