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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-shuffle-undef.ll:24
+  %B = urem <32 x i16> <i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1>, <i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1>
+  %S = shufflevector <32 x i16> %B, <32 x i16> %B, <32 x i32> undef
+  store <32 x i16> %S, <32 x i16>* %A2, align 64
----------------
Peter wrote:
> Peter wrote:
> > fhahn wrote:
> > > can the vectors be shortened here? Does it also crash with a `poison` mask?
> > From my experience, it seems only lengths of 32 will trigger the bug.
> > 
> > I just tested again. `undef` crashes both Debug and Release build. `poison` only crashes Debug build. This just adds the mystery for me...
> > 
> > I checked the failed testcases in Transform, it seems they only considered when part of the Mask is undef or poison, not the whole Mask.
> Ok I managed to find a testcase with only vector length of 16.
> 
> ```
> ; ModuleID = 'PoC.ll'
> source_filename = "M"
> 
> define void @f() {
> BB:
>   %A2 = alloca <16 x i32>, align 64
>   %B = urem <16 x i32> <i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1>, <i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1, i32 -1>
>   %S = shufflevector <16 x i32> %B, <16 x i32> %B, <16 x i32> undef
>   store <16 x i32> %S, <16 x i32>* %A2, align 64
>   ret void
> }
> ```
> 
> What I don't understand is, if we change this case's integer type from i32 to i16, `llc` will complaint, even though the instruction seems legit to me
> 
> ```
> ./build-debug/bin/llc: error: ./build-debug/bin/llc: PoC.ll:8:22: error: invalid shufflevector operands
>   %S = shufflevector <16 x i16> %B, <16 x i16> %B, <16 x i16> undef
> ```
> 
> But when switching to `<16 x i32>`, it crashes...
Could be a bit simplified further: https://llvm.godbolt.org/z/663P9z73G

Yeah, the vector of indices needs to have `i32` as element type I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132634