[llvm] r223500 - BFI: Saturate when combining edges to a successor

[Tom Stellard]

On Fri, Dec 05, 2014 at 06:36:41PM -0800, Duncan P. N. Exon Smith wrote:
> Tom,
> 
> Is this okay for 3.5?
> 

cc'ing Chris who is the default code owner.

-Tom

> > On 2014 Dec 5, at 11:13, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> > 
> > Author: dexonsmith
> > Date: Fri Dec  5 13:13:42 2014
> > New Revision: 223500
> > 
> > URL: http://llvm.org/viewvc/llvm-project?rev=223500&view=rev
> > Log:
> > BFI: Saturate when combining edges to a successor
> > 
> > When a loop gets bundled up, its outgoing edges are quite large, and can
> > just barely overflow 64-bits.  If one successor has multiple incoming
> > edges -- and that successor is getting all the incoming mass --
> > combining just its edges can overflow.  Handle that by saturating rather
> > than asserting.
> > 
> > This fixes PR21622.
> > 
> > Added:
> >    llvm/trunk/test/Analysis/BlockFrequencyInfo/extremely-likely-loop-successor.ll
> > Modified:
> >    llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp
> > 
> > Modified: llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp?rev=223500&r1=223499&r2=223500&view=diff
> > ==============================================================================
> > --- llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp (original)
> > +++ llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp Fri Dec  5 13:13:42 2014
> > @@ -14,6 +14,7 @@
> > #include "llvm/Analysis/BlockFrequencyInfoImpl.h"
> > #include "llvm/ADT/SCCIterator.h"
> > #include "llvm/Support/raw_ostream.h"
> > +#include <numeric>
> > 
> > using namespace llvm;
> > using namespace llvm::bfi_detail;
> > @@ -122,8 +123,12 @@ static void combineWeight(Weight &W, con
> >   }
> >   assert(W.Type == OtherW.Type);
> >   assert(W.TargetNode == OtherW.TargetNode);
> > -  assert(W.Amount < W.Amount + OtherW.Amount && "Unexpected overflow");
> > -  W.Amount += OtherW.Amount;
> > +  assert(OtherW.Amount && "Expected non-zero weight");
> > +  if (W.Amount > W.Amount + OtherW.Amount)
> > +    // Saturate on overflow.
> > +    W.Amount = UINT64_MAX;
> > +  else
> > +    W.Amount += OtherW.Amount;
> > }
> > static void combineWeightsBySorting(WeightList &Weights) {
> >   // Sort so edges to the same node are adjacent.
> > @@ -206,11 +211,19 @@ void Distribution::normalize() {
> >     Shift = 33 - countLeadingZeros(Total);
> > 
> >   // Early exit if nothing needs to be scaled.
> > -  if (!Shift)
> > +  if (!Shift) {
> > +    // If we didn't overflow then combineWeights() shouldn't have changed the
> > +    // sum of the weights, but let's double-check.
> > +    assert(Total == std::accumulate(Weights.begin(), Weights.end(), UINT64_C(0),
> > +                                    [](uint64_t Sum, const Weight &W) {
> > +                      return Sum + W.Amount;
> > +                    }) &&
> > +           "Expected total to be correct");
> >     return;
> > +  }
> > 
> >   // Recompute the total through accumulation (rather than shifting it) so that
> > -  // it's accurate after shifting.
> > +  // it's accurate after shifting and any changes combineWeights() made above.
> >   Total = 0;
> > 
> >   // Sum the weights to each node and shift right if necessary.
> > 
> > Added: llvm/trunk/test/Analysis/BlockFrequencyInfo/extremely-likely-loop-successor.ll
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/BlockFrequencyInfo/extremely-likely-loop-successor.ll?rev=223500&view=auto
> > ==============================================================================
> > --- llvm/trunk/test/Analysis/BlockFrequencyInfo/extremely-likely-loop-successor.ll (added)
> > +++ llvm/trunk/test/Analysis/BlockFrequencyInfo/extremely-likely-loop-successor.ll Fri Dec  5 13:13:42 2014
> > @@ -0,0 +1,40 @@
> > +; RUN: opt < %s -analyze -block-freq | FileCheck %s
> > +
> > +; PR21622: Check for a crasher when the sum of exits to the same successor of a
> > +; loop overflows.
> > +
> > +; CHECK-LABEL: Printing analysis {{.*}} for function 'extremely_likely_loop_successor':
> > +; CHECK-NEXT: block-frequency-info: extremely_likely_loop_successor
> > +define void @extremely_likely_loop_successor() {
> > +; CHECK-NEXT: entry: float = 1.0, int = [[ENTRY:[0-9]+]]
> > +entry:
> > +  br label %loop
> > +
> > +; CHECK-NEXT: loop: float = 1.0,
> > +loop:
> > +  %exit.1.cond = call i1 @foo()
> > +  br i1 %exit.1.cond, label %exit, label %loop.2, !prof !0
> > +
> > +; CHECK-NEXT: loop.2: float = 0.0000000
> > +loop.2:
> > +  %exit.2.cond = call i1 @foo()
> > +  br i1 %exit.2.cond, label %exit, label %loop.3, !prof !0
> > +
> > +; CHECK-NEXT: loop.3: float = 0.0000000
> > +loop.3:
> > +  %exit.3.cond = call i1 @foo()
> > +  br i1 %exit.3.cond, label %exit, label %loop.4, !prof !0
> > +
> > +; CHECK-NEXT: loop.4: float = 0.0,
> > +loop.4:
> > +  %exit.4.cond = call i1 @foo()
> > +  br i1 %exit.4.cond, label %exit, label %loop, !prof !0
> > +
> > +; CHECK-NEXT: exit: float = 1.0, int = [[ENTRY]]
> > +exit:
> > +  ret void
> > +}
> > +
> > +declare i1 @foo()
> > +
> > +!0 = metadata !{metadata !"branch_weights", i32 4294967295, i32 1}
> > 
> > 
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>