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

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I presume float versions would be useful too? Do you plan to add them?



================
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};
----------------
fhahn wrote:
> dmgreen wrote:
> > v4i16?
> Yeah I missed that one initially. Should be fixed now.
Look like it would be worth making sure there are tests too.


================
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; }))
----------------
fhahn wrote:
> 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.
Yeah I agree. Identical costs between sizes and opcodes don't make it very useful here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89953