[PATCH] D155190: [amdgpu][lds] Remove recalculation of LDS frame from backend

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:48
+  // pass is disabled. If graphics does not use dynamic LDS, this is never
+  // profitable. Leaving cleanup for a later change.
+  LDSSize = F.getFnAttributeAsParsedInteger("amdgpu-lds-size", 0);
----------------
arsenm wrote:
> Not sure exactly what two variables or "profitable" this is talking about 
The LDSSize and StaticLDSSize immediately below the comment. I'm pretty sure we don't need both of them. The distinction is meant to elide some padding in some edge cases, but  I don't think that edge case exists any more. I'm having some trouble working out exactly how graphics use LDS.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:76
+    std::optional<uint32_t> MaybeAbs = getLDSAbsoluteAddress(GV);
+    if (MaybeAbs) {
+      // Absolute address LDS variables that exist prior to the LDS lowering
----------------
arsenm wrote:
> If we did the same thing, could probably fix the bug with amdgpu-gds-size not interacting well with globals
Seems likely that gds lowering would work well with absolute addresses, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155190