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

[Dave Green via Phabricator via llvm-commits]

dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

Looks sensible to me, thanks



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:293
+    if (const auto *Entry = CostTableLookup(CtpopCostTbl, ISD::CTPOP, MTy)) {
+      if (!MTy.isVector() ||
+          MTy.getScalarSizeInBits() == RetTy->getScalarSizeInBits())
----------------
I may be simpler to write it as this, without the if:
```
  // Extra cost of +1 when illegal vector types are legalized by promoting
  // the integer type.
  int ExtraCost = MTy.getScalarSizeInBits() != RetTy->getScalarSizeInBits() ? 1 : 0;
  return LT.first * Entry->Cost + ExtraCost;
```


================
Comment at: llvm/test/Analysis/CostModel/AArch64/ctpop.ll:64
 define <2 x i32> @test_ctpop_v2i32(<2 x i32> %a) {
-;
 ; CHECK-LABEL: 'test_ctpop_v2i32'
----------------
CarolineConcatto wrote:
> nit: I don't know why ';' was added here in all functions, but I don't think you should use this patch to remove them.
If this is what the update script produces, I'm OK with the extra lines being removed. Either here or in a separate NFC patch.


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

https://reviews.llvm.org/D103952