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

[Sanjay Patel via Phabricator via llvm-commits]

spatel 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,
----------------
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'.



https://reviews.llvm.org/D43769