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

[David]

davidxl added a comment.

I suggest also adding another test case (documented in the PR -- the one that has problem with jump-threading + loop-rotation + jump-threading).

This reminds me of a way to do profile sanity check/validation when debug check is turned on:

1. compute the BB freq before the pass
2. cfg transformation + BB freq update
3. branch probability weight update

4. recompute BFI using updated weight in 3). The result of 4) should match that of 2).


================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1572
@@ +1571,3 @@
+  // frequency of BB.
+  auto BBFreq = BFI->getBlockFreq(BB);
+  auto PredBBFreq = BFI->getBlockFreq(PredBB);
----------------
change the name to OrigBBFreq

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1578
@@ +1577,3 @@
+  // distributed to CFG nodes. In this case we just set the new frequency of
+  // BB to zero.
+  uint64_t NewBBFreq = (PredBBFreq <= BBFreq)
----------------
Is this still an issue that needs the workaround here?

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1596
@@ +1595,3 @@
+               "The weight on BB->SuccBB is not coherent with it in BPI");
+        if (BB2SuccBBFreq.getFrequency() > 0) {
+          DEBUG(dbgs() << "  Update the weight on the edge from "
----------------
Why this condition?

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1599
@@ +1598,3 @@
+                       << BB->getName() << " to " << SuccBB->getName() << "\n");
+          if (PredBBFreq <= BB2SuccBBFreq) {
+            // Here we record the index of SuccBB in successors of BB and
----------------
It is unclear how can this happen.

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1614
@@ +1613,3 @@
+    if (SuccIndexToUpdate >= 0) {
+      // We need to update the weights of BB's successors.
+      // Instead of scaling the weight of BB->SuccBB by 1 - Freq(PredBB->BB) /
----------------
The scaling code looks unnecessarily complicated. How about this:

a) Pre-normalization 
  1) first sum up all the weights before the update
  2) if the sum is too small, scale it up to a fixed val such as ScaledNumber<64>(1,20) 
  3) compute the scale factor and rescale the weights

b) update BB->SuccBB with (1- Freq(Pred->BB)/Freq(BB->SuccBB))

c) post-normalization: scale all weights such that the weight sum is 2^20

Some helper routine can be defined.






http://reviews.llvm.org/D10979