[llvm] r184590 - Revert "BlockFrequency: Saturate at 1 instead of 0 when multiplying a frequency with a branch probability."
Benjamin Kramer
benny.kra at gmail.com
Sun Jun 23 01:58:20 PDT 2013
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.
- 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130623/f25da5d4/attachment.html>
More information about the llvm-commits
mailing list