This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Adjust default fp extend and trunc costs
ClosedPublic

Authored by dmgreen on Jun 24 2020, 5:52 AM.

Details

Summary

This adds some default costs for fp extends and truncates, generally costing them as 1 per lane. If the type is not legal then the cost will include a call to an __aeabi_ function.

Some NEON code is also adjusted to make sure it applies to the expected types, now that fp16 is a more common thing.

Diff Detail

Event Timeline

dmgreen created this revision.Jun 24 2020, 5:52 AM
SjoerdMeijer added inline comments.Jun 24 2020, 6:09 AM
llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
253

Quickly looking at this, I was wondering 2 things here:

  • how about MVE?
  • and how about f16?
llvm/test/Analysis/CostModel/ARM/cast.ll
134–143

I can't remember if Sam asked the same question on a different patch, but the cost looks a lot higher than the unchanged instructions here. Is that correct?

dmgreen marked 2 inline comments as done.Jun 24 2020, 6:56 AM
dmgreen added inline comments.
llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
253

This is just the bit of code removed from above, put into a better place and with checks for the actual types this was expecting. The old code was obviously written from a time before fp16 and doesn't account for it properly.

MVE is handled in a different patch. These were all interrelated but split up from D81813.

FP16 on NEON will fall back to the "1 vcvt per lane" code below, which will be better when fp16 is not present and will give roughly the same numbers when fp16 is present. But that is not what I am trying to fix here, I'm just trying to make sure it's not worse than before.

llvm/test/Analysis/CostModel/ARM/cast.ll
134–143

Yep. The "neon" target will not include fp16, so the converts will all go through __aeabi calls so should be expensive.

Same for the MVE costs below, which do not by default include double.

dmgreen updated this revision to Diff 273014.Jun 24 2020, 6:59 AM

Rebase tests

SjoerdMeijer accepted this revision.Jun 25 2020, 2:11 AM
SjoerdMeijer added inline comments.
llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
253

Ah okay, cheers, I see that now.
Looks reasonable.

This revision is now accepted and ready to land.Jun 25 2020, 2:11 AM
This revision was automatically updated to reflect the committed changes.