I guess, there is one more reason to getting back to default alignment.It seems we break ABI with alignment forced to 4. This may be an issue for mixed build clang + llvm.

Proper fix is here: https://reviews.llvm.org/D142586

In D142508#4081622, @jdoerfert wrote:

Your approach fails the second scenario in the D142586 test case. The problem is you look at the first member mapped but that is not necessarily the first member mapped later.

Thank you for the patch! I checked it with the D135462 PR that changes alloca alignment - everything works fine.

In D141737#4053125, @tra wrote:

Do we need to do it? I mean -- are there any observable differences between enforced alignment of 4 and not? For register-passed parameters it will make no difference, and for the parameters passed via constant/local memory, it may or may not make a difference.

I guess at the extreme it would impact how many 1-byte parameters we may have, as the total size of param address space has an upper bound (for kernels it's 4K: https://docs.nvidia.com/cuda/cuda-c-programming-guide/#parameter-buffer-layout). If that's the case, then it probably does not matter whether we can pass 4K arguments or only 1K.

In other words, I do not see a practical need to introduce an option which may not have any practical use.

Sorry for long response.
OK, I'll remove the option. I just wanted to clarify, are you agree in general with removing this workaround?

• Limit Padding values

In D142508#4078596, @grokos wrote:

Padding was added to libomptarget in order to ensure 8-byte struct members remain properly aligned, e.g. pointers, doubles etc. If you remove padding, then such a member may find itself in a non 8-aligned address, making its access result in a segfault.

For instance, in the code you provided, add a third struct member k of type double

struct S {
  int i;
  int j;
  double k;
}

and partially map the struct map(s.j, s.k). There are architectures (e.g. CUDA for sure) where memalloc will always return an 8-aligned (or even higher) address. If we don't add a 4-byte padding at the beginning of the struct on the device, j will start at an address of the form 0x...0, then k will find itself at an address of the form 0x...4, i.e. k will not be 8-aligned. Try accessing it on virtually any architecture and you'll get a segfault. By adding 4 dummy bytes at the beginning of the struct, j finds itself at 0x...4 and k at 0x...8, which is what we need to ensure.

In D142508#4078481, @jdoerfert wrote:

Why not add the test?

In D142508#4078505, @grokos wrote:

In D142508#4078497, @pavelkopyl wrote:

https://reviews.llvm.org/D13546

Is that link correct? It points to a very old and seemingly irrelevant patch.

Hi All,

I've prepared a new review where ptxas is run -march=sm_60: https://reviews.llvm.org/D141736

In D140436#4026879, @tra wrote:

Description could use some details. I.e. what makes sm_35 special? I assume it's due to the fact that sm_35 is no longer supported.

Instead of special-casing ptxas-12.0, perhpas a better way is to just always run ptxas with -arch=sm_60 so it would work with ptxas from all reasonably new CUDA versions.

• Rebase

• Fixes according to review notes

• Rebased
• Added hepler function for computing byval param alignments of device functions

• Rebase
• Fixes according to a review notes

• Update to correct patch version.

Example of failing tests: https://lab.llvm.org/buildbot/#/builders/232/builds/2407

-Updated commit message

In D140581#4015109, @ldrumm wrote:

In your commit message:

• s/parametrs/parameters

I'd prefer to split off a separate test that only checks the alignment of various calls, rather than reuse the byval bitcast test. Otherwise LGTM, but I'd wait on someone more familiar with ptx, e.g. ~tra

• Rebased
• Added fixes according to review notes

• Rebased
• Added run of ptxas to the test

• Added fixes according to review comments

In D140238#4002294, @tra wrote:

LGTM. Thank you for the patch.

Just curious, does it buy us any measurable improvements?

• Fix typo in commit msg.

• Removed unneeded comment from vaarg.ll test
• Removed unneeded comment from LowerFormalArguments()

• Rebased
• Added description for getParamSymbol()

Replace fixed %VAParam name with <function>_vararg.

Fix regexp for .align

Move align hardcode to NVPTXSubtarget where it's available via getMaxRequiredAlignment()

Updated vaargs.ll test to make it more clear how vaarg related instructions / intrinsics get lowered.

In D138531#3957954, @tra wrote:

Note that aggregates passed by value as variadic arguments are not currently supported.

What happens when a user does try to pass an aggregate as a var arg?

That will trigger llvm_unreachable() at llvm/lib/CodeGen/ValueTypes.cpp:551
But this is common issue - aggregates are not allowed (at least now) in variadic arguments.

Fix issue after rebasing.