[PATCH] D89953: [AArch64] Implement getIntrinsicInstrCost, handle min/max intrinsics.

Thu Oct 22 05:56:01 PDT 2020

dmgreen added a comment.

Yeah, this sounds useful. I know we've hit problems with min/max costs in the past (the cmp/select kind, I don't know if they were float). It would be good to see them costed more accurately.

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:222
+  case Intrinsic::umax: {
+    static const auto ValidMinMaxTys = {MVT::v8i8, MVT::v16i8, MVT::v8i16,
+                                        MVT::v2i32, MVT::v4i32};
----------------
v4i16?

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:224
+                                        MVT::v2i32, MVT::v4i32};
+    std::pair<int, MVT> LT = TLI->getTypeLegalizationCost(DL, RetTy);
+    if (any_of(ValidMinMaxTys, [&LT](MVT M) { return M == LT.second; }))
----------------
use auto instead? It's common to use CostTableLookup too, but I would guess that makes this more verbose?

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:100

+  using BaseT::getIntrinsicInstrCost;
+  unsigned getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
----------------
What does this using do?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89953