[PATCH] D10979: Update the branch weight metadata in JumpThreading pass.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/Support/BlockFrequency.cpp:70
@@ +69,3 @@
+BlockFrequency BlockFrequency::operator-(const BlockFrequency &Freq) const {
+  BlockFrequency NewFreq(Frequency);
+  NewFreq -= Freq;
----------------
No underflow check here?

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1591
@@ +1590,3 @@
+  auto BBOrigFreq = BFI->getBlockFreq(BB);
+  auto PredBB2BBFreq = BFI->getBlockFreq(NewBB);
+  auto BB2SuccBBFreq = BBOrigFreq * BPI->getEdgeProbability(BB, SuccBB);
----------------
It is better to call PredBB2BBFreq NewBBFreq (with comment: frequency of the newly split BB)

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1601
@@ +1600,3 @@
+    auto SuccFreq = (*I == SuccBB)
+                        ? BB2SuccBBFreq - PredBB2BBFreq
+                        : BBOrigFreq * BPI->getEdgeProbability(BB, *I);
----------------
operator - does not have underflow check (see comment above).

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1607
@@ +1606,3 @@
+  // Normalize edge weights in Weights64 so that the sum of them can fit in
+  MachineBranchProbabilityInfo::normalizeEdgeWeights(BBSuccFreq);
+
----------------
Is this interface available in  trunk?

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1616
@@ +1615,3 @@
+    BPI->setEdgeWeight(BB, I, Weights[I]);
+
+  if (Weights.size() >= 2) {
----------------
BPI for PredBB also needs to be updated for completeness -- as its successor BB is now NewBB


http://reviews.llvm.org/D10979