[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

[Artem Belevich via Phabricator via llvm-commits]

tra added a comment.
Herald added a project: All.

In D120129#3350839 <https://reviews.llvm.org/D120129#3350839>, @kovdan01 wrote:

> I suppose that you are correct and we can always set alignment to 16. The reason why I implemented such logic is that I try to be as conservative as possible.



> For example, if we have two values of `[5 x i32]` aligned as 4, they might be placed together without any gaps.

I do not think PTX gives us any guarantees on how parameters are actually arranged. For all we know they may be packed in registers and alignment is completely irrelevant.
I've poked a bit at the SASS generated for various args and alignments and it appears that first few arguments are actually passed via registers. Up to 12 ints are passed via registers on sm_86, Subsequent arguments are passed via local memory.
Here's an example of alignment effect on the argument layout: https://cuda.godbolt.org/z/87Mj1hz4h

In any case, if the code ends up with local memory  loads/stores in the hot path, we can already forget about performance and being able to use vectorized local/stores will not help much.
I think the best we can do here is give ptxas as much flexibility as we can. Setting alignment for return values to 16 should be safe. We rarely return multiple values.

> If we align them as 16 - an additional gap of 12 bytes will appear. If keeping such king of "layout" stable is not important - I'll change the logic so alignment is always 16 in param space.

Larger alignment will potentially waste some of the limited param space, so it's a trade-off between guaranteeing efficient loads/stores of parameters vs not being able to pass very large number of arguments. E.g. if someone were to have a function with hundreds of `[5 x i32]` arguments, we would probably be able to handle more such args if they were aligned by 4.
Efficient loads/stores are arguably much more important as they are very common and affect about everyone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120129