[llvm-commits] [llvm] r142751 - in /llvm/trunk: include/llvm/Support/BranchProbability.h lib/Analysis/BranchProbabilityInfo.cpp lib/Support/BranchProbability.cpp unittests/Support/BlockFrequencyTest.cpp

[Chandler Carruth]

After some discussion on IRC...

On Sun, Oct 23, 2011 at 4:19 AM, Benjamin Kramer
<benny.kra at googlemail.com>wrote:

> +  int64_t compare(BranchProbability RHS) const {
> +    return (uint64_t)N * RHS.D - (uint64_t)D * RHS.N;
> +  }
>

It would be good to comment here that the result of the LHS of the subtract
can't be larger than INT64_MAX without the RHS being large enough to reduce
it because N <= D. It might also be nice to just assert that fact.


> --- llvm/trunk/unittests/Support/BlockFrequencyTest.cpp (original)
> +++ llvm/trunk/unittests/Support/BlockFrequencyTest.cpp Sun Oct 23 06:19:14
> 2011
> @@ -53,4 +53,24 @@
>   EXPECT_EQ(Freq.getFrequency(), UINT64_MAX);
>  }
>
> +TEST(BlockFrequencyTest, ProbabilityCompare) {
> +  BranchProbability A(4, 5);
> +  BranchProbability B(4U << 29, 5U << 29);
>

Maybe add a test case that compares B against a RHS with a similarly large
denominator value to ensure that overflow of INT64_MAX is indeed brought
back down in the computation? In particular the interesting one to my mind
is (UINT32_MAX, UINT32_MAX) vs. (1, UINT32_MAX).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111023/fdca861e/attachment.html>