[llvm] r215343 - In LVI(Lazy Value Info), originally value on a BB can only be caculated once,

[Pete Cooper]

> On Nov 20, 2014, at 6:50 PM, Hans Wennborg <hans at chromium.org> wrote:
> 
> On Thu, Nov 20, 2014 at 4:41 PM, Hans Wennborg <hans at chromium.org <mailto:hans at chromium.org>> wrote:
>> On Thu, Nov 20, 2014 at 4:39 PM, Pete Cooper <peter_cooper at apple.com> wrote:
>>>> On Nov 20, 2014, at 4:27 PM, Hans Wennborg <hans at chromium.org> wrote:
>>>> On Thu, Nov 20, 2014 at 11:27 AM, Pete Cooper <peter_cooper at apple.com> wrote:
>>>>> I came across this independently a few days ago.  My example was similar.  I
>>>>> had
>>>>> 
>>>>> if (x == 0) {
>>>>> if (…) {  … } else { … }
>>>>> ++x
>>>>> }
>>>> 
>>>> I'm still not very familiar with this analysis and its interaction
>>>> with jump threading. Could you turn your example into a compilable
>>>> function that I can experiment with?
>>> I was looking at correlated-value-propagation actually, but I guess this would apply to jump threading too.
>>> 
>>> I’ll get you a testcase shortly, and a real patch.
>> 
>> Thanks!
>> 
>>>> I applied your patch, but it seems to be causing miscompiles, as one
>>>> of the programs got into an infinite loop and consumed all my memory.
>>> Damn.  When I thought about the algorithm I convinced myself that it would terminate as you’ll always eventually chase the BBs up to a loop preheader or the function entry BB.  Perhaps I got that wrong.
>> 
>> It wasn't clang that inflooped, it was a program compiled by clang.
> 
> I stepped through the LazyValueInfo code to understand it better, and
> your patch seems perfectly reasonable.
Cool.  Thanks!

I was just writing the following, so if anyone is interested, this is how the algorithm works after the attached patch on the attached @test2.

We work from %merge up to %bb.

Starting at with %inner_false to %merge:
- visit %inner_false, not all predecessors are visited so stay undefined
- visit %bb_true, not all predecessors are visited so stay undefined
- visit %bb.  This defines %tmp35 and returns a constantrange > 0
- revisit %bb_true.  It gets ‘constantrange > 0’ from %bb
- revisit %inner_false.  It gets ‘constantrange > 0’ from %bb_true.

Now with %inner_true to %merge
- visit %inner_false.  It gets ‘constantrange > 0’ from %bb_true.

We’ve now visited all predecessors of %merge and got the same result from both, so set the value to false on the call.

> 
> But something is going wrong. I'm attaching a reduction of the problem
> I saw. With your patch applied, using "clang -O2 /tmp/a.cc <http://a.cc/>", the
> attached program goes into an infinite loop.
> 
Thanks!  I’ll take a look.

Cheers,
Pete


> - Hans
> <a.cc>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141120/8f35ccfa/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvi.diff
Type: application/octet-stream
Size: 2678 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141120/8f35ccfa/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141120/8f35ccfa/attachment-0001.html>