[PATCH] D103261: [AMDGPU] Fix natural alignment of LDS globals during LDS lowering.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Do we actually see underaligned LDS variables in practice? What if the user is forcing a lower alignment to prioritize space utilization?



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:180-181
+    for (auto *GV : FoundLocalVars) {
+      Align Alignment = AMDGPU::getAlign(DL, GV);
+      Align FixedAlignment = AMDGPU::fixAlignment(GV, Alignment, DL);
+      if (Alignment != FixedAlignment) {
----------------
Separating these functions is just making this harder to follow. How about just one function to fully inspect the GV and increase alignment


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:153
+// Fix natural alignment for LDS global `GV` based on its size.
+Align fixAlignment(const GlobalVariable *GV, Align Alignment,
+                   const DataLayout &DL) {
----------------
"Fix" isn't very descriptive. How about increaseAlignment?


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPULDSUtils.cpp:155
+                   const DataLayout &DL) {
+  TypeSize GVSize = DL.getTypeAllocSize(GV->getValueType());
+
----------------
This is ignoring the explicit alignment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103261