[PATCH] GVN: Don't crash on an unreachable predecessor

[Bruno Cardoso Lopes]

Agreed, LGTM too!

On Sun, Nov 30, 2014 at 3:54 PM, Duncan Exon Smith <dexonsmith at apple.com> wrote:
>
>> On Nov 29, 2014, at 8:11 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>
>> On 29 November 2014 at 10:14, Rafael Espíndola
>> <rafael.espindola at gmail.com> wrote:
>>> Unfortunately it would still crash on the original testcase :-(
>>>
>>> It now reduces to
>>>
>>> define i32 @f(i32 **%f) {
>>> bb0:
>>>  %bar = load i32** %f
>>>  br label %bb2
>>> bb1:
>>>  %zed = load i32** %f
>>>  br i1 false, label %bb2, label %bb1
>>> bb2:
>>>  %foo = phi i32* [ null, %bb0 ], [ %zed, %bb1 ]
>>>  %storemerge = load i32* %foo, align 4
>>>  ret i32 %storemerge
>>> }
>>>
>>> So the assert fails since bb1 has a predecessor: itself.
>>>
>>> That can be avoided with the attached patch (which would still need
>>> the stronger test added). It fixes the original test is the bug report
>>> and the clang-interpreter link where I first hit the crash.
>>>
>>> I am currently testing a full bootstrap.
>>
>> The 3 stage lto bootstrap finished fine.
>>
>> The attached patch includes the modified testcase. Is it OK?
>
> LGTM.
>
>> I am no
>> gvn expert either, but it looks like we are better with it than
>> without :-)
>
> Agreed!
>
>> Cheers,
>> Rafael
>> <t.patch>



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc