[PATCH] D147322: [AArch64] Improve fshl cost modeling if 3rd arg is constant.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 511032.
fhahn added a comment.

Add TODOs, use cost 2 for i8 and i16.

In D147322#4239303 <https://reviews.llvm.org/D147322#4239303>, @dmgreen wrote:

> Hello. I looked briefly into funnel shifts recently as I had noticed regressions (from a patch that was not in the end committed). Some things I am aware of:

Thanks for taking a look!

> - If both operands are the same then it simplifies to a ror. I don't think that modifies the costs here though.
> - An extr with two different operands can be a higher cost instruction. Again I don't think that needs to change the costs though
> - For integers less than i32, one of the operands needs to be shifted so that the bits are in the top of the register. So the cost is 1 higher that i32/i64.

Yes, the thing I was struggeling with was how this could be modeled with CostTableLookup. It would automatically promote i8/i16 to MVT::i32 and I didn't find where it would account for the extra conversion cost. I added checking using the IR types for now.

> - For vectors we should really be generating USHR+SLI, but we don't do that yet.

That sounds like a good improvement!

> - I believe everything that applies to fshl in this patch should also equally apply to fshr? So long as the shift amount is const the costs should all be the same.

I *think* so. I'll create a test for fshr based on fshl.ll and then submit a follow-up patch if that sounds good to you?

> It is probably worth adding a TODO/FIXME that non-const shifts are not yet added.

I added 2 TODOs as suggsted,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147322

Files:
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/test/Analysis/CostModel/AArch64/fshl.ll
  llvm/test/Transforms/SLPVectorizer/AArch64/fshl.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147322.511032.patch
Type: text/x-patch
Size: 7782 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230405/05b2e5c4/attachment.bin>