[PATCH] D102401: [AMDGPU] Allocate LDS globals in sorted order of their alignment and size.

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D102401#2759276 <https://reviews.llvm.org/D102401#2759276>, @hsmhsm wrote:

> In D102401#2757441 <https://reviews.llvm.org/D102401#2757441>, @rampitec wrote:
>
>> We probably need a test which only runs this lowering and showing a resulting transformed IR. Test the alignment, sorting and situation with multiple kernels using intersecting LDS variable sets.
>
> Added test - lds-allocation.ll.  However, note that we actually do not update the alignment of LDS globals within the module,  during allocation, we only just compute the required natural alignment for proper allocation.

I mean an IR test where we can see what lowering has done, not just total allocation amount.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:83
+
+SmallVector<const GlobalVariable *, 8>
+getSortedUsedLDSGlobals(const Function *K) {
----------------
SmallVectorImpl<>


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUMachineFunction.cpp:61
+    const auto *I = dyn_cast<Instruction>(UU);
+    assert(I && "Expected an instruction");
+    const Function *F = I->getFunction();
----------------
hsmhsm wrote:
> rampitec wrote:
> > It can probably be Operator.
> I did not understand this comment.
llvm::Operator.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:115
+  SmallVector<const GlobalVariable *, 8> StaticLDSGlobals =
+      collectStaticLDSGlobals(&M);
+
----------------
Maybe just pass a filter lambda there?


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.h:34
 
+SmallVector<const GlobalVariable *, 8> collectStaticLDSGlobals(const Module *M);
+
----------------
SmallVectorImpl<>.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102401