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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
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};
----------------
dmgreen wrote:
> v4i16?
Yeah I missed that one initially. Should be fixed now.


================
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; }))
----------------
dmgreen wrote:
> use auto instead? It's common to use CostTableLookup too, but I would guess that makes this more verbose?
I think CostTableLookup is overkill, because at the moment this just uses the same cost for each supported type. Updated to use auto.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:100
 
+  using BaseT::getIntrinsicInstrCost;
+  unsigned getIntrinsicInstrCost(const IntrinsicCostAttributes &ICA,
----------------
dmgreen wrote:
> What does this using do?
IIRC there used to be some compilers that had trouble with calling BaseT::getIntrinsicInstrCost without this, but it builds fine on my system without it. Let's see if any bot complains.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89953