[PATCH] D92208: [AArch64][CostModel] Fixed costs for mul <2 x i64>

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:641
   case ISD::MUL:
+  {
+    // Since we do not have a MUL.2d instruction, a mul <2 x i64> is expensive
----------------
Formatting I think is usually done differently here?


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:645
+    auto *VecTy = dyn_cast<FixedVectorType>(Ty);
+    bool IsInt64 = Ty->getScalarType()->isIntegerTy(64);
+    if (VecTy && IsInt64)
----------------
I think this can just use LT.second == MVT::v2i64? (Otherwise it needs to account for scalable vectors, but dealing with the MVT is probably simpler).


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:647
+    if (VecTy && IsInt64)
+      return 1 * VecTy->getNumElements() + VecTy->getNumElements();
+    return (Cost + 1) * LT.first;
----------------
Hmm. According this this it should have a cost around 8:
https://godbolt.org/z/fjjEc7
LT.first is the cost factor to get it to the MVE::v2i64 type. getScalarizationOverhead could be used to get that overhead.

What do you think of something like LT.first * (2 + 2*getScalarizationOverhead(extract) + getScalarizationOverhead(insert)) ? I'm not sure what cost that would give.


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

https://reviews.llvm.org/D92208