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

[Peter Rong via Phabricator via llvm-commits]

Peter added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13369
+  // If all of them are -1, OOB read will happen later.
+  if (std::none_of(Mask.begin(), Mask.end(), [VecTy](unsigned Idx) {
+        return Idx < VecTy->getNumElements() * 2;
----------------
fhahn wrote:
> fhahn wrote:
> > LLVM provides `any_of`/`all_of` helpers that take a iterator range. It would be good to use them here. 
> > 
> > >   // If mask is `undef` or `poison`, `Mask` may be a vector of -1s.
> > 
> > There's a `constexpr` for `UndefMaskElem`: https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/IR/Instructions.h#L1996
> > 
> > It would probably be better to check for that. IIUC this is the only case where an index could be out-of-range; other cases would be already rejected by the verifier I think.
> Don't we want `any_of` here? Won't a single `-1` create an issue in the mask?
No, It seems a few UndefMaskElem won't affect how it works. See test case `https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/InterleavedAccess/AArch64/interleaved-accesses-inseltpoison.ll`.

Only a whole vector of UndefMaskElem will crash it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132634