Reapply "[AMDGPU] Modify adjustInliningThreshold to also consider the cost of passing function arguments through the stack"

[JanekvO]

Summary

Reapplies 142c28ffa1323e9a8d53200a22c80d5d778e0d0f as part of D140242 which got reverted due to amdgpu openmp test failures.

This diff fixes said failures by eliding most of adjustInliningThresholdUsingCallee for indirect calls as the callee function is unavailable for indirect calls.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

1178

This also will fail for constexpr casts and aliases. You don’t need to give up on these cases. You should be going off the callee type you can get from the CallBase

[JanekvO]

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

1178

I couldn't get isArgPassedInSGPR working with only the CallBase. Do indirect calls always pass args through VGPR?

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

1178

The calling convention is always the same and can’t be different for direct or indirect calls. The ABI is fully determined by the call type, the calling convention and the attribute set. Is there just a missing attribute wrapper for inreg that checks callee and callsite attributes?

[JanekvO]

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

1178

Ah, I thought isArgPassedInSGPR depended more on information on the Argument and its associated called Function. Seem it only really needs the inreg/byval attribute info of the arguments.

[scchan]

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

1178

If it's an indirect call, where's no inlining opportunity or am I missing something?

[JanekvO]

• Stop bailing out for unknown callees and consider args of indirect calls

[JanekvO]

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

1178

It can possibly still inline in specific cases (such as the one in the provided test). Moreover, the llvm::getInlineCost (CallBase &Call, Function *Callee, ... API that eventually calls this TTI hook may have calls (e.g., from LTO) where the CallBase &Call is for an indirect call without a known Called function but the user of the API provides a known Function *Callee (i.e., Call.getCalledFunction() != Callee).

[arsenm]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

2488–2489 ↗

(On Diff #495811)

You should go through CallBase::paramHasAttr rather than directly inspecting the AttributeList

[JanekvO]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

2488–2489 ↗

(On Diff #495811)

I tried to retain existing API for isArgPassedInSGPR(const Argument *A) which doesn't have access to any CallBase. I think I could force something with function_ref but not sure if that's the way to go.

[arsenm]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

2488–2489 ↗

(On Diff #495811)

Do we really need to retain the Argument version?

[JanekvO]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

2488–2489 ↗

(On Diff #495811)

Looking into it, there seems to be a only 2 uses of isArgPassedInSGPR(const Argument *A), however, I couldn't find a way to rewrite them to use a more CallBase friendly API.

[arsenm]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

2488–2489 ↗

(On Diff #495811)

I'm not really sure what the policy is for mismatched call site and declaration ABI attributes policy is supposed to be. We don't have a verifier check for it. My current assumption is for inreg, it would be present on the declaration and never in the call site attributes which won't work here. If you were to only pick one, you would be better off going off the callee's attribute set.

The Argument and call case are also different. The Argument version is for incoming and CallBase for outgoing. You might just need to split the two

[JanekvO]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

2488–2489 ↗

(On Diff #495811)

My current assumption is for inreg, it would be present on the declaration and never in the call site attributes which won't work here. If you were to only pick one, you would be better off going off the callee's attribute set.

This might be tough in the case of indirect calls since the CallBase is all that's provided to the TTI hook and CallBase::getCalledfunction will return nullptr. Perhaps adding the Callee function to the existing TTI or a new TTI hook to adjust cost/threshold where the Callee function can be accessed might be more appropriate?

[arsenm]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

2488–2489 ↗

(On Diff #495811)

I think just two duplicate functions for the incoming and outgoing case is fine. One will use Argument and the other CallBase and parameter index

[JanekvO]

• Duplicate isArgPassedInSGPR

[arsenm]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

2504 ↗

(On Diff #496508)

This is kind of meaningless but not really wrong for entry points

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp

 }
 
 static unsigned adjustInliningThresholdUsingCallee(const Function *Callee,
                                                    const SITargetLowering *TLI,
                                                    const GCNTTIImpl *TTIImpl) {
   const int NrOfSGPRUntilSpill = 26;
   const int NrOfVGPRUntilSpill = 32;
 
+  // In case of indirect calls, callee Function may not be retrievable from the
+  // CallBase.
+  if (!Callee)
+    return 0;
+
   const DataLayout &DL = TTIImpl->getDataLayout();
 
   unsigned adjustThreshold = 0;
   int SGPRsInUse = 0;
   int VGPRsInUse = 0;
   for (const Argument &A : Callee->args()) {
     SmallVector<EVT, 4> ValueVTs;
     ComputeValueVTs(*TLI, DL, A.getType(), ValueVTs);

llvm/test/Transforms/Inline/AMDGPU/amdgpu-inline-stack-indirect-call-argument.ll

+; REQUIRES: asserts
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -S -passes=inline -inline-cost-full=true -debug-only=inline-cost < %s 2>&1 | FileCheck %s
+
+; CHECK: Analyzing call of Dummy... (caller:Caller) 
+
+define void @Wrapper(ptr nocapture nofree noundef readonly %func, ptr noundef %arg, i32 noundef %a) {
+entry:
+  call void %func(ptr noundef %arg, i32 noundef %a)
+  ret void
+}
+
+define internal void @Dummy(ptr noundef %ptrarg, i32 noundef %intarg) {
+entry:
+  ret void
+}
+
+define void @Caller() {
+entry:
+  %arg = alloca ptr
+  call void @Wrapper(ptr noundef @Dummy, ptr noundef %arg, i32 42)
+  ret void
+}