[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;
----------------
congh wrote:
> davidxl wrote:
> > No underflow check here?
> -= below is using the new operator defined above, so underflow is checked.
ok

================
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);
+
----------------
congh wrote:
> davidxl wrote:
> > Is this interface available in  trunk?
> Not yet :( Still waiting for the review of that patch.
Since the interfaces are under large rework,  I suggest simply extract extract the minimal necessary normalization code from that patch here (and abandon that one). Is it doable?

================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:1616
@@ +1615,3 @@
+    BPI->setEdgeWeight(BB, I, Weights[I]);
+
+  if (Weights.size() >= 2) {
----------------
congh wrote:
> davidxl wrote:
> > BPI for PredBB also needs to be updated for completeness -- as its successor BB is now NewBB
> The old PredBB's successor BB is replaced by NewBB in the same position, so BPI doesn't have to be updated. (In BPI, the successor weights are stores in a map with the index of the successor as the key. The NewBB is set as a successor using the following code:
> 
>     if (PredTerm->getSuccessor(i) == BB) {
>       BB->removePredecessor(PredBB, true);
>       PredTerm->setSuccessor(i, NewBB);
>     }
> 
> So the position of this new successor is not changed.)
ok then. 


http://reviews.llvm.org/D10979