[llvm-commits] ScalarEvolution fix for treeadd

[Nick Lewycky]

Dan Gohman wrote:
>>>Would it be too intrusive to ask ScalarEvolution to use a
>>>PostDominanceFrontier for this?
>>
>>No, that's what I implemented at first. Unfortunately, it doesn't cover
>>all the possible cases (specifically, it broke 2007-01-06-TripCount.ll).
>>
>>In 2007-01-06-TripCount.ll there's two "loops" sharing one loop header.
>>The first runs from header -> exit -> A -> header and the other is
>>header -> B -> A -> header. I was testing exit postdom header, which it
>>does, but that didn't catch this case where the transform is still
>>unsafe.
>>
>>If you think I merely had my test wrong, please let me know what you
>>think it ought to be and I'll implement it and see.
> 
> I do think you merely had the wrong test. The post-dominance frontiers
> are needed here.
> 
> Running -analyze -postdomfrontier on 2007-01-06-TripCount.ll gives this:
> 
> Printing analysis 'Post-Dominance Frontier Construction' for function 'test':
>   DomFrontier for BB %bb is:     %bb2 %cond_next
>   DomFrontier for BB %bb2 is:    %bb2 %cond_next
>   DomFrontier for BB %cond_true is:      %bb2
>   DomFrontier for BB %cond_next is:      %cond_next
>   DomFrontier for BB %bb6 is:
>   DomFrontier for BB %return is:
>   DomFrontier for BB %entry is:
> 
> It looks like %bb2 is the loop header, and %cond_next is the block that
> contains the exit branch. The frontier sets for these two blocks are
> different, so they're not control-equivalent, and that disqualifies the
> loop for what ScalarEvolution is doing here.

Cool! I've implemented that (patch attached) and it works for both
treeadd and TripCount.

Here's the numbers:

  Original:

  predicted:     4874  (21%)
  not predicted: 18361 (79%)

  PDF test only:

  predicted:     4865  (21%)
  not predicted: 18372 (79%)

There was one other consequence of that change. PDF entries only exist
for nodes that postdominate the exit. One could have a countable loop
that preceeds an infinite loop, and it would no longer be analyzed.

I was going to just have it refuse to analyze that case, but it occured
to me that event loops and such work this way, and it could cause a real
problem for real code. So I thought I'd try keeping it.

  Both tests:

  predicted:     4881  (21%)
  not predicted: 18354 (79%)

I'm not sure why, but it doesn't seem as promising as the previous test
I had. Could it be that when comparing the PDF I should ignore blocks
that are not part of the loop?

Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: scev.patch
Type: text/x-diff
Size: 3812 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20070725/0c70dfff/attachment.patch>