[PATCH] D103952: [CostModel][AArch64] Improve the cost estimate of CTPOP intrinsic

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:281
+    // llvm/test/CodeGen/AArch64/arm64-vpopcnt.ll
+    static const CostTblEntry CtpopCostTbl[] = {
+        {ISD::CTPOP, MVT::i64, 4},
----------------
>From a quick look, lookup tables here are more rare, while the it seems a lot more prevalent in the x86 implementations. But I quite like this little table here, it's accurate and easy to read, so have no problem with it.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:289
+    MVT MTy = MVT::getIntegerVT(RetTy->getScalarSizeInBits());
+    if (const auto *Entry = CostTableLookup(CtpopCostTbl, ISD::CTPOP, MTy)) {
+      return LT.first * Entry->Cost;
----------------
Nit: you don't need the curly brackets for an if with 1 statement.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:289
+    MVT MTy = MVT::getIntegerVT(RetTy->getScalarSizeInBits());
+    if (const auto *Entry = CostTableLookup(CtpopCostTbl, ISD::CTPOP, MTy)) {
+      return LT.first * Entry->Cost;
----------------
SjoerdMeijer wrote:
> Nit: you don't need the curly brackets for an if with 1 statement.
Do we have a test case where the table look up fails?


================
Comment at: llvm/test/Analysis/CostModel/AArch64/ctpop.ll:168
 ; CHECK-LABEL: 'test_ctpop_v32i8'
-; CHECK-NEXT:  Cost Model: Found an estimated cost of 4 for instruction: %ctpop = call <32 x i8> @llvm.ctpop.v32i8(<32 x i8> %a)
+; CHECK-NEXT:  Cost Model: Found an estimated cost of 2 for instruction: %ctpop = call <32 x i8> @llvm.ctpop.v32i8(<32 x i8> %a)
 ; CHECK-NEXT:  Cost Model: Found an estimated cost of 0 for instruction: ret <32 x i8> %ctpop
----------------
Does this really have a cost of only 2?

I guess it's 2 because `LT.first * Entry->Cost = 2 * 1 = 2`

So I am wondering if it is easier to expand the table with vector types, and if the lookup fails let it fall back to the generic function that will hopefully then assign a high cost for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103952