[PATCH] D96263: [RISCV] Support scalable-vector masked gather operations

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1778
+    // unsigned scaled indices it helps prevent overflow when scaling.
+    if (IndexVT.getVectorElementType().bitsLT(XLenVT)) {
+      IndexVT = IndexVT.changeVectorElementType(XLenVT);
----------------
frasercrmck wrote:
> frasercrmck wrote:
> > craig.topper wrote:
> > > Is it possible that IndexVT has 32-bit elements on a 64-bit target and that the IndexVT is already LMUL==8 such that this SIGN_EXTEND/ZERO_EXTEND would produce an illegal LMUL==16 type?
> > Yes, it's possible, and similarly on a 32-bit target with an illegal 8- or 16-bit index type, but I think that implies the original intrinsic would have an illegal pointer type, like `<vscale x 16 x i32*>`. So in that sense it's ideally something LLVM would help legalize (split) for us, but sadly it folds the illegal intrinsic to one with legal types, e.g.: `i32* base, <vscale x 16 x i32> idxs` so it doesn't see that until it asks us to custom-lower.
> > 
> > Am I right in thinking we have to custom-legalize this ourselves, duplicating logic from `LegalizeVectorTypes`?
> It should be possible to do a DAGCombine pre-legalization which creates the illegal XLEN index types and passes it back to go through the vanilla legalization, if you think that's a valid route.
> 
> Either way it's going to require support for scalable-vector insert/extract subvector. In the worst case we'll have to expand nxv64i64 (from nxv64i8) into 8 x nxv8i64.
Would the DAG combine end up fighting with the combine that uses shouldRemoveExtendFromGSIndex?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96263