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

[Joel E. Denny via Phabricator via Openmp-commits]

jdenny abandoned this revision.
jdenny added a comment.

In D149685#4332051 <https://reviews.llvm.org/D149685#4332051>, @grokos wrote:

> It is a case of "explicitly mapping more" of the same object, which is forbidden in OpenMP.

Indeed, I misunderstood.  Sorry for the noise, and thanks for explaining.  I'll abandon the patch.

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

I'm not sure there is.

> 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"

For the record, while I designed that test to trigger detection, libomptarget does not always detect such cases.  For example, the second pair of directives in the existing test `openmp/libomptarget/test/mapping/low_alignment.c` seem to be designed to avoid detection.  In other cases, detection depends on the amount of padding on the device, which depends on how the host memory happens to be aligned, which can vary from run to run of the same program.  That is, detection is non-deterministic.

Now that I have a better handle on what's happening, it seems to me that the second pair of directives in `openmp/libomptarget/test/mapping/low_alignment.c` should be removed.  I think that test only passes when all of the following conditions are true, but OpenMP doesn't guarantee them to be true:

1. The runtime doesn't diagnose the restriction violation because `map(from : t.i, t.j)` has too little device padding for `map(from : t.a)` to collide with it.

2. `t.i = 21;` and `t.j = 31;` do not produce memory access violations even though they write to the offsets of `t.i` and `t.j` computed from the base device address of `t` from `map(from : t.a)` and even though those offsets are not guaranteed to be allocated on the device.

3. The desired values for `t.i` and `t.j` are copied back to the host even though `t.i = 21;` and `t.j = 31;` do *not* write to the offset of `t.i` and `t.j` computed from the base device address of `t` from `map(from : t.i, t.j)`.  The reason the correct values are copied back is that the previous kernel wrote the same values to `s.i` and `s.j`, which happened to have the same device addresses.

Agreed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149685