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

via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 7 13:42:34 PST 2015


Thanks.
I will do the former one.

> This patch doesn't make sense.
>
>
> Either  you need to have it assign the GV and fix the leader tables so
> that
> the lookup at this point succeeds (which is ideal), or VN.lookup(op) ==
> null, it should set success=false and break.
>
>
> The former is *greatly* preferable to the latter.
>
> What you've done here makes no sense, because findLeader(anything, <new
> value number>) will always return nothing, since addLeaderToTable has not
> been called on the new value number.
> All you are doing in that case is wasting time calling findLeader.
>
>
>
> On Fri, Nov 6, 2015 at 11:22 PM, Weiming Zhao via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> weimingz created this revision.
>> weimingz added a subscriber: llvm-commits.
>>
>> when coercing loads, it inserts some instructions, which have no GV
>> assigned.
>>
>> https://llvm.org/bugs/show_bug.cgi?id=25440
>>
>>
>> http://reviews.llvm.org/D14479
>>
>> Files:
>>   lib/Transforms/Scalar/GVN.cpp
>>
>> Index: lib/Transforms/Scalar/GVN.cpp
>> ===================================================================
>> --- lib/Transforms/Scalar/GVN.cpp
>> +++ lib/Transforms/Scalar/GVN.cpp
>> @@ -2529,7 +2529,7 @@
>>      if (isa<Argument>(Op) || isa<Constant>(Op) || isa<GlobalValue>(Op))
>>        continue;
>>
>> -    if (Value *V = findLeader(Pred, VN.lookup(Op))) {
>> +    if (Value *V = findLeader(Pred, VN.lookup_or_add(Op))) {
>>        Instr->setOperand(i, V);
>>      } else {
>>        success = false;
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>




More information about the llvm-commits mailing list