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

[Cong Hou]

congh marked an inline comment as done.

================
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)
----------------
davidxl wrote:
> Is this still an issue that needs the workaround here?
Yes. The issue emerges after jump-threading + loop-rotation + jump-threading will lead to this.

================
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 "
----------------
davidxl wrote:
> Why this condition?
I forget to add a comment when I did this. But I think this condition can be removed now.

================
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
----------------
davidxl wrote:
> It is unclear how can this happen.
Consider the following CFG and assume the edge frequency is 100 for each edge:


```
A   B
 \ /
  C
 / \
D   E
```

In some pass C may be split into two blocks:


```
 A   B
 |   |
 C1 C2
 | X |
 D   E
```

Then edges from C1/C2 have frequency 50. Now suppose in jump-threading pass we thread B->E across C2 (C2->D is impossible at runtime), then B has frequency 100 but C2->E has frequency 50. This is when this condition is not met.

================
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) /
----------------
davidxl wrote:
> 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.
> 
> 
> 
> 
This method looks good. I have two questions:

1. How to tell if the sum is too small? Can we just normalize all weights such that the sum of them is 2^20?
2. Is c) necessary?


http://reviews.llvm.org/D10979