[llvm] r184590 - Revert "BlockFrequency: Saturate at 1 instead of 0 when multiplying a frequency with a branch probability."

Sun Jun 23 06:07:35 PDT 2013

----- Original Message -----
> 
> On 23.06.2013, at 07:50, Chandler Carruth < chandlerc at google.com >
> wrote:
> 
> 
> 
> 
> 
> 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).
> 
> 
> Right. The PPC self host bot just happened to be the first one that
> failed.

Thanks! So 1 is problematic and 0 is not?

 -Hal

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

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory