[PATCH] D15259: Normalize MBB's successors' probabilities in several locations.

[Cong Hou via llvm-commits]

congh added a comment.

In http://reviews.llvm.org/D15259#305550, @davidxl wrote:

> Are there any test case changes?


There is no test changes.


================
Comment at: lib/CodeGen/IfConversion.cpp:1263
@@ -1262,2 +1262,3 @@
     BBI.BB->addSuccessor(CvtBBI->FalseBB, NewFalse);
+    BBI.BB->normalizeSuccProbs();
   }
----------------
davidxl wrote:
> Add a brief comment about why normalization is needed (i.e., why the input BPs do not sum up to 1)?
I tried removing this normalization and there isn't new test failures. So I think this is redundant. I added a runtime check instead, which is a new interface of MBB that checks if all successors' probabilities sum up to one and produces assertion failure if not.

================
Comment at: lib/CodeGen/IfConversion.cpp:1719
@@ -1717,3 +1718,3 @@
       // If the edge from ToBBI.BB to Succ already exists, update the
       // probability of this edge by adding NewWeight to it. An example is shown
       // below, in which A is ToBBI.BB and B is FromBBI.BB. In this case we
----------------
davidxl wrote:
> The comment needs to be fixed (probably in a different patch).
As this is a very small change, I think it is ok to do it in this patch. Done.

================
Comment at: lib/CodeGen/IfConversion.cpp:1753
@@ -1751,1 +1752,3 @@
 
+  ToBBI.BB->normalizeSuccProbs();
+  FromBBI.BB->normalizeSuccProbs();
----------------
davidxl wrote:
> It is unclear why this is needed.
This is needed as here we have adjusted its successors's probabilities which cannot guarantee the sum is one.

================
Comment at: lib/CodeGen/IfConversion.cpp:1754
@@ +1753,3 @@
+  ToBBI.BB->normalizeSuccProbs();
+  FromBBI.BB->normalizeSuccProbs();
+
----------------
davidxl wrote:
> Will its successor edge be discarded later?
I checked all uses of this function and found after each call FromBBI.BB's successors are all normalized somehow. I commented this line and didn't see any new failures in my test. So we don't have to normalize FromBBI.BB's successors.


http://reviews.llvm.org/D15259