[PATCH] D105871: Followup to D99355: implementation of sdag support for vp load/store/gather/scatter.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added a comment.

Sorry that my review feedback is piecemeal. What I'm doing is running the following test (`declare`s omitted for brevity) with RISC-V. I think we want to see "cannot select" errors for each function, but I'm currently only getting that with `vp.load` and fixed-length `vp.gather`. The rest assert.

  ; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -riscv-v-vector-bits-min=128 < %s
  
  define <vscale x 2 x i32> @vp_gather_nxv2i32(<vscale x 2 x i32*> %ptrs, <vscale x 2 x i1> %mask, i32 zeroext %evl) {
    %ld = call <vscale x 2 x i32> @llvm.vp.gather.nxv2i32.p0nxv2i32(<vscale x 2 x i32*> %ptrs, <vscale x 2 x i1> %mask, i32 %evl)
    ret <vscale x 2 x i32> %ld
  }
  
  define <4 x i32> @vp_gather_v4i32(<4 x i32*> %ptrs, <4 x i1> %mask, i32 zeroext %evl) {
    %ld = call <4 x i32> @llvm.vp.gather.v4i32.p0v4i32(<4 x i32*> %ptrs, <4 x i1> %mask, i32 %evl)
    ret <4 x i32> %ld
  }
  
  define <vscale x 2 x i32> @vp_load_nxv2i32(<vscale x 2 x i32>* %ptr, <vscale x 2 x i1> %mask, i32 zeroext %evl) {
    %ld = call <vscale x 2 x i32> @llvm.vp.load.nxv2i32.p0nxv2i32(<vscale x 2 x i32>* %ptr, <vscale x 2 x i1> %mask, i32 %evl)
    ret <vscale x 2 x i32> %ld
  }
  
  define <4 x i32> @vp_load_v4i32(<4 x i32>* %ptr, <4 x i1> %mask, i32 zeroext %evl) {
    %ld = call <4 x i32> @llvm.vp.load.v4i32.p0v4i32(<4 x i32>* %ptr, <4 x i1> %mask, i32 %evl)
    ret <4 x i32> %ld
  }
  
  define void @vp_store_nxv2i32(<vscale x 2 x i32> %val, <vscale x 2 x i32>* %ptr, <vscale x 2 x i1> %mask, i32 zeroext %evl) {
    call void @llvm.vp.store.nxv2i32.p0nxv2i32(<vscale x 2 x i32> %val, <vscale x 2 x i32>* %ptr, <vscale x 2 x i1> %mask, i32 %evl)
    ret void
  }
  
  define void @vp_store_v4i32(<4 x i32> %val, <4 x i32>* %ptr, <4 x i1> %mask, i32 zeroext %evl) {
    call void @llvm.vp.store.v4i32.p0v4i32(<4 x i32> %val, <4 x i32>* %ptr, <4 x i1> %mask, i32 %evl)
    ret void
  }
  
  define void @vp_scatter_v4i32(<4 x i32> %val, <4 x i32*> %ptrs, <4 x i1> %mask, i32 zeroext %evl) {
    call void @llvm.vp.scatter.v4i32.p0v4i32(<4 x i32> %val, <4 x i32*> %ptrs, <4 x i1> %mask, i32 %evl)
    ret void
  }
  
  define void @vp_scatter_nxv2i32(<vscale x 2 x i32> %val, <vscale x 2 x i32*> %ptrs, <vscale x 2 x i1> %mask, i32 zeroext %evl) {
    call void @llvm.vp.scatter.nxv2i32.p0nxv2i32(<vscale x 2 x i32> %val, <vscale x 2 x i32*> %ptrs, <vscale x 2 x i1> %mask, i32 %evl)
    ret void
  }



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7898
+
+  assert(N->getMask().getValueType().getVectorNumElements() ==
+             N->getValueType(0).getVectorNumElements() &&
----------------
This should be `getVectorElementCount` so it doesn't crash on scalable vectors


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7936
+
+  assert(N->getMask().getValueType().getVectorNumElements() ==
+             N->getValue().getValueType().getVectorNumElements() &&
----------------
`getVectorElementCount` for scalable vectors.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7375
+    bool AddToChain = true;
+    ;
+    if (Opcode == ISD::VP_LOAD) {
----------------
nit: empty line


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7431
+    Value *PtrOperand = VPIntrin.getArgOperand(1);
+    EVT VT = ValueVTs[0];
+    MaybeAlign Alignment = DAG.getEVTAlign(VT);
----------------
This is an out-of-bounds access because `vp.store`/`vp.gather` return `void` and so `ValueVTs` is empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105871