<div dir="ltr">I don't think this had anything specifically to do with PPC. Saturating at 1 causes the fundamental mathematical model of block frequencies to break down and do all kinds of strange things (including triggering assertions).</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Jun 22, 2013 at 9:51 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">----- Original Message -----<br>
> Author: d0k<br>
> Date: Fri Jun 21 15:20:27 2013<br>
> New Revision: 184590<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=184590&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=184590&view=rev</a><br>
> Log:<br>
> Revert "BlockFrequency: Saturate at 1 instead of 0 when multiplying a<br>
> frequency with a branch probability."<br>
><br>
> This reverts commit r184584. Breaks PPC selfhost.<br>
<br>
</div>That seems odd. Which build was this? Do you have any additional details?<br>
<br>
Thanks again,<br>
Hal<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> Removed:<br>
> llvm/trunk/test/Analysis/BlockFrequencyInfo/singularity.ll<br>
> Modified:<br>
> llvm/trunk/include/llvm/Support/BlockFrequency.h<br>
> llvm/trunk/lib/Support/BlockFrequency.cpp<br>
> llvm/trunk/unittests/Support/BlockFrequencyTest.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/Support/BlockFrequency.h<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/BlockFrequency.h?rev=184590&r1=184589&r2=184590&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/BlockFrequency.h?rev=184590&r1=184589&r2=184590&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/Support/BlockFrequency.h (original)<br>
> +++ llvm/trunk/include/llvm/Support/BlockFrequency.h Fri Jun 21<br>
> 15:20:27 2013<br>
> @@ -30,20 +30,12 @@ class BlockFrequency {<br>
> public:<br>
> BlockFrequency(uint64_t Freq = 0) : Frequency(Freq) { }<br>
><br>
> - /// \brief Returns the frequency of the entry block of the<br>
> function.<br>
> static uint64_t getEntryFrequency() { return ENTRY_FREQ; }<br>
> -<br>
> - /// \brief Returns the frequency as a fixpoint number scaled by<br>
> the entry<br>
> - /// frequency.<br>
> uint64_t getFrequency() const { return Frequency; }<br>
><br>
> - /// \brief Multiplies with a branch probability. The computation<br>
> will never<br>
> - /// overflow. If the result is equal to zero but the input wasn't<br>
> this method<br>
> - /// will return a frequency of one.<br>
> BlockFrequency &operator*=(const BranchProbability &Prob);<br>
> const BlockFrequency operator*(const BranchProbability &Prob)<br>
> const;<br>
><br>
> - /// \brief Adds another block frequency using saturating<br>
> arithmetic.<br>
> BlockFrequency &operator+=(const BlockFrequency &Freq);<br>
> const BlockFrequency operator+(const BlockFrequency &Freq) const;<br>
><br>
><br>
> Modified: llvm/trunk/lib/Support/BlockFrequency.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/BlockFrequency.cpp?rev=184590&r1=184589&r2=184590&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/BlockFrequency.cpp?rev=184590&r1=184589&r2=184590&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Support/BlockFrequency.cpp (original)<br>
> +++ llvm/trunk/lib/Support/BlockFrequency.cpp Fri Jun 21 15:20:27<br>
> 2013<br>
> @@ -65,9 +65,6 @@ uint64_t div96bit(uint64_t W[2], uint32_<br>
><br>
><br>
> BlockFrequency &BlockFrequency::operator*=(const BranchProbability<br>
> &Prob) {<br>
> - if (Frequency == 0)<br>
> - return *this;<br>
> -<br>
> uint32_t n = Prob.getNumerator();<br>
> uint32_t d = Prob.getDenominator();<br>
><br>
> @@ -87,15 +84,10 @@ BlockFrequency &BlockFrequency::operator<br>
> // 64-bit.<br>
> mult96bit(Frequency, n, W);<br>
> Frequency = div96bit(W, d);<br>
> - } else {<br>
> - // Fast case.<br>
> - Frequency = mulRes / d;<br>
> + return *this;<br>
> }<br>
><br>
> - // Limit the result to 1; 0 is a sentinel value. This keeps<br>
> BlockFrequencyInfo<br>
> - // from getting stuck at zero frequencies just because a value<br>
> became too<br>
> - // small to be represented as a BlockFrequency.<br>
> - Frequency = (n == 0 || Frequency != 0) ? Frequency : 1;<br>
> + Frequency = mulRes / d;<br>
> return *this;<br>
> }<br>
><br>
><br>
> Removed: llvm/trunk/test/Analysis/BlockFrequencyInfo/singularity.ll<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/BlockFrequencyInfo/singularity.ll?rev=184589&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/BlockFrequencyInfo/singularity.ll?rev=184589&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/Analysis/BlockFrequencyInfo/singularity.ll<br>
> (original)<br>
> +++ llvm/trunk/test/Analysis/BlockFrequencyInfo/singularity.ll<br>
> (removed)<br>
> @@ -1,65 +0,0 @@<br>
> -; RUN: opt < %s -analyze -block-freq | FileCheck %s<br>
> -; PR16402<br>
> -<br>
> -define void @test1(i32 %n) nounwind {<br>
> -entry:<br>
> - %call = tail call i32* @cond() nounwind<br>
> - %tobool = icmp eq i32* %call, null<br>
> - br i1 %tobool, label %land.lhs.true, label %if.end<br>
> -<br>
> -land.lhs.true: ; preds = %entry<br>
> - %call1 = tail call i32* @cond() nounwind<br>
> - %tobool2 = icmp eq i32* %call1, null<br>
> - br i1 %tobool2, label %land.lhs.true3, label %if.end<br>
> -<br>
> -land.lhs.true3: ; preds =<br>
> %land.lhs.true<br>
> - %call4 = tail call i32* @cond() nounwind<br>
> - %tobool5 = icmp eq i32* %call4, null<br>
> - br i1 %tobool5, label %land.lhs.true6, label %if.end<br>
> -<br>
> -land.lhs.true6: ; preds =<br>
> %land.lhs.true3<br>
> - %call7 = tail call i32* @cond() nounwind<br>
> - %tobool8 = icmp eq i32* %call7, null<br>
> - br i1 %tobool8, label %land.lhs.true9, label %if.end<br>
> -<br>
> -land.lhs.true9: ; preds =<br>
> %land.lhs.true6<br>
> - %call10 = tail call i32* @cond() nounwind<br>
> - %tobool11 = icmp eq i32* %call10, null<br>
> - br i1 %tobool11, label %land.lhs.true12, label %if.end<br>
> -<br>
> -land.lhs.true12: ; preds =<br>
> %land.lhs.true9<br>
> - %call13 = tail call i32* @cond() nounwind<br>
> - %tobool14 = icmp eq i32* %call13, null<br>
> - br i1 %tobool14, label %land.lhs.true15, label %if.end<br>
> -<br>
> -land.lhs.true15: ; preds =<br>
> %land.lhs.true12<br>
> - %call16 = tail call i32* @cond() nounwind<br>
> - %tobool17 = icmp eq i32* %call16, null<br>
> - br i1 %tobool17, label %for.cond.preheader, label %if.end<br>
> -<br>
> -for.cond.preheader: ; preds =<br>
> %land.lhs.true15<br>
> - %cmp21 = icmp eq i32 %n, 0<br>
> - br i1 %cmp21, label %for.end, label %for.body<br>
> -<br>
> -for.body: ; preds =<br>
> %for.cond.preheader, %for.body<br>
> - %i.022 = phi i32 [ %inc, %for.body ], [ 0, %for.cond.preheader ]<br>
> - %call18 = tail call i32 @call() nounwind<br>
> - %inc = add nsw i32 %i.022, 1<br>
> - %cmp = icmp eq i32 %inc, %n<br>
> - br i1 %cmp, label %for.end, label %for.body<br>
> -<br>
> -for.end: ; preds =<br>
> %for.body, %for.cond.preheader<br>
> - %call19 = tail call i32* @cond() nounwind<br>
> - br label %if.end<br>
> -<br>
> -if.end: ; preds =<br>
> %land.lhs.true15, %land.lhs.true12, %land.lhs.true9,<br>
> %land.lhs.true6, %land.lhs.true3, %land.lhs.true, %entry, %for.end<br>
> - ret void<br>
> -<br>
> -; CHECK: entry = 1024<br>
> -; CHECK-NOT: for.body = 0<br>
> -; CHECK-NOT: for.end = 0<br>
> -}<br>
> -<br>
> -declare i32* @cond() nounwind<br>
> -<br>
> -declare i32 @call() nounwind<br>
><br>
> Modified: llvm/trunk/unittests/Support/BlockFrequencyTest.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/BlockFrequencyTest.cpp?rev=184590&r1=184589&r2=184590&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/BlockFrequencyTest.cpp?rev=184590&r1=184589&r2=184590&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/unittests/Support/BlockFrequencyTest.cpp (original)<br>
> +++ llvm/trunk/unittests/Support/BlockFrequencyTest.cpp Fri Jun 21<br>
> 15:20:27 2013<br>
> @@ -8,22 +8,11 @@ using namespace llvm;<br>
><br>
> namespace {<br>
><br>
> -TEST(BlockFrequencyTest, ZeroToZero) {<br>
> - BlockFrequency Freq(0);<br>
> - BranchProbability Prob(UINT32_MAX - 1, UINT32_MAX);<br>
> - Freq *= Prob;<br>
> - EXPECT_EQ(Freq.getFrequency(), 0u);<br>
> -<br>
> - Freq = 1;<br>
> - Freq *= BranchProbability::getZero();<br>
> - EXPECT_EQ(Freq.getFrequency(), 0u);<br>
> -}<br>
> -<br>
> TEST(BlockFrequencyTest, OneToZero) {<br>
> BlockFrequency Freq(1);<br>
> BranchProbability Prob(UINT32_MAX - 1, UINT32_MAX);<br>
> Freq *= Prob;<br>
> - EXPECT_EQ(Freq.getFrequency(), 1u);<br>
> + EXPECT_EQ(Freq.getFrequency(), 0u);<br>
> }<br>
><br>
> TEST(BlockFrequencyTest, OneToOne) {<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>