[PATCH] D92668: [SLP]Merge reorder and reuse shuffles.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D92668#2473257 <https://reviews.llvm.org/D92668#2473257>, @rupprecht wrote:

> I think I'm seeing a miscompile as a result of the patch. The issue is not the transformation itself, but a dependent instruction is reading the wrong value.
>
> Before this patch, we have this:
>
>     %171 = load <2 x i32>, <2 x i32>* %114, align 8
>     %reorder_shuffle = shufflevector <2 x i32> %171, <2 x i32> undef, <2 x i32> <i32 1, i32 0>
>     %shuffle = shufflevector <2 x i32> %reorder_shuffle, <2 x i32> undef, <4 x i32> <i32 0, i32 1, i32 0, i32 1>
>   ...
>     %186 = add nsw <2 x i32> %reorder_shuffle, <i32 -1, i32 -1>
>
> After this patch the same snippet looks like:
>
>     %171 = load <2 x i32>, <2 x i32>* %114, align 8
>     %shuffle = shufflevector <2 x i32> %171, <2 x i32> undef, <4 x i32> <i32 1, i32 0, i32 1, i32 0>
>   ...
>     %186 = add nsw <2 x i32> %171, <i32 -1, i32 -1>
>
> IIUC, if we suppose `%171` loads the values `(a, b)`, then:
>
> - In the original version, `%reorder_shuffle` is `(b, a)`, so `%186` is `(b - 1, a - 1)`
> - In the modified version, `%186` reads directly from `%171`, so the result is `(a - 1, b - 1)`, which is swapped
>
> Do you know what the issue might be? I'm trying to reduce the test case so I can share more details.

Hi, thanks for report, looks like you're right. I'll fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92668