[AMDGPU] Report minimum scratch size in code object v5 and later by default

[abinavpp]

Summary

This change sets
-amdgpu-assume-{external-call-stack-size | dynamic-stack-object-size}
options to zero by default for code object v5 and later. The runtime is
expected to adjust the scratch size if the amdhsa_uses_dynamic_stack bit
in the kernel descriptor is set.

[arsenm]

This should remove the amdgpu-assume-external-call-stack-size and amdgpu-assume-dynamic-stack-object-size flags too

llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

698–699 ↗

(On Diff #439028)

Dynamic call size does not imply 0 for the statically reported size. We still need to report the minimum statically known requirement

[abinavpp]

llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

698–699 ↗

(On Diff #439028)

Does that mean we should do the following instead?

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp b/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
index f7f93c75c870..e6a5aa989c9d 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -165,8 +151,6 @@ AMDGPUResourceUsageAnalysis::analyzeResourceUsage(
 
   // Assume a big number if there are any unknown sized objects.
   Info.HasDynamicallySizedStack = FrameInfo.hasVarSizedObjects();
-  if (Info.HasDynamicallySizedStack)
-    Info.PrivateSegmentSize += AssumedStackSizeForDynamicSizeObjects;
 
   if (MFI->isStackRealigned())
     Info.PrivateSegmentSize += FrameInfo.getMaxAlign().value();
@@ -469,16 +453,12 @@ AMDGPUResourceUsageAnalysis::analyzeResourceUsage(
             // directly call the tail called function. If a kernel directly
             // calls a tail recursive function, we'll assume maximum stack size
             // based on the regular call instruction.
-            CalleeFrameSize =
-              std::max(CalleeFrameSize,
-                       static_cast<uint64_t>(AssumedStackSizeForExternalCall));
+            CalleeFrameSize = 0;
           }
         }
 
         if (IsIndirect || I == CallGraphResourceInfo.end()) {
-          CalleeFrameSize =
-              std::max(CalleeFrameSize,
-                       static_cast<uint64_t>(AssumedStackSizeForExternalCall));
+          CalleeFrameSize = 0;

[abinavpp]

Stop falling back to 16k (WIP)

[abinavpp]

llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

698–699 ↗

(On Diff #439028)

Updated the revision with the above change. Let me know if we're good with the test changes.

[abinavpp]

Rebase

[abinavpp]

Do we need to worry about non HSA platforms here? Like the behavior of AMDGPUPALMetadata::setFunctionScratchSize(). Also, the calculation of SIProgramInfo::ScratchBlocks?

[arsenm]

In D128346#3607596, @abinavpp wrote:

Do we need to worry about non HSA platforms here? Like the behavior of AMDGPUPALMetadata::setFunctionScratchSize(). Also, the calculation of SIProgramInfo::ScratchBlocks?

I think this should be consistent everywhere. The fixed size was always an undesirable hack. I'm not sure if the shaders have a defined way to represent this though

[yaxunl]

Will this patch cause HIP apps using the dynamic stack to fail if HIP runtime change is not in place?

Do we have a plan for a smooth transition of dynamic stack support in HIP runtime? Thanks.

[arsenm]

In D128346#3608431, @yaxunl wrote:

Will this patch cause HIP apps using the dynamic stack to fail if HIP runtime change is not in place?

Yes, The runtime changes should happen in advance. This is also a rocr change

[abinavpp]

Rebase

[arsenm]

llvm/test/CodeGen/AMDGPU/GlobalISel/non-entry-alloca.ll

139 ↗

(On Diff #439817)

This should now check the dynamic size fields

llvm/test/CodeGen/AMDGPU/call-graph-register-usage.ll

185–188

Ditto for the rest

[abinavpp]

• Rebase
• Updated the tests (GlobalISel/)non-entry-alloca.ll, recursion.ll, hsa-metadata-resource-usage-function-ordering.ll to check the dynamic stack

[abinavpp]

llvm/test/CodeGen/AMDGPU/call-graph-register-usage.ll

185–188

There's a check for is_dynamic_callstack from v2 here

[abinavpp]

• Get the new scratch reporting specific to v5+ as per internal discussion
• Not removing the -amdgpu-assume-* options now; Might need this when we run into problems

[arsenm]

llvm/docs/AMDGPUUsage.rst

3558–3561

This sentence doesn't read right at all. I don't know why this would be mentioned here and not just in the table with the description of the field

[abinavpp]

llvm/docs/AMDGPUUsage.rst

3558–3561

In amdgpu-amdhsa-kernel-descriptor-v3-table? We could do that; It's under the section "Code Object V3 Kernel Descriptor", can we add V5+ details there?

[arsenm]

llvm/docs/AMDGPUUsage.rst

3558–3561

Can mention the poor support for unknown sized stacks

[abinavpp]

Moving the doc change to amdgpu-amdhsa-kernel-descriptor-v3-table; I think we
should remove the "V3" bit from that section since the kernel descriptor applies
for later code object versions

llvm/docs/AMDGPUUsage.rst

.. warning::
   Code object V5 is not the default code object version emitted by this version
   of LLVM.
 
 
 Code object V5 metadata is the same as
 :ref:`amdgpu-amdhsa-code-object-metadata-v4` with the changes defined in table
 :ref:`amdgpu-amdhsa-code-object-metadata-map-table-v5` and table
 :ref:`amdgpu-amdhsa-code-object-kernel-argument-metadata-map-table-v5`.
 
   .. table:: AMDHSA Code Object V5 Metadata Map Changes
      :name: amdgpu-amdhsa-code-object-metadata-map-table-v5
 
      ================= ============== ========= =======================================
      String Key        Value Type     Required? Description
      ================= ============== ========= =======================================
      "amdhsa.version"  sequence of    Required  - The first integer is the major
                        2 integers                 version. Currently 1.
                                                 - The second integer is the minor
                                                   version. Currently 2.
      ================= ============== ========= =======================================
31:0    4 bytes GROUP_SEGMENT_FIXED_SIZE        The amount of fixed local
                                                      include any dynamically
                                                      allocated local address
                                                      space memory that may be
                                                      added when the kernel is
                                                      dispatched.
      63:32   4 bytes PRIVATE_SEGMENT_FIXED_SIZE      The amount of fixed
                                                      private address space
                                                      memory required for a
-                                                     work-item in bytes.
-                                                     Additional space may need to
-                                                     be added to this value if
-                                                     the call stack has
-                                                     non-inlined function calls.
+                                                     work-item in bytes.  When
+                                                     this cannot be predicted,
+                                                     code object v4 and older
+                                                     sets this value to be
+                                                     higher than the minimum
+                                                     requirement.
      95:64   4 bytes KERNARG_SIZE                    The size of the kernarg
                                                      memory pointed to by the
                                                      AQL dispatch packet. The
                                                      kernarg memory is used to
                                                      pass arguments to the
                                                      kernel.
 
                                                      * If the kernarg pointer in

llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp

 using namespace llvm;
 using namespace llvm::AMDGPU;
 
 #define DEBUG_TYPE "amdgpu-resource-usage"
 
 char llvm::AMDGPUResourceUsageAnalysis::ID = 0;
 char &llvm::AMDGPUResourceUsageAnalysisID = AMDGPUResourceUsageAnalysis::ID;
 
-// We need to tell the runtime some amount ahead of time if we don't know the
-// true stack size. Assume a smaller number if this is only due to dynamic /
-// non-entry block allocas.
+// In code object v4 and older, we need to tell the runtime some amount ahead of
+// time if we don't know the true stack size. Assume a smaller number if this is
+// only due to dynamic / non-entry block allocas.
 static cl::opt<uint32_t> AssumedStackSizeForExternalCall(
     "amdgpu-assume-external-call-stack-size",
     cl::desc("Assumed stack use of any external call (in bytes)"), cl::Hidden,
     cl::init(16384));
 
 static cl::opt<uint32_t> AssumedStackSizeForDynamicSizeObjects(
     "amdgpu-assume-dynamic-stack-object-size",
     cl::desc("Assumed extra stack use if there are any "
bool AMDGPUResourceUsageAnalysis::runOnModule(Module &M) {
 
   MachineModuleInfo &MMI = getAnalysis<MachineModuleInfoWrapperPass>().getMMI();
   const TargetMachine &TM = TPC->getTM<TargetMachine>();
   bool HasIndirectCall = false;
 
   CallGraph CG = CallGraph(M);
   auto End = po_end(&CG);
 
+  // By default, for code object v5 and later, track only the minimum scratch
+  // size
+  if (AMDGPU::getAmdhsaCodeObjectVersion() >= 5) {
+    if (!AssumedStackSizeForDynamicSizeObjects.getNumOccurrences())
+      AssumedStackSizeForDynamicSizeObjects = 0;
+    if (!AssumedStackSizeForExternalCall.getNumOccurrences())
+      AssumedStackSizeForExternalCall = 0;
+  }
+
   for (auto IT = po_begin(&CG); IT != End; ++IT) {
     Function *F = IT->getFunction();
     if (!F || F->isDeclaration())
       continue;
 
     MachineFunction *MF = MMI.getMachineFunction(*F);
     assert(MF && "function must have been generated already");
 

llvm/test/CodeGen/AMDGPU/call-graph-register-usage.ll

 ; RUN: llc -mtriple=amdgcn-amd-amdhsa --amdhsa-code-object-version=2 -enable-ipra=0 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,CI %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa --amdhsa-code-object-version=5 -enable-ipra=0 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN-V5 %s
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa --amdhsa-code-object-version=2 -mcpu=fiji -enable-ipra=0 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,VI,VI-NOBUG %s
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa --amdhsa-code-object-version=2 -mcpu=iceland -enable-ipra=0 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,VI,VI-BUG %s
 
 ; Make sure to run a GPU with the SGPR allocation bug.
 
 ; GCN-LABEL: {{^}}use_vcc:
 ; GCN: ; NumSgprs: 34
 ; GCN: ; NumVgprs: 0

 
 
 declare void @external() #0
 
 ; GCN-LABEL: {{^}}usage_external:
 ; GCN: is_dynamic_callstack = 1
 ; NumSgprs: 48
 ; NumVgprs: 24
 ; GCN: ScratchSize: 16384
+;
+; GCN-V5-LABEL: {{^}}usage_external:
+; GCN-V5: ScratchSize: 0
 define amdgpu_kernel void @usage_external() #0 {
   call void @external()
   ret void
 }
 
 declare void @external_recurse() #2
 
 ; GCN-LABEL: {{^}}usage_external_recurse:
 ; GCN: is_dynamic_callstack = 1
 ; NumSgprs: 48
 ; NumVgprs: 24
 ; GCN: ScratchSize: 16384
+;
+; GCN-V5-LABEL: {{^}}usage_external_recurse:
+; GCN-V5: ScratchSize: 0
 define amdgpu_kernel void @usage_external_recurse() #0 {
   call void @external_recurse()
   ret void
 }
 
 ; GCN-LABEL: {{^}}direct_recursion_use_stack:
 ; GCN: ScratchSize: 18448{{$}}
+;
+; GCN-V5-LABEL: {{^}}direct_recursion_use_stack:
+; GCN-V5: ScratchSize: 2064{{$}}
 define void @direct_recursion_use_stack(i32 %val) #2 {
   %alloca = alloca [512 x i32], align 4, addrspace(5)
   call void asm sideeffect "; use $0", "v"([512 x i32] addrspace(5)* %alloca) #0
   %cmp = icmp eq i32 %val, 0
   br i1 %cmp, label %ret, label %call
 
 call:
   %val.sub1 = sub i32 %val, 1
   call void @direct_recursion_use_stack(i32 %val.sub1)
   br label %ret
 
 ret:
   ret void
 }
 
 ; GCN-LABEL: {{^}}usage_direct_recursion:
 ; GCN: is_ptr64 = 1
 ; GCN: is_dynamic_callstack = 1
 ; GCN: workitem_private_segment_byte_size = 18448{{$}}
+;
+; GCN-V5-LABEL: {{^}}usage_direct_recursion:
+; GCN-V5: .amdhsa_private_segment_fixed_size 2064{{$}}
 define amdgpu_kernel void @usage_direct_recursion(i32 %n) #0 {
   call void @direct_recursion_use_stack(i32 %n)
   ret void
 }
 
 ; Make sure there's no assert when a sgpr96 is used.
 ; GCN-LABEL: {{^}}count_use_sgpr96_external_call
 ; GCN: ; sgpr96 s[{{[0-9]+}}:{{[0-9]+}}]

llvm/test/CodeGen/AMDGPU/non-entry-alloca.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck -check-prefixes=DEFAULTSIZE,MUBUF %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs -amdhsa-code-object-version=5 < %s | FileCheck -check-prefixes=DEFAULTSIZE-V5,MUBUF %s
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs -amdgpu-assume-dynamic-stack-object-size=1024 < %s | FileCheck -check-prefixes=ASSUME1024,MUBUF %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs -amdgpu-assume-dynamic-stack-object-size=1024 -amdhsa-code-object-version=5 < %s | FileCheck -check-prefixes=ASSUME1024,MUBUF %s
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs -mattr=+enable-flat-scratch < %s | FileCheck -check-prefixes=DEFAULTSIZE,FLATSCR %s
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs -mattr=+enable-flat-scratch -amdgpu-assume-dynamic-stack-object-size=1024 < %s | FileCheck -check-prefixes=ASSUME1024,FLATSCR %s
 
 ; FIXME: Generated test checks do not check metadata at the end of the
 ; function, so this also includes manually added checks.
 
 ; Test that we can select a statically sized alloca outside of the
 ; entry block.
bb.1:
   br label %bb.2
 
 bb.2:
   store volatile i32 0, i32 addrspace(1)* undef
   ret void
 }
 ; DEFAULTSIZE: .amdhsa_private_segment_fixed_size 4112
 ; DEFAULTSIZE: ; ScratchSize: 4112
+; DEFAULTSIZE-V5: .amdhsa_private_segment_fixed_size 16
+; DEFAULTSIZE-V5: .amdhsa_uses_dynamic_stack 1
+; DEFAULTSIZE-V5: ; ScratchSize: 16
 
 ; ASSUME1024: .amdhsa_private_segment_fixed_size 1040
 ; ASSUME1024: ; ScratchSize: 1040
 
 define amdgpu_kernel void @kernel_non_entry_block_static_alloca_uniformly_reached_align64(i32 addrspace(1)* %out, i32 %arg.cond, i32 %in) {
 ; MUBUF-LABEL: kernel_non_entry_block_static_alloca_uniformly_reached_align64:
 ; MUBUF:       ; %bb.0: ; %entry
 ; MUBUF-NEXT:    s_load_dwordx2 s[6:7], s[4:5], 0x8

 
 bb.1:
   store volatile i32 0, i32 addrspace(1)* undef
   ret void
 }
 
 ; DEFAULTSIZE: .amdhsa_private_segment_fixed_size 4160
 ; DEFAULTSIZE: ; ScratchSize: 4160
+; DEFAULTSIZE-V5: .amdhsa_private_segment_fixed_size 64
+; DEFAULTSIZE-V5: .amdhsa_uses_dynamic_stack 1
+; DEFAULTSIZE-V5: ; ScratchSize: 64
 
 ; ASSUME1024: .amdhsa_private_segment_fixed_size 1088
 ; ASSUME1024: ; ScratchSize: 1088
 
 
 define void @func_non_entry_block_static_alloca_align4(i32 addrspace(1)* %out, i32 %arg.cond0, i32 %arg.cond1, i32 %in) {
 ; MUBUF-LABEL: func_non_entry_block_static_alloca_align4:
 ; MUBUF:       ; %bb.0: ; %entry

llvm/test/CodeGen/AMDGPU/recursion.ll

 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -verify-machineinstrs --amdhsa-code-object-version=5 < %s | FileCheck -check-prefixes=V5 %s
 
 ; CHECK-LABEL: {{^}}recursive:
 ; CHECK: ScratchSize: 16
 define void @recursive() {
   call void @recursive()
   store volatile i32 0, i32 addrspace(1)* undef
   ret void
 }

 ; CHECK-LABEL: {{^}}tail_recursive_with_stack:
 define void @tail_recursive_with_stack() {
   %alloca = alloca i32, addrspace(5)
   store volatile i32 0, i32 addrspace(5)* %alloca
   tail call void @tail_recursive_with_stack()
   ret void
 }
 
-; For an arbitrary recursive call, report a large number for unknown stack usage.
+; For an arbitrary recursive call, report a large number for unknown stack
+; usage for code object v4 and older
 ; CHECK-LABEL: {{^}}calls_recursive:
 ; CHECK: .amdhsa_private_segment_fixed_size 16400{{$}}
+;
+; V5-LABEL: {{^}}calls_recursive:
+; V5: .amdhsa_private_segment_fixed_size 0{{$}}
 define amdgpu_kernel void @calls_recursive() {
   call void @recursive()
   ret void
 }
 
 ; Make sure we do not report a huge stack size for tail recursive
 ; functions
 ; CHECK-LABEL: {{^}}kernel_indirectly_calls_tail_recursive:
 ; CHECK: .amdhsa_private_segment_fixed_size 0{{$}}
 define amdgpu_kernel void @kernel_indirectly_calls_tail_recursive() {
   call void @calls_tail_recursive()
   ret void
 }
 
 ; TODO: Even though tail_recursive is only called as a tail call, we
 ; end up treating it as generally recursive call from the regular call
 ; in the kernel.
 
 ; CHECK-LABEL: {{^}}kernel_calls_tail_recursive:
 ; CHECK: .amdhsa_private_segment_fixed_size 16384{{$}}
+;
+; V5-LABEL: {{^}}kernel_calls_tail_recursive:
+; V5: .amdhsa_private_segment_fixed_size 0{{$}}
 define amdgpu_kernel void @kernel_calls_tail_recursive() {
   call void @tail_recursive()
   ret void
 }
 
 ; CHECK-LABEL: {{^}}kernel_calls_tail_recursive_with_stack:
 ; CHECK: .amdhsa_private_segment_fixed_size 16384{{$}}
+;
+; V5-LABEL: {{^}}kernel_calls_tail_recursive_with_stack:
+; V5: .amdhsa_private_segment_fixed_size 8{{$}}
 define amdgpu_kernel void @kernel_calls_tail_recursive_with_stack() {
   call void @tail_recursive_with_stack()
   ret void
 }