Page MenuHomePhabricator

[TTI] getMemcpyCost
ClosedPublic

Authored by SjoerdMeijer on Mar 12 2019, 6:59 AM.

Details

Summary

This adds new function getMemcpyCost to TTI so that the cost of a memcpy can be
modeled and queried.

Diff Detail

Repository
rL LLVM

Event Timeline

SjoerdMeijer created this revision.Mar 12 2019, 6:59 AM
t.p.northover added inline comments.
include/llvm/Analysis/TargetTransformInfoImpl.h
144 ↗(On Diff #190249)

That seems optimistic, even as a default. I'd probably bump it to TCC_Expensive if just one load/store is needed, and a size-based estimate would be even better (is there a DataLayout handy to calculate length in terms of pointer-words?)

SjoerdMeijer marked an inline comment as done.Mar 12 2019, 9:18 AM
SjoerdMeijer added inline comments.
include/llvm/Analysis/TargetTransformInfoImpl.h
144 ↗(On Diff #190249)

Thanks for looking into this. Agreed that TCC_Basic doesn't make much sense.

As a default size based estimate, I am thinking of this default:

(2 * TCC_Basic)  * (memcpy-count / pointer-size)

where:
(2 * TCC_Basic) is a load/store pair, and
(memcpy-count / pointer-size) models the number of load/stores needed

Since this will change behaviour, I probably need a test somewhere. :-)

arsenm added a subscriber: arsenm.Mar 12 2019, 9:51 AM
arsenm added inline comments.
include/llvm/Analysis/TargetTransformInfoImpl.h
144 ↗(On Diff #190249)

A pointer size isn't relevant. We already have things like getMemcpyLoopLoweringType for when memcpy is expanded. The cost should also be much higher for unknown amounts

SjoerdMeijer updated this revision to Diff 190431.EditedMar 13 2019, 9:39 AM
SjoerdMeijer edited the summary of this revision. (Show Details)

In my previous comment, I commented on a possible default cost calculation for a memcpy, which approximated the cost by calculating the number of load/stores pairs. I probably could see this working as a default estimation, because I think when this would be used, the result will be that memcpy's are considered expensive relative to other instructions, which is probably correct. However, I appreciate that this is an approximation, and that there are many things missing here, like source/destination alignment, available load/store instruction, etc., which really shows this is a target dependent decision. So, I've changed getMemcpyCost to just simply return TCC_Expensive, so that target can override this method and implement their decision making there, like is done for many/most functions here in TTI.

I've added a regression test.

Apologies for already asking, but I was curious if @t.p.northover or @arsenm had more feedback/concerns or if this looks somewhat reasonable.

samparker accepted this revision.Mar 19 2019, 12:06 AM

LGTM. I think returning Expensive is the only reasonable thing we can do for a default implementation. It's more realistic than Basic and for anything else more complicated, it should be up to a target to override.

This revision is now accepted and ready to land.Mar 19 2019, 12:06 AM

Thanks for looking Sam!
I will wait a day with committing this, just in case there are more comments.

Closed by commit rL356555: [TTI] getMemcpyCost (authored by SjoerdMeijer, committed by ). · Explain WhyMar 20 2019, 7:18 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 20 2019, 7:18 AM