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

Pavel Kopyl via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Wed Jan 25 09:02:53 PST 2023

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?

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list