[PATCH] D103261: [AMDGPU] Increase alignment of LDS globals if necessary before LDS lowering.

[Mahesha S via Phabricator via llvm-commits]

hsmhsm marked 2 inline comments as done.
hsmhsm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:178
+    for (auto *GV : FoundLocalVars) {
+      unsigned AlignValue = GV->getAlignment();
+      if (AlignValue == 0) {
----------------
foad wrote:
> This should really be using getAlign. getAlignment has a FIXME comment saying it will be removed.
> 
> You should use the Align or MaybeAlign types throughout instead of unsigned.
I was using getAlign only, but l had to change it based on one of review comments, I think. Anyway, I will fix it in a new patch that I am planning to submit soon, to introduce a command line flag for this transformation.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:181
+        GV->setAlignment(DL.getABITypeAlign(GV->getValueType()));
+        continue;
+      }
----------------
foad wrote:
> Why continue here? Surely it's better to fall through into the code that increases alignment?
Will fix it in new patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103261