[PATCH] D111668: [LoopPredication] Calculate profitability without BPI

Tue Oct 19 05:43:33 PDT 2021

anna added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:971
+
+  auto ComputeBranchProbability =
+      [&](const BasicBlock *ExitingBlock,
----------------
ebrevnov wrote:
> This looks like a natural generalization of "bool Instruction::extractProfMetadata(uint64_t &TrueVal, uint64_t &FalseVal) const". Why don't we extend existing implementation of "extractProfMetadata" to handle more than 2 operands and overloaded existing API with "bool Instruction::extractProfMetadata(SmallVectorImpl<uint64_t> &) const"?
> 
> PS: Looks tempting to place total weight as a first element in the resulting vector. WDYT?
The `SmallVector` won't buy us what we need. After retrieving those individual profile data, we still need to calculate the `Numerator` by checking ` if (Term->getSuccessor(i) == ExitBlock)`, i.e. what we do at line 983.

Also, btw, we have `Instruction::extractProfTotalWeight`, which computes what I have in `Denominator`, but we anyway need to compute the `Numerator` so not sure if it's worth it.

================
Comment at: llvm/lib/Transforms/Scalar/LoopPredication.cpp:977
+    unsigned NumSucc = Term->getNumSuccessors();
+    if (isValidProfileData(ProfileData, Term)) {
+      uint64_t Numerator = 0, Denominator = 0, ProfVal = 0;
----------------
ebrevnov wrote:
> Looks like we already check that there is a "valid" profile at line 968. Why do it again?
Because the first check is for Latch terminators, while this one is for all branches exiting the loop. See usage of `ComputeBranchProbability` below.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111668