[PATCH] D140242: [AMDGPU] Modify adjustInliningThreshold to also consider the cost of passing function arguments through the stack

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:86-88
+static cl::opt<unsigned> ArgStackInlinePenalty(
+    "amdgpu-inline-arg-stack-cost", cl::Hidden, cl::init(15),
+    cl::desc("Cost per argument for function arguments passed through stack"));
----------------
JanekvO wrote:
> arsenm wrote:
> > Should be able to compute this directly from the existing costs for stack stores
> Sorry, I couldn't find any constant or cl option for stack store costs. Did you have anything in mind to replace this cl option?
I mean TargetTransformInfo::getMemoryOpCost (looking now and I don't think we ever implemented this. I know I tried to implement at one point but I guess never pushed it. We should implement this too)


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:1198
+    if (AMDGPU::isArgPassedInSGPR(&A))
+      SGPRsInUse++;
+    else
----------------
arsenm wrote:
> Raw argument counts don't correspond to register counts, need to get the type legalized register size
Get getPrimitiveSizeInBits doesn't work for pointers. You're also going to be repeating a lot of legalization logic. You're better off using getRegisterTypeForCallingConv and getNumRegistersForCallingConv


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140242