[PATCH] D14670: Fix bug 25440: GVN assertion after coercing loads

[via llvm-commits]

Hmmm. That's interesting.
I will dig more and try to find out why.


> Err, then the algorithm is broken somewhere.
>
> This is definitely not the right fix from what i've seen so far.
> Can you see what decides to insert this?
>
> I suspect, based on your failure mode, that the leader table is not
> ordered
> locally by dominance, only by bb, and so when it calls findLeader, it
> grabs
> the first thing it finds, and does a broken thing.
>
> It would only work in the processInstruction case because
> processInstrucion
> goes in dominance order.
>
> If so, that would be bad (and hard to fix well without serious surgery),
> and we should probably declare it broken, revert your set of patches, and
> just abort PRE  (IE the other option i suggested where we set
> success=false
> and bailing)
>
>
>
> On Fri, Nov 13, 2015 at 5:30 PM, Weiming Zhao <weimingz at codeaurora.org>
> wrote:
>
>> weimingz added a comment.
>>
>> This patch adds a reduced test from SPASS.
>>
>> we need the check:
>>
>>   if (Num >= NextNum)
>>           addToLeaderTable(Num, I, I->getParent());
>>
>> Otherwise, BB %wile.end (see line 91) will become :
>>
>> while.end:                                        ; preds =
>> %while.cond.16
>>
>>   %add.ptr = getelementptr inbounds i8, i8* %v2, i32 undef
>>   store i8* %add.ptr, i8** @dfg_text, align 4
>>   %sub.ptr.sub26 = sub i32 0, %0  ==> use here
>>   %0 = ptrtoint i8* %add.ptr to i32  ==> def here
>>   switch i32 undef, label %sw.default [
>>     i32 65, label %while.bodythread-pre-split
>>
>>
>> http://reviews.llvm.org/D14670
>>
>>
>>
>>
>