[PATCH] D97858: [AArch64][SVE] Fold vector ZExt/SExt into gather loads where possible

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14385
+          Opc == AArch64ISD::GLD1S_SXTW_SCALED_MERGE_ZERO ||
+          Opc == AArch64ISD::GLD1S_IMM_MERGE_ZERO) &&
+         "Unexpected opcode");
----------------
Are these opcodes in a range, i.e. could you assert something like:

assert((Opc >= Op1 && Opc <= Op2) || (Opc >= Op3 && Opc <= Op4))

It might make the assert look a bit nicer perhaps?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14396
+                    Opc == AArch64ISD::GLD1_UXTW_SCALED_MERGE_ZERO;
+  const bool Extended = SExt || ZExt;
+
----------------
In fact, you could move the above assert here instead and have something like:

assert(Scaled || Signed || SExt || ZExt)

maybe?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15827
+  case AArch64ISD::GLD1S_SXTW_SCALED_MERGE_ZERO:
+  case AArch64ISD::GLD1S_IMM_MERGE_ZERO:
+    return performGLD1Combine(N, DAG);
----------------
I wonder if you maybe want to make the combine function simpler for now in terms of asserts and checking by only passing in the opcodes that you actually intend to combine? If later on you want to add more then it's easy enough to update. This is just a suggestion though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97858