[PATCH] D98362: [AMDGPU] Fix -amdgpu-inline-arg-alloca-cost

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:1192
   if (AllocaSize)
-    return ArgAllocaCost;
+    return ArgAllocaCost * getInliningThresholdMultiplier();
   return 0;
----------------
arsenm wrote:
> rampitec wrote:
> > dfukalov wrote:
> > > rampitec wrote:
> > > > arsenm wrote:
> > > > > Should the just adjust for the scale then?
> > > > I thought about this, but whenever we will adjust the scale the next time we will have to visit it again.
> > > Nit: It seems instead of this modification you can just swap two lines
> > > ```
> > > 1582:  Threshold *= TTI.getInliningThresholdMultiplier();
> > > 1583:  Threshold += TTI.adjustInliningThreshold(&Call);
> > > 
> > > ```
> > > [[ https://reviews.llvm.org/source/llvm-github/browse/main/llvm/lib/Analysis/InlineCost.cpp$1582-1583 | in InlineCost.cpp ]] so we'll stay with just one place of `* getInliningThresholdMultiplier()`.
> > That would change behavior for all targets.
> I thought the point of the multiplier was to just amplify the expense of calls. I don't understand scaling up the cost here
It's more like a uniform target cost multiplier which shall be applied to everything. But probably Daniil is correct and I have to swap two lines in the inliner instead, until noone uses it as is.


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

https://reviews.llvm.org/D98362