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

[Chris Lattner]

> On Dec 8, 2014, at 8:52 AM, Tom Stellard <tom at stellard.net> wrote:
> 
> 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.

LGTM, though I’m not the best to review this.   Maybe we should get a profiling/coverage code owner?

-Chris


> 
> -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
>>