[PATCH] D71501: Cost Annotation Writer for InlineCost

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:107
+  // pair of Cost and Threshold is associated.
+  llvm::DenseMap <const Instruction *, std::pair<int, int>> CostThresholdMap;
+
----------------
Nit: consider replacing std::pair<int,int> with a struct, e.g. struct InliningCostDetail {int Cost, int Threshold} - for readability.


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:576
+        return ;
+    Writer.CostThresholdMap[I].first = Cost - Writer.CostThresholdMap[I].first;
+    Writer.CostThresholdMap[I].second = Threshold -
----------------
I initially thought that the map stores a snapshot of Cost & Threshold values. It seems it actually stores cost and threshold variations due to the analysis of a specific instruction - relative cost of that one instruction, and threshold effects. 

If that's correct, could you please detail this in the comment describing the map (or struct members, if using a struct instead of std::pair)


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

https://reviews.llvm.org/D71501