[PATCH] D132055: [LoopInterchange][PR57148] Ensure LCSSA form after loop interchnange

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added a comment.

In D132055#3814831 <https://reviews.llvm.org/D132055#3814831>, @uabelho wrote:

> Hello @congzhe ,
>
> I think I'm seeing a miscompile with this patch:
>
>   opt "-passes=function(loop(loop-interchange))" bbi-74005_x86.ll -S -o -
>
> Now I don't know how this is supposed to work but it looks to me that in the input code we read
>
>   %arrayidx14.promoted.i = load i32, ptr %arrayidx14.i, align 1
>
> then do 512 rounds of the inner loop and add the read elements, and then we store what we have so far:
>
>   %18 = tail call i32 @llvm.vector.reduce.add.v4i32(<4 x i32> %16)
>   store i32 %18, ptr %arrayidx14.i, align 1
>
> But after loop-interchange we load, then add, but then we dont do the store, but do the load and execute the inner loop again? So it looks like we throw away the calculated addition and just read the same values again?
> Or am I missing something here?
>
> F24704792: bbi-74005_x86.ll <https://reviews.llvm.org/F24704792>



In D132055#3817047 <https://reviews.llvm.org/D132055#3817047>, @congzhe wrote:

> In D132055#3814831 <https://reviews.llvm.org/D132055#3814831>, @uabelho wrote:
>
>> Hello @congzhe ,
>>
>> I think I'm seeing a miscompile with this patch:
>>
>>   opt "-passes=function(loop(loop-interchange))" bbi-74005_x86.ll -S -o -
>>
>> Now I don't know how this is supposed to work but it looks to me that in the input code we read
>>
>>   %arrayidx14.promoted.i = load i32, ptr %arrayidx14.i, align 1
>>
>> then do 512 rounds of the inner loop and add the read elements, and then we store what we have so far:
>>
>>   %18 = tail call i32 @llvm.vector.reduce.add.v4i32(<4 x i32> %16)
>>   store i32 %18, ptr %arrayidx14.i, align 1
>>
>> But after loop-interchange we load, then add, but then we dont do the store, but do the load and execute the inner loop again? So it looks like we throw away the calculated addition and just read the same values again?
>> Or am I missing something here?
>>
>> F24704792: bbi-74005_x86.ll <https://reviews.llvm.org/F24704792>
>
> Hi Mikael, thanks for finding it out, I'll take a look at this issue,

Posted D134930 <https://reviews.llvm.org/D134930> to fix it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132055/new/

https://reviews.llvm.org/D132055