[PATCH] D37170: [TargetTransformInfo] Add a new public interface getInstructionCost

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:115
+    TCK_Throughput, ///< The traditional cost model, it actually
+                    ///< means reciprocal throughputs.
+    TCK_Latency,    ///< The latency of instruction.
----------------
throughputs -> throughput


================
Comment at: include/llvm/Analysis/TargetTransformInfo.h:127
+    case TCK_Throughput:
+      return getUserCost(I);
+
----------------
This is good, but this is the wrong function. getUserCost is what the inliner uses, and could be viewed as returning normalized code size (right now), not throughput.

If we want to have a single interface like this for throughput, which just takes an arbitrary instruction, then we can, but we need to pull the code out of CostModelAnalysis::getInstructionCost (in lib/Analysis/CostModel.cpp). I'd be highly supportive of doing this (currently that code lives in the analysis pass because that's how we test the code-model interface, but it's not otherwise reusable, and that's not great).

Then we need to decide how to integrate everything. One option is to add this `TargetCostKind kind` parameter to the existing throughput cost-model interface. Another option is to only have this parameter on this generic function taking an Instruction* and leave everything else alone. Or we leave all of this alone and just add new interfaces for now providing latency and size (which we do by renaming the 'user cost' function).


https://reviews.llvm.org/D37170