[Openmp-commits] [PATCH] D149685: [OpenMP] libomptarget: Don't map alignment padding to host

[George Rokos via Phabricator via Openmp-commits]

grokos added a comment.

I understand what you are trying to achieve here and indeed this patch does exactly what you want to. But my question is, do we really want to allow this behavior? The test case which was added to demonstrate what problem this patch fixes looks illegal from an OpenMP perspective. It is a case of "explicitly mapping more" of the same object, which is forbidden in OpenMP. I wasn't sure whether that restriction has been relaxed and it slipped under my radar, but the latest TR11 spec still says about `map` clauses (https://www.openmp.org/wp-content/uploads/openmp-TR11.pdf, pg. 159, lines 29-32):

> If a list item is an element of a structure, and a different element of the structure has a
> corresponding list item in the device data environment prior to a task encountering the
> construct associated with the map clause, then the list item must also have a corresponding
> list item in the device data environment prior to the task encountering the construct.

In other words, if you want to map a struct member via a later `map` clause and other members of the same struct have been mapped via an earlier `map` clause, then the former member should also have been mapped via the earlier `map` clause.

If there is a valid scenario that doesn't work without this patch then:

1. The valid scenario should be added as a new test case to highlight what this patch fixes
2. Modify this patch so that the illegal scenario in `padding_not_mapped.c` still fails, as libomptarget correctly detects the attempt to "explicitly map more of the same struct"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149685