Page MenuHomePhabricator

[TTI] getUserCost to return getCastInstrCost
AbandonedPublic

Authored by samparker on Apr 21 2020, 2:23 AM.

Details

Summary

If the generic cost model, based on DataLayout, says that a cast is free then return zero. Otherwise we now return the cost provided by getCastInstrCost.

Diff Detail

Event Timeline

samparker created this revision.Apr 21 2020, 2:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2020, 2:23 AM
spatel added inline comments.Apr 21 2020, 7:09 AM
llvm/test/Analysis/CostModel/X86/size-cost.ll
33 ↗(On Diff #258934)

Let me know if I'm missing larger context, but this is not correct. A cast from xmm <-> gpr ("movq %xmm0, %rax") is not free in any sense on any x86 target AFAIK.

Do we need to fix the x86 override as a preliminary step?

samparker marked an inline comment as done.Apr 21 2020, 7:36 AM
samparker added inline comments.
llvm/test/Analysis/CostModel/X86/size-cost.ll
33 ↗(On Diff #258934)

I didn't expect it to be correct.. I'm happy to look into the fix.

spatel added inline comments.Apr 21 2020, 9:06 AM
llvm/test/Analysis/CostModel/X86/size-cost.ll
33 ↗(On Diff #258934)

Sounds good - thanks!

Actually, let me take a step back: we're saying that getCastInstrCost() is a code-size query? Is that what the current callers are assuming? Do we need to invent a new API that makes it explicit that we are modeling size cost?

lebedev.ri added inline comments.
llvm/test/Analysis/CostModel/X86/size-cost.ll
33 ↗(On Diff #258934)

Is it thought? Note that we test *size* cost here.
Does bitcast instruction itself actually has size cost?
(does it get lowered to any assembly instructions?)

spatel added inline comments.Apr 21 2020, 10:28 AM
llvm/test/Analysis/CostModel/X86/size-cost.ll
33 ↗(On Diff #258934)

I was assuming this and the other test requires one of:

movq %xmm0, %rax
movq %rax, %xmm0

Actually, let me take a step back: we're saying that getCastInstrCost() is a code-size query? Is that what the current callers are assuming? Do we need to invent a new API that makes it explicit that we are modeling size cost?

getUserCost is called for code size, but it's not explicitly code-size only, I believe it was designed around the vague notion of hybrid of size and performance which the 'TargetCostConstants' describe and this is the same for getCastInstrCost. I'd like to change these methods to take a TargetCostKind argument so that we can end this ambiguity. It would also allow removal of the 'getInstructionLatency' stuff as it only used in one place and has no backend implementation.

This now looks like an NFC with the bitcasts fixed, but I strongly suspect that some backends will need tweaking when D78922 is in too.

samparker abandoned this revision.Fri, May 15, 1:17 AM

Now handled in D79848.