[PATCH] D79483: [CostModel] Replace getUserCost with getInstructionCost. WIP

[Sotiris Apostolakis via Phabricator via llvm-commits]

apostolakis added inline comments.


================
Comment at: llvm/test/CodeGen/X86/select-optimize.ll:395
+; CHECK:       select.end:
+; CHECK-NEXT:    [[SUB:%.*]] = phi double [ [[TMP1]], [[FOR_BODY]] ], [ 0.000000e+00, [[SELECT_FALSE]] ]
 ; CHECK-NEXT:    [[X_ADDR_1]] = fsub double [[X_ADDR_010]], [[SUB]]
----------------
RKSimon wrote:
> @apostolakis Please would take a look at this - we're about to start work on improving the cost numbers for latency/size and just this initial cleanup is causing this test to fail - which suggests you're relying on some very inaccurate costs.
I will remove this test. This particular test was a bit flaky to begin with given that it depended on the instruction latency of the instructions. At least it served the purpose of notifying me of the changes in the underlying cost modeling. 

Just to clarify the use in the select-optimize pass. The goal was to approximate TargetSchedModel::computeInstrLatency and compute the length of dependence chains. getInstructionCost seemed the best approximation based on the documentation. getUserCost had an unclear purpose to me, did not account for latency queries (I guess this will change now) and takes the operands as input which is worrisome since I just wanted the cost of the instructions on their own regardless of their operands (in practice the operands are mostly ignored so that might not be an issue). The biggest change in cost modeling at least from switching from getInstructionCost to getUserCost seem to be the function calls which might be for the better.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79483/new/

https://reviews.llvm.org/D79483