[PATCH] D68881: [AMDGPU] Improve code size cost model

[Daniil Fukalov via Phabricator via llvm-commits]

dfukalov marked 3 inline comments as done.
dfukalov added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInline.cpp:54
 static cl::opt<size_t>
-MaxBB("amdgpu-inline-max-bb", cl::Hidden, cl::init(300),
+MaxBB("amdgpu-inline-max-bb", cl::Hidden, cl::init(1100),
       cl::desc("Maximum BB number allowed in a function after inlining"
----------------
arsenm wrote:
> This is a separate change
The parameter' default value should be updated to correspond changed cost model, to avoid performance regressions.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:698
+                                 ArrayRef<const Value *> Operands) {
+  // Estimate extractelement elimination
+  if (const ExtractElementInst *EE = dyn_cast<ExtractElementInst>(U)) {
----------------
arsenm wrote:
> We already report vector insert/extract as free. Why does this need to look at these specifically? What is the purpose of Operands which seems to be ignored?
> 
> What uses this version? I thought the set of cost model function with specific value contexts were only used by the vectorizers
CostModel has three estimation modes: RecipThroughput, Latency and CodeSize. Vectorizer uses the first one but inliner and unroller use code size estimations.
Insert/extract and other estimations were implemented for RecipThroughput path only so e.g. inliner got wrong code size costs estimations for such instructions. The change introduces the same estimations for some trivial cases by overloading getUserCost().


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h:207
 
-  unsigned getInliningThresholdMultiplier() { return 7; }
+  unsigned getInliningThresholdMultiplier() { return 9; }
 
----------------
arsenm wrote:
> This is a separate change
The parameter' default value should be updated to correspond changed cost model, to avoid performance regressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68881