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

Duncan Exon Smith dexonsmith at apple.com
Sun Nov 30 09:54:12 PST 2014


> 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>




More information about the llvm-commits mailing list