[PATCH] D150609: [AMDGPU] Do not assume stack size for PAL code object indirect calls

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:115-117
+  if ((STI.getTargetTriple().getOS() == Triple::AMDPAL ||
+       AMDGPU::getCodeObjectVersion(M) >= AMDGPU::AMDHSA_COV5) &&
+      !AssumedStackSizeForExternalCall.getNumOccurrences())
----------------
Should use explicit brackets around the && clause.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp:120-122
+  if (AMDGPU::getCodeObjectVersion(M) >= AMDGPU::AMDHSA_COV5 &&
+      !AssumedStackSizeForDynamicSizeObjects.getNumOccurrences())
+      AssumedStackSizeForDynamicSizeObjects = 0;
----------------
arsenm wrote:
> Dynamic objects should be treated identically 
I think the logic here is that we treat PAL code objects the same as AMDHSL code objects >= 5 (PAL code objects have no amdgpu_code_object_version).
If there is an indirect call, something outside the compiler needs to compute the stack size, in the case of PAL/graphics, that is the driver.

So probably we should use `if (AMDGPU::getCodeObjectVersion(M) >= AMDGPU::AMDHSA_COV5 || STI.getTargetTriple().getOS() == Triple::AMDPAL)` for both, `AssumedStackSizeForDynamicSizeObjects` and `AssumedStackSizeForExternalCall`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150609