[all-commits] [llvm/llvm-project] 002d61: [OpenMP] Fix `present` for exit from `omp target d...

[Joel E. Denny via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 002d61db2b7790dc884953bf9271878bf0af3a8e
      https://github.com/llvm/llvm-project/commit/002d61db2b7790dc884953bf9271878bf0af3a8e
  Author: Joel E. Denny <jdenny.ornl at gmail.com>
  Date:   2020-08-05 (Wed, 05 Aug 2020)

  Changed paths:
    M clang/lib/CodeGen/CGOpenMPRuntime.cpp
    M clang/lib/CodeGen/CGOpenMPRuntime.h
    M clang/lib/CodeGen/CGStmtOpenMP.cpp
    M clang/test/OpenMP/target_data_codegen.cpp
    M openmp/libomptarget/src/omptarget.cpp
    A openmp/libomptarget/test/mapping/present/target_data_at_exit.c

  Log Message:
  -----------
  [OpenMP] Fix `present` for exit from `omp target data`

Without this patch, the following example fails but shouldn't
according to OpenMP TR8:

```
 #pragma omp target enter data map(alloc:i)
 #pragma omp target data map(present, alloc: i)
 {
   #pragma omp target exit data map(delete:i)
 } // fails presence check here
```

OpenMP TR8 sec. 2.22.7.1 "map Clause", p. 321, L23-26 states:

> If the map clause appears on a target, target data, target enter
> data or target exit data construct with a present map-type-modifier
> then on entry to the region if the corresponding list item does not
> appear in the device data environment an error occurs and the
> program terminates.

There is no corresponding statement about the exit from a region.
Thus, the `present` modifier should:

1. Check for presence upon entry into any region, including a `target
   exit data` region.  This behavior is already implemented correctly.

2. Should not check for presence upon exit from any region, including
   a `target` or `target data` region.  Without this patch, this
   behavior is not implemented correctly, breaking the above example.

In the case of `target data`, this patch fixes the latter behavior by
removing the `present` modifier from the map types Clang generates for
the runtime call at the end of the region.

In the case of `target`, we have not found a valid OpenMP program for
which such a fix would matter.  It appears that, if a program can
guarantee that data is present at the beginning of a `target` region
so that there's no error there, that data is also guaranteed to be
present at the end.  This patch adds a comment to the runtime to
document this case.

Reviewed By: grokos, RaviNarayanaswamy, ABataev

Differential Revision: https://reviews.llvm.org/D84422


  Commit: 26cf9c17044515cdde3e7baeea843001ba33be59
      https://github.com/llvm/llvm-project/commit/26cf9c17044515cdde3e7baeea843001ba33be59
  Author: Joel E. Denny <jdenny.ornl at gmail.com>
  Date:   2020-08-05 (Wed, 05 Aug 2020)

  Changed paths:
    M clang/docs/OpenMPSupport.rst

  Log Message:
  -----------
  [OpenMP][Docs] Add map clause reordering status as unclaimed


  Commit: 03bb545b68c2edb9dc5bd092104bdb83a8e5e347
      https://github.com/llvm/llvm-project/commit/03bb545b68c2edb9dc5bd092104bdb83a8e5e347
  Author: Joel E. Denny <jdenny.ornl at gmail.com>
  Date:   2020-08-05 (Wed, 05 Aug 2020)

  Changed paths:
    M clang/docs/OpenMPSupport.rst

  Log Message:
  -----------
  [OpenMP][Docs] Mark `present` map type modifier as done


  Commit: 5ab43989c353a2378910d20c7b88e44ea92b3aee
      https://github.com/llvm/llvm-project/commit/5ab43989c353a2378910d20c7b88e44ea92b3aee
  Author: Joel E. Denny <jdenny.ornl at gmail.com>
  Date:   2020-08-05 (Wed, 05 Aug 2020)

  Changed paths:
    M openmp/libomptarget/src/device.cpp
    M openmp/libomptarget/src/device.h
    M openmp/libomptarget/src/omptarget.cpp
    A openmp/libomptarget/test/mapping/present/target_update_array_extension.c
    A openmp/libomptarget/test/mapping/target_update_array_extension.c

  Log Message:
  -----------
  [OpenMP] Fix `omp target update` for array extension

OpenMP TR8 sec. 2.15.6 "target update Construct", p. 183, L3-4 states:

> If the corresponding list item is not present in the device data
> environment and there is no present modifier in the clause, then no
> assignment occurs to or from the original list item.

L10-11 states:

> If a present modifier appears in the clause and the corresponding
> list item is not present in the device data environment then an
> error occurs and the program termintates.

(OpenMP 5.0 also has the first passage but without mention of the
present modifier of course.)

In both passages, I assume "is not present" includes the case of
partially but not entirely present.  However, without this patch, the
target update directive misbehaves in this case both with and without
the present modifier.  For example:

```
 #pragma omp target enter data map(to:arr[0:3])
 #pragma omp target update to(arr[0:5]) // might fail on data transfer
 #pragma omp target update to(present:arr[0:5]) // might fail on data transfer
```

The problem is that `DeviceTy::getTgtPtrBegin` does not return a null
pointer in that case, so `target_data_update` sees the data as fully
present, and the data transfer then might fail depending on the target
device.  However, without the present modifier, there should never be
a failure.  Moreover, with the present modifier, there should always
be a failure, and the diagnostic should mention the present modifier.

This patch fixes `DeviceTy::getTgtPtrBegin` to return null when
`target_data_update` is the caller.  I'm wondering if it should do the
same for more callers.

Reviewed By: grokos, jdoerfert

Differential Revision: https://reviews.llvm.org/D85246


Compare: https://github.com/llvm/llvm-project/compare/d21ce4082181...5ab43989c353