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

[Duncan P. N. Exon Smith]

Tom,

Is this okay for 3.5?

> 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