[PATCH] D155190: [amdgpu][lds] Remove recalculation of LDS frame from backend
Jon Chesterfield via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 13 15:54:09 PDT 2023
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
More information about the llvm-commits
mailing list