[PATCH] D104316: [AMDGPU] Propagate LDS align into to instructions

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked 3 inline comments as done.
rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:324
+      uint64_t Off = DL.getStructLayout(LDSTy)->getElementOffset(I);
+      Align A = commonAlignment(MaxAlign, Off);
+      refineUsesAlignment(GEP, A, DL);
----------------
hsmhsm wrote:
> Did not understand the logic behind calling commonAlignment() here. I thought, alignment of GV and GEP are same, and we just need to propogate the alignment of GV? Because, we already have properly updated the alignment of GV?
> Did not understand the logic behind calling commonAlignment() here. I thought, alignment of GV and GEP are same, and we just need to propogate the alignment of GV? Because, we already have properly updated the alignment of GV?

Assume struct:
```
{ i64 x 16, i32 }
```
Second field has align 4 and that is the alignment of GV and GEP. But given the structure layout we can tell that actual efective alignment is 8. That is what commonAlignment() call is about.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:374
+          Align GA = commonAlignment(A, Off.getLimitedValue());
+          refineUsesAlignment(GEP, GA, DL, MaxDepth - 1);
+        }
----------------
hsmhsm wrote:
> In case of GEP instruction, I thought, we should be exploring the uses of GEP?
> In case of GEP instruction, I thought, we should be exploring the uses of GEP?

Yes, this is recursive call to refineUsesAlignment() with GEP as a Ptr.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:381
+            I->getOpcode() == Instruction::AddrSpaceCast)
+          refineUsesAlignment(I, A, DL, MaxDepth - 1);
+      }
----------------
hsmhsm wrote:
> Same here, in case of bitcast instruction, I thought, we should be exploring uses of bitcast?
> Same here, in case of bitcast instruction, I thought, we should be exploring uses of bitcast?

Same here, recursive call explores uses of a cast.


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

https://reviews.llvm.org/D104316