[PATCH] D102755: [AArch64] Add cost tests for bitreverse

[Caroline via Phabricator via llvm-commits]

CarolineConcatto added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:295
+    const auto *Entry =
+        CostTableLookup(BitreverseTbl, ICA.getID(), (LegalisationCost.second));
+    if (TLI->getValueType(DL, RetTy, true) == MVT::i8 ||
----------------
nit:
s/(LegalisationCost.second)/LegalisationCost.second/


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:298
+        TLI->getValueType(DL, RetTy, true) == MVT::i16)
+      return LegalisationCost.first * Entry->Cost + 1;
+    if (TLI->getValueType(DL, RetTy, true) != MVT::Other)
----------------
Is the '+1' added because the ICA.getID() is informing the type is MVT::i32?
If so, then should we remove these types from the table
    {Intrinsic::bitreverse, MVT::i8, 2},
    {Intrinsic::bitreverse, MVT::i16, 2}
?
If not(the problem is not with the wrong type being selected on the table) can you add an explanation why the table does not solve the problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102755