Page MenuHomePhabricator

[TTI] BasicTTI::getInterleavedMemoryOpCost(): use getScalarizationOverhead()
ClosedPublic

Authored by RKSimon on Sep 29 2021, 6:43 AM.

Details

Summary

getScalarizationOverhead() results in a somewhat better cost estimation
than counting the insertion/extraction costs directly.
Notably, this is still overestimating the costs.

Diff Detail

Event Timeline

lebedev.ri requested review of this revision.Sep 29 2021, 6:43 AM
lebedev.ri created this revision.

(note that i haven't update all the affected tests, only the checkline blocks where there is at least one change have been updated)

I've done my own version and I have all the tests updated - would you mind if I commandeered this with my version?

I've done my own version and I have all the tests updated - would you mind if I commandeered this with my version?

Sure, go ahead!

RKSimon commandeered this revision.Sep 29 2021, 7:47 AM
RKSimon updated this revision to Diff 375888.
RKSimon added a reviewer: lebedev.ri.

Update all tests and just getScalarizationOverhead wrapper when all elements are demanded

lebedev.ri accepted this revision.Sep 29 2021, 7:53 AM

LG, it's a bit of an improvement in modelling precision, but not much.
Thank you!

llvm/include/llvm/CodeGen/BasicTTIImpl.h
1257

Please can you name the bool params?

This revision is now accepted and ready to land.Sep 29 2021, 7:53 AM
lebedev.ri retitled this revision from [WIP] BasicTTI::getInterleavedMemoryOpCost(): use getScalarizationOverhead() to [TTI] BasicTTI::getInterleavedMemoryOpCost(): use getScalarizationOverhead().Sep 29 2021, 7:57 AM
lebedev.ri edited the summary of this revision. (Show Details)

cheers - it seems to be enough to cause an actual test change so is at least going in the right direction. A better step would be to get this converted to shuffle costs, but we have really poor handling of length changing shuffles

llvm/include/llvm/CodeGen/BasicTTIImpl.h
1257

yes I'll add them

This revision was landed with ongoing or failed builds.Sep 29 2021, 8:42 AM
This revision was automatically updated to reflect the committed changes.