[PATCH] D14973: Replace all weight-based interfaces in MBB with probability-based interfaces, and update all uses of old interfaces.

[Cong Hou via llvm-commits]

congh marked 3 inline comments as done.
congh added a comment.

Thanks for the review!


================
Comment at: lib/CodeGen/BranchFolding.cpp:1107
@@ -1105,1 +1106,3 @@
 
+  // Scale down SumEdgeFreq to fit in a 32-bit integer.
+  int Scale = 0;
----------------
davidxl wrote:
> I remembered we talked about adding a new constructor interface in BranchProbablity to take 64 bit integer arguments -- or replace the 32bit version. The handling of overflow in intermediate step should be handled in BP -- otherwise each client will need to deal with the overflow and scaling which is not desirable.
OK. I agree. I have added a static method in BP to build a BP from 64-bit integers.

================
Comment at: lib/CodeGen/IfConversion.cpp:1247
@@ +1246,3 @@
+    // Use zero probability as a place holder and we'll update it later.
+    BBI.BB->addSuccessor(CvtBBI->FalseBB, BranchProbability::getZero());
+    // Update the edge probability for both CvtBBI->FalseBB and NextBBI.
----------------
davidxl wrote:
> Is it cleaner to move this after NewFalseBB is computed?
> 
> After NewNext and NewFalse are computed, do
> 
> BBI.BB->addSuccessor (... , NewFalse);
> BBI.BB->setSuccProbability(NewTrueBB, NewNext)
Yes, this is much cleaner. Updated.

================
Comment at: lib/CodeGen/MachineBranchProbabilityInfo.cpp:31
@@ -30,18 +30,3 @@
 
-uint32_t MachineBranchProbabilityInfo::
-getSumForBlock(const MachineBasicBlock *MBB, uint32_t &Scale) const {
-  // First we compute the sum with 64-bits of precision, ensuring that cannot
-  // overflow by bounding the number of weights considered. Hopefully no one
-  // actually needs 2^32 successors.
-  assert(MBB->succ_size() < UINT32_MAX);
-  uint64_t Sum = 0;
-  Scale = 1;
-  for (MachineBasicBlock::const_succ_iterator I = MBB->succ_begin(),
-       E = MBB->succ_end(); I != E; ++I) {
-    uint32_t Weight = getEdgeWeight(MBB, I);
-    Sum += Weight;
-  }
-
-  // If the computed sum fits in 32-bits, we're done.
-  if (Sum <= UINT32_MAX)
-    return Sum;
+uint32_t MachineBranchProbabilityInfo::getEdgeWeight(
+    const MachineBasicBlock *Src,
----------------
davidxl wrote:
> It is not safe to keep this interface -- it gives user an impression that weight can be compared across edges with different Src BBs. Why not just change clients to use BP directly?
This interface is still used by BFI to calculate frequencies. Once we replace all weights by probabilities in BFI (on IR level) we can safely remove those two interfaces.


http://reviews.llvm.org/D14973