[PATCH] D132634: [AArch64] Add index check before lowerInterleavedStore() uses ShuffleVectorInst's mask

[Peter Rong via Phabricator via llvm-commits]

Peter added a comment.

In D132634#3825541 <https://reviews.llvm.org/D132634#3825541>, @dmgreen wrote:

>> I am confused why index is calculated as IdxJ * Factor + IdxI, isn't IdxJ = StoreCount * LaneLen * Factor + j; already multiplied by Factor? Why do it twice?
>
> Yeah - I think that logic is just wrong, so it is going out of range. Mask elements are almost never undef, and the combination of an undef first elements and wider-than-legal vectors is rare enough that no-one has noticed it is incorrect before. It would be good to support it though, if we can get the logic correct.
>
> The `Mask[IdxJ * Factor + IdxI]` index into Mask should never be out-of-range. It needs to find the first element that isn't undef (-1) in that store-chunk, for elements in that Factor, and use that to find the equivalent StartMask. I think it should just be `StoreCount * LaneLen * Factor + j * Factor + i`, but I may have the LaneLen and Factor backwards. There is a comment that suggests that at least one of the elements shouldn't be undef, but it appears from your test if it is getting there then that might not be true.

I am more confident to fix it once you feel like it's a bug too. I just submitted latest change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132634