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

[Jonathan Roelofs via llvm-commits]

On 12/8/17 5:03 PM, Duncan P. N. Exon Smith wrote:
>
>
>> On Dec 8, 2017, at 15:38, Jonathan Roelofs <jonathan at codesourcery.com 
>> <mailto:jonathan at codesourcery.com>> wrote:
>>
>>
>>
>> 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?
>
> It would have helped if I'd linked to the review in the commit message:
> https://reviews.llvm.org/D40891
>
> - `getEdgeProbability(BasicBlock&, edge_iterator)` returns the 
> probability of a single edge.
> - `getEdgeProbability(BasicBlock&, BasicBlock&)` returns the sum of 
> the edges between blocks.
>

ahhh, thanks!


Jon

>
>>
>>
>> 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 <mailto:llvm-commits at lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>> --
>> Jon Roelofs
>> jonathan at codesourcery.com <mailto:jonathan at codesourcery.com>
>> CodeSourcery / Mentor Embedded / Siemens
>

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171208/5cabba56/attachment.html>