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

[Daniil Fukalov via Phabricator via llvm-commits]

dfukalov marked an inline comment as done.
dfukalov added inline comments.


================
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:
> dfukalov wrote:
> > 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().
> Nothing here looks target specific though? It's just forwarding the calls. Why doesn't the base implementation do this?
The parameters preparation and calls forwarding scheme is from base implementation of getInstructionThroughput() but we cannot say that for all targets zero cost in terms of throughtput means zero code size.
Moreover, getVectorInstrCost() is target specific here and I'm going to add estimation with overloaded getShuffleCost() too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68881