[PATCH] D82496: [amdgpu] Add codegen support for HIP dynamic shared memory.

[Michael Liao via Phabricator via llvm-commits]

hliao added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:59
+  // memory.
+  LDSSize = alignTo(StaticLDSSize, DynLDSAlign);
 
----------------
arsenm wrote:
> hliao wrote:
> > arsenm wrote:
> > > This is an independent field and should not be changed here
> > As the sequence of static LDS allocation and dynamic LDS alignment updates are processed in the program order or reverse of that order, we need to collect all static LDS usage and dynamic LDS alignment. As we remove the previous the one single point adjustment, we need to update LDSSizze if there's any static LDS allocation or dynamic LDS alignment updates.
> OK yes, this is in the right place now. However,r it should be where the alignment is updated
As we don't know which one will be processed last, we need to update `LDSSize` in both cases to ensure the correct one is calculated.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5588
+        // Adjust alignment for that dynamic shared memory array.
+        MFI->setDynLDSAlign(DAG.getDataLayout().getABITypeAlign(Ty));
+        return SDValue(
----------------
arsenm wrote:
> This logic should be moved into allocateLDSGlobal
the allocation of dynamic LDS is not handled by the compiler. We only collect the alignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82496