[PATCH] D70542: [AArch64][SVE] Add intrinsics for gather loads with 64-bit offsets

[Andrzej Warzynski via Phabricator via llvm-commits]

andwar marked 15 inline comments as done.
andwar added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:617
   setTargetDAGCombine(ISD::SIGN_EXTEND);
+  setTargetDAGCombine(ISD::SIGN_EXTEND_INREG);
   setTargetDAGCombine(ISD::BITCAST);
----------------
sdesmalen wrote:
> Is this change supposed to be here?
Removed, thanks!


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:776
     // directly.
-    for (MVT VT : MVT::fixedlen_vector_valuetypes()) {
+    for (MVT VT : MVT::vector_valuetypes()) {
       setOperationAction(ISD::SIGN_EXTEND_INREG, VT, Expand);
----------------
sdesmalen wrote:
> is this change supposed to be here?
Good catch, thanks, removed!


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11821
+  // means that we can avoid adding TableGen patterns for FPs.
+  if (RetVT.isFloatingPoint()) {
+    EVT OutFpVT = RetVT;
----------------
sdesmalen wrote:
> Can this code not simply do a BITCAST to `OutVT`?
Good catch, thanks! Updated.


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-gather-loads-64bit-offset.ll:11
+; CHECK: ld1b { z0.d }, p0/z, [x0, z0.d]
+; CHECK-NEXT:	mov	w8, #255
+; CHECK-NEXT: mov	z1.d, x8
----------------
sdesmalen wrote:
> The check-lines for the sign/zero extend are not really necessary, but it will make it more obvious to see them removed in a patch that folds the sign/zero extension into the load itself.
Ack


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70542