[PATCH] D124506: [NFC][AArch64][CodeGen] Use ArrayRef in TargetLowering functions

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:378
   // type. The LowerXXX function will be trivial when f128 isn't involved.
-  setOperationAction(ISD::FP_TO_SINT, MVT::i32, Custom);
-  setOperationAction(ISD::FP_TO_SINT, MVT::i64, Custom);
-  setOperationAction(ISD::FP_TO_SINT, MVT::i128, Custom);
-  setOperationAction(ISD::STRICT_FP_TO_SINT, MVT::i32, Custom);
-  setOperationAction(ISD::STRICT_FP_TO_SINT, MVT::i64, Custom);
-  setOperationAction(ISD::STRICT_FP_TO_SINT, MVT::i128, Custom);
-  setOperationAction(ISD::FP_TO_UINT, MVT::i32, Custom);
-  setOperationAction(ISD::FP_TO_UINT, MVT::i64, Custom);
-  setOperationAction(ISD::FP_TO_UINT, MVT::i128, Custom);
-  setOperationAction(ISD::STRICT_FP_TO_UINT, MVT::i32, Custom);
-  setOperationAction(ISD::STRICT_FP_TO_UINT, MVT::i64, Custom);
-  setOperationAction(ISD::STRICT_FP_TO_UINT, MVT::i128, Custom);
-  setOperationAction(ISD::SINT_TO_FP, MVT::i32, Custom);
-  setOperationAction(ISD::SINT_TO_FP, MVT::i64, Custom);
-  setOperationAction(ISD::SINT_TO_FP, MVT::i128, Custom);
-  setOperationAction(ISD::STRICT_SINT_TO_FP, MVT::i32, Custom);
-  setOperationAction(ISD::STRICT_SINT_TO_FP, MVT::i64, Custom);
-  setOperationAction(ISD::STRICT_SINT_TO_FP, MVT::i128, Custom);
-  setOperationAction(ISD::UINT_TO_FP, MVT::i32, Custom);
-  setOperationAction(ISD::UINT_TO_FP, MVT::i64, Custom);
-  setOperationAction(ISD::UINT_TO_FP, MVT::i128, Custom);
-  setOperationAction(ISD::STRICT_UINT_TO_FP, MVT::i32, Custom);
-  setOperationAction(ISD::STRICT_UINT_TO_FP, MVT::i64, Custom);
-  setOperationAction(ISD::STRICT_UINT_TO_FP, MVT::i128, Custom);
-  setOperationAction(ISD::FP_ROUND, MVT::f16, Custom);
-  setOperationAction(ISD::FP_ROUND, MVT::f32, Custom);
-  setOperationAction(ISD::FP_ROUND, MVT::f64, Custom);
-  setOperationAction(ISD::STRICT_FP_ROUND, MVT::f16, Custom);
-  setOperationAction(ISD::STRICT_FP_ROUND, MVT::f32, Custom);
-  setOperationAction(ISD::STRICT_FP_ROUND, MVT::f64, Custom);
-
-  setOperationAction(ISD::FP_TO_UINT_SAT, MVT::i32, Custom);
-  setOperationAction(ISD::FP_TO_UINT_SAT, MVT::i64, Custom);
-  setOperationAction(ISD::FP_TO_SINT_SAT, MVT::i32, Custom);
-  setOperationAction(ISD::FP_TO_SINT_SAT, MVT::i64, Custom);
+  setOperationAction({ISD::FP_TO_SINT, ISD::STRICT_FP_TO_SINT, ISD::FP_TO_UINT,
+                      ISD::STRICT_FP_TO_UINT, ISD::SINT_TO_FP,
----------------
I can definitely some benefit here in terms of removing duplication, but in other places (I've left comments in a couple of examples) it's not obvious what the benefit is.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:881
     for (auto VT : {MVT::nxv16i8, MVT::nxv8i16, MVT::nxv4i32, MVT::nxv2i64}) {
-      setOperationAction(ISD::BITREVERSE, VT, Custom);
-      setOperationAction(ISD::BSWAP, VT, Custom);
-      setOperationAction(ISD::CTLZ, VT, Custom);
-      setOperationAction(ISD::CTPOP, VT, Custom);
-      setOperationAction(ISD::CTTZ, VT, Custom);
-      setOperationAction(ISD::INSERT_SUBVECTOR, VT, Custom);
-      setOperationAction(ISD::UINT_TO_FP, VT, Custom);
-      setOperationAction(ISD::SINT_TO_FP, VT, Custom);
-      setOperationAction(ISD::FP_TO_UINT, VT, Custom);
-      setOperationAction(ISD::FP_TO_SINT, VT, Custom);
-      setOperationAction(ISD::MGATHER, VT, Custom);
-      setOperationAction(ISD::MSCATTER, VT, Custom);
-      setOperationAction(ISD::MLOAD, VT, Custom);
-      setOperationAction(ISD::MUL, VT, Custom);
-      setOperationAction(ISD::MULHS, VT, Custom);
-      setOperationAction(ISD::MULHU, VT, Custom);
-      setOperationAction(ISD::SPLAT_VECTOR, VT, Custom);
-      setOperationAction(ISD::VECTOR_SPLICE, VT, Custom);
-      setOperationAction(ISD::SELECT, VT, Custom);
-      setOperationAction(ISD::SETCC, VT, Custom);
-      setOperationAction(ISD::SDIV, VT, Custom);
-      setOperationAction(ISD::UDIV, VT, Custom);
-      setOperationAction(ISD::SMIN, VT, Custom);
-      setOperationAction(ISD::UMIN, VT, Custom);
-      setOperationAction(ISD::SMAX, VT, Custom);
-      setOperationAction(ISD::UMAX, VT, Custom);
-      setOperationAction(ISD::SHL, VT, Custom);
-      setOperationAction(ISD::SRL, VT, Custom);
-      setOperationAction(ISD::SRA, VT, Custom);
-      setOperationAction(ISD::ABS, VT, Custom);
-      setOperationAction(ISD::ABDS, VT, Custom);
-      setOperationAction(ISD::ABDU, VT, Custom);
-      setOperationAction(ISD::VECREDUCE_ADD, VT, Custom);
-      setOperationAction(ISD::VECREDUCE_AND, VT, Custom);
-      setOperationAction(ISD::VECREDUCE_OR, VT, Custom);
-      setOperationAction(ISD::VECREDUCE_XOR, VT, Custom);
-      setOperationAction(ISD::VECREDUCE_UMIN, VT, Custom);
-      setOperationAction(ISD::VECREDUCE_UMAX, VT, Custom);
-      setOperationAction(ISD::VECREDUCE_SMIN, VT, Custom);
-      setOperationAction(ISD::VECREDUCE_SMAX, VT, Custom);
-
-      setOperationAction(ISD::UMUL_LOHI, VT, Expand);
-      setOperationAction(ISD::SMUL_LOHI, VT, Expand);
-      setOperationAction(ISD::SELECT_CC, VT, Expand);
-      setOperationAction(ISD::ROTL, VT, Expand);
-      setOperationAction(ISD::ROTR, VT, Expand);
-
-      setOperationAction(ISD::SADDSAT, VT, Legal);
-      setOperationAction(ISD::UADDSAT, VT, Legal);
-      setOperationAction(ISD::SSUBSAT, VT, Legal);
-      setOperationAction(ISD::USUBSAT, VT, Legal);
-      setOperationAction(ISD::UREM, VT, Expand);
-      setOperationAction(ISD::SREM, VT, Expand);
-      setOperationAction(ISD::SDIVREM, VT, Expand);
-      setOperationAction(ISD::UDIVREM, VT, Expand);
+      setOperationAction({ISD::BITREVERSE,
+                          ISD::BSWAP,
----------------
This change looks more cosmetic than functional - we're not really reducing lines of code here.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:968
+
+    setOperationAction({ISD::MLOAD, ISD::MSTORE, ISD::MGATHER, ISD::MSCATTER},
+                       {MVT::v4f16, MVT::v8f16, MVT::v2f32, MVT::v4f32,
----------------
Again, not sure of the benefit of changes like this - we've not removed duplication, but it has made it harder to grep for operation actions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124506