[TTI] getUserCost to return getCastInstrCost

[samparker]

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.

[spatel]

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]

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]

llvm/test/Analysis/CostModel/X86/size-cost.ll

33 ↗

(On Diff #258934)

Sounds good - thanks!

[spatel]

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]

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]

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

[samparker]

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.

[samparker]

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]

Now handled in D79848.

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

     case Instruction::UDiv:
     case Instruction::URem:
       // FIXME: Unlikely to be true for CodeSize.
       return TTI::TCC_Expensive;
     case Instruction::IntToPtr:
     case Instruction::PtrToInt:
     case Instruction::Trunc:
     case Instruction::BitCast:
-      if (TargetTTI->getCastInstrCost(Opcode, Ty, OpTy, CostKind, I) ==
-          TTI::TCC_Free)
-        return TTI::TCC_Free;
-      break;
+      return TargetTTI->getCastInstrCost(Opcode, Ty, OpTy, CostKind, I);
     case Instruction::FPExt:
     case Instruction::SExt:
     case Instruction::ZExt:
       if (I && TargetTTI->getExtCost(I, Operands.back()) == TTI::TCC_Free)
         return TTI::TCC_Free;
       break;
     }
     // By default, just classify everything as 'basic'.