[llvm] r320208 - Revert part of "Cleanup some GraphTraits iteration code"

[Jonathan Roelofs via llvm-commits]

On 12/8/17 3:42 PM, Duncan P. N. Exon Smith via llvm-commits wrote:
> Author: dexonsmith
> Date: Fri Dec  8 14:42:43 2017
> New Revision: 320208
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=320208&view=rev
> Log:
> Revert part of "Cleanup some GraphTraits iteration code"
> 
> This reverts part of r300656, which caused a regression in
> propagateMassToSuccessors by counting edges n^2 times, where n is the
> number of edges from the source basic block to the same successor basic
> block. The result was both incorrect and very slow to compute for large
> values of n (e.g. switches with multiple cases that go to the same basic
> block).
> 
> Patch by Andrew Scheidecker!
> 
> Added:
>      llvm/trunk/test/Analysis/BlockFrequencyInfo/redundant_edges.ll
> Modified:
>      llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h
> 
> Modified: llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h?rev=320208&r1=320207&r2=320208&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h (original)
> +++ llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h Fri Dec  8 14:42:43 2017
> @@ -1314,9 +1314,12 @@ BlockFrequencyInfoImpl<BT>::propagateMas
>         return false;
>     } else {
>       const BlockT *BB = getBlock(Node);
> -    for (const auto Succ : children<const BlockT *>(BB))
> -      if (!addToDist(Dist, OuterLoop, Node, getNode(Succ),
> -                     getWeightFromBranchProb(BPI->getEdgeProbability(BB, Succ))))
> +    for (auto SI = GraphTraits<const BlockT *>::child_begin(BB),
> +              SE = GraphTraits<const BlockT *>::child_end(BB);
> +         SI != SE; ++SI)
> +      if (!addToDist(
> +              Dist, OuterLoop, Node, getNode(*SI),
> +              getWeightFromBranchProb(BPI->getEdgeProbability(BB, SI))))


Where was the bug here, does children() recompute more or something?


Jon

>           // Irreducible backedge.
>           return false;
>     }
> 
> Added: llvm/trunk/test/Analysis/BlockFrequencyInfo/redundant_edges.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/BlockFrequencyInfo/redundant_edges.ll?rev=320208&view=auto
> ==============================================================================
> --- llvm/trunk/test/Analysis/BlockFrequencyInfo/redundant_edges.ll (added)
> +++ llvm/trunk/test/Analysis/BlockFrequencyInfo/redundant_edges.ll Fri Dec  8 14:42:43 2017
> @@ -0,0 +1,22 @@
> +; RUN: opt < %s -analyze -block-freq | FileCheck %s
> +; RUN: opt < %s -analyze -lazy-block-freq | FileCheck %s
> +; RUN: opt < %s -passes='print<block-freq>' -disable-output 2>&1 | FileCheck %s
> +
> +define void @test1() {
> +; CHECK-LABEL: Printing analysis {{.*}} for function 'test1':
> +; CHECK-NEXT: block-frequency-info: test1
> +; CHECK-NEXT: entry: float = 1.0, int = [[ENTRY:[0-9]+]]
> +entry:
> +  br label %loop
> +
> +; CHECK-NEXT: loop: float = 32.0
> +loop:
> +  switch i32 undef, label %loop [
> +    i32 0, label %return
> +    i32 1, label %return
> +  ]
> +
> +; CHECK-NEXT: return: float = 1.0
> +return:
> +  ret void
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded / Siemens