[PATCH] D43769: [TTI] rename getArithmeticInstructionCost() to getUnitThroughput(); NFC

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:723
+  /// cases or optimizations based on those values.
+  int getUnitThroughput(
       unsigned Opcode, Type *Ty, OperandValueKind Opd1Info = OK_AnyValue,
----------------
spatel wrote:
> ABataev wrote:
> > spatel wrote:
> > > ABataev wrote:
> > > > Seems to me very general, the name does not show that this is the throughput for arithmetic instructions only. I think it is going to be enough just to clarify the comment.
> > > getXXXCost is the most ambiguous. getALUUnitThroughput is better?
> > > 
> > > I'd like to make it so we don't have to read the comments to distinguish this from getUserCost and getOperationCost. So if we don't change this name, then change those? 
> > > 
> > > I suppose we should change all of the throughput APIs in one shot if we're going to do this. But if there's no support, then I'll just update the code comment.
> > Still not sure. We're estimating the cost of the arithmetic LLVM instructions here in terms of throughput, not the ALU Unit throughput.
> Maybe I'm still confused. As I understand D43733, we're not estimating the LLVM instruction throughput for the target as a whole. If we were, then fmul/fadd/fsub should have a higher cost than integer add/sub because all recent x86 have greater overall throughput for scalar integer ops than FP ops because there are more scalar ALUs.
> 
> IMO, the more important part is that we change Cost -> Throughput for everything (I think) that the vectorizers are using, so we're at least different than 'user cost' or 'operation cost'. I guess people will have to read the comment anyway to distinguish the subtlety between 'getArithmeticThroughput' / 'getALUThroughput'.
> 
As I understand, we estimate throughout for the target as a whole. And because of that we assume that the cost of the integer scalar instructions is just 1 (though it may be lower). This just needs to be fixed somehow, e.g by introducing some multiplier.


https://reviews.llvm.org/D43769