[Openmp-commits] [PATCH] D142508: [OpenMP][libomptarget] Fix alignment calculation for mapping struct members.

[Pavel Kopyl via Phabricator via Openmp-commits]

pavelkopyl added a comment.

In D142508#4078596 <https://reviews.llvm.org/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.

Thank you for clarification. I understand that we need to care about alignment of partially mapped structures on a device side. As I see, the only small issue here is in calculation of Padding value and updating mapping begin address:

  HstPtrBegin = (char *)HstPtrBegin - Padding; 

In case of a 4-aligned structure that has two integers, we may get, which is wrong I guess. This patch just limits Padding value. If padded begin address is less that HstPtrBase, we just need to map starting from beginning of the structure. Does this make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142508