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

Cong Hou via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 10 14:35:26 PST 2015


congh added inline comments.

================
Comment at: lib/CodeGen/IfConversion.cpp:1263
@@ -1262,2 +1262,3 @@
     BBI.BB->addSuccessor(CvtBBI->FalseBB, NewFalse);
+    BBI.BB->normalizeSuccProbs();
   }
----------------
congh wrote:
> 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.
Unfortunately the validation fails several times here, which means sum != 1 for BBI.BB's successors. However, they are going to be normalized later in RemoveExtraEdges(BBI) so it seems the normalization is still redundant.

The if converter allows many invalid temporary states which are later fixed by some clean-up functions. This makes updating and validating probabilities quite difficult.


http://reviews.llvm.org/D15259





More information about the llvm-commits mailing list