[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) {
----------------
rampitec wrote:
> 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.
> > 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.

I have checked, SDag only calls refineAlignment when it folds a node or for a mem intrinsic, which does not happen with the tests I am using. I hit zero bps in refineAlignment.


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

https://reviews.llvm.org/D104316