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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:339
+
+  void refineUsesAlignment(Value *Ptr, Align A, const DataLayout &DL,
+                       unsigned MaxDepth = 5) {
----------------
arsenm wrote:
> arsenm wrote:
> > This looks like you are reinventing getOrEnforceKnownAlignment
> I'm not sure why you need to do this. SelectionDAG does already try to increase the alignment after this point
> This looks like you are reinventing getOrEnforceKnownAlignment

It is not the same. I do not need to enforce alignment on an Value itself but propagate it down to loads and stores. I did not find such helper.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULowerModuleLDSPass.cpp:339
+
+  void refineUsesAlignment(Value *Ptr, Align A, const DataLayout &DL,
+                       unsigned MaxDepth = 5) {
----------------
rampitec wrote:
> arsenm wrote:
> > arsenm wrote:
> > > This looks like you are reinventing getOrEnforceKnownAlignment
> > I'm not sure why you need to do this. SelectionDAG does already try to increase the alignment after this point
> > This looks like you are reinventing getOrEnforceKnownAlignment
> 
> It is not the same. I do not need to enforce alignment on an Value itself but propagate it down to loads and stores. I did not find such helper.
> I'm not sure why you need to do this. SelectionDAG does already try to increase the alignment after this point

It actually does not, at least not in this scenario. You may notice there are actual codegen changes. I was checking -print-after-all and alignment stays the same all the way past selection. Besides there is also global isel. I assume the earlier we get better alignment the better.


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

https://reviews.llvm.org/D104316