[PATCH] D104471: [llvm][sve] Lowering for VLS truncating stores

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

This patch is making way too many seemingly unrelated code changes without much explanation.  Is all of this really necessary for the initial patch?



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1225
+      for (MVT InnerVT : MVT::integer_fixedlen_vector_valuetypes()) {
+        setTruncStoreAction(VT, InnerVT, Custom);
+      }
----------------
Can we change this to make sure we only mark stores that we can actually lower "custom"?  (i.e. the value type is a legal integer vector, and the memory type has a legal element type.)  That should make the rest of the patch simpler, I think.


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-trunc-stores.ll:21
+; Don't use SVE when its registers are no bigger than NEON.
+; NO_SVE-NOT: ptrue
+
----------------
Why don't we want to optimize store_trunc_v2i64i8 when SVE registers are 128 bits wide?


================
Comment at: llvm/test/CodeGen/AArch64/sve-fixed-length-trunc-stores.ll:183
+; VBITS_EQ_256-DAG: uzp1 [[Z1]].h, [[Z1]].h, [[Z1]].h
+; VBITS_EQ_256-DAG: splice [[Z1]].h, [[PG]], [[Z1]].h, [[Z0]].h
+; VBITS_EQ_256-DAG: ptrue [[PG]].h, vl16
----------------
Can we "uzp1 z0, z0, z1" or something like that in store_trunc_v16i32i16?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104471