[PATCH] D71773: [AArch64][SVE] Update the definition of AdvSIMD_GatherLoad_VecTorBase_Intrinsic

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen accepted this revision.
sdesmalen added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12149
+    uint64_t MaxIndex = 31;
+    uint64_t SrcElSize = SrcElVT.getStoreSize().getKnownMinSize();
+
----------------
nit: SrcEltVT is fixed-sized, so you can ask for `getFixedSize()` here.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12152
+    ConstantSDNode *OffsetConst = dyn_cast<ConstantSDNode>(Offset.getNode());
+    if (nullptr == OffsetConst ||
+        OffsetConst->getZExtValue() > MaxIndex * SrcElSize ||
----------------
nit: `!OffsetConst`


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12158
+      else
+        Opcode = AArch64ISD::SST1;
+
----------------
nit: perhaps add an assert here that Base.getValueType().getSimpleVT().SimpleTy == MVT::nxv2i64 ?


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-scatter-stores-vector-base-imm-offset.ll:17
+                                                                       <vscale x 4 x i32> %base,
+                                                                       i64 16)
+  ret void
----------------
Maybe worth adding one test-case with a negative value (that should be covered by the use of `getZExtValue()`) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71773