[PATCH] D123346: AMDGPU: Align the implicit kernel argument segment to 8 bytes for v5

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp:804
 
+  Offset = alignTo(Offset, ST.getAlignmentForImplicitArgPtr());
+
----------------
cfang wrote:
> arsenm wrote:
> > This isn't the real alignment. You should assert the alignment you compute using the kernarg base alignment is at least getAlignmentForImplicitArgPtr
> I think this is to align Offset to ST.getAlignmentForImplicitArgPtr which is 8 bytes.
> Offset = alignTo(Offset, ST.getAlignmentForImplicitArgPtr());
> 
> We do need this alignment to be in sync with the actual load of the implicit kernel argument.
You do not want to use alignTo. You are not adjusting the alignment or offset here. You want to use commonAlignment(16, Offset), and assert this is >= ST.getAlignmentForImplicitArgPtr()


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

https://reviews.llvm.org/D123346