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

[Irina Dobrescu via Phabricator via llvm-commits]

Rin added inline comments.


================
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)
----------------
CarolineConcatto wrote:
> 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?
> 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

Yes, that's a good point. I'll remove them.

I think David mentioned the reason why the table does not sole the problem. Because it's trying to use the legal type that i8 will be converted to (i32) for the costs. Which is why I added that if. I'll add a comment in the code to highlight that. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102755