[PATCH] D104630: [AArch64][CostModel] Add cost model for experimental.vector.splice

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1814
+
+  assert(Kind == TTI::SK_Splice && "Expected Kind Splice");
+  auto AdjustType = [&](MVT M) {
----------------
is it even needed to pass a Kind or Mask in the first place, they seem unused.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1815
+  assert(Kind == TTI::SK_Splice && "Expected Kind Splice");
+  auto AdjustType = [&](MVT M) {
+    if (M == MVT::nxv16i1)
----------------
This can be a switch statement instead?

Also, how about giving the a name like `getPromotedTypeForPredicate` ?


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1842
+  static const TypeConversionCostTblEntry VectorSelectTbl[] = {
+      { ISD::TRUNCATE, MVT::nxv2i1, MVT::nxv2i64,  2 }, // promote + truncate
+      { ISD::TRUNCATE, MVT::nxv4i1, MVT::nxv4i32,  2 },
----------------
This could just use `getCastInstrCost` instead of the custom table?


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1852
+  if (Index < 0) {
+    unsigned CmpOpcode =
+        Tp->isFPOrFPVectorTy() ? Instruction::FCmp : Instruction::ICmp;
----------------
The compare is always an integer compare, i,e.  `cmp ge <0, 1, 2, 3, ... N-1>, <idx, idx, idx, ... idx>`


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1862
+
+  if (const auto *Entry = CostTableLookup(
+          ShuffleTbl, Kind,
----------------
I think the cost has to find one, otherwise we have an unhandled/illegal type.
So instead of `if`, this should have an `assert` that Entry != nullptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104630