[Patch] libgomp: Fix 'target enter data' with always pointer

[Patch] libgomp: Fix 'target enter data' with always pointer

[Tobias Burnus]

[-- Attachment #1: Type: text/plain, Size: 1943 bytes --]

The problem is that GOMP_MAP_ALWAYS_POINTER, there is a lookup for "i - 1"
but with 'target enter data', GOMP_MAP_ALWAYS_POINTER and its data were passed
as separate entities.

I am not sure whether there is a legitimate reason to have two
GOMP_MAP_ALWAYS_POINTER in a row; the check in gomp_map_vars_internal
seems to indicate that there is. Hence, I assumed there is and I add
an 'i > 0' check to that function and also a check the kinds[i] isn't
an always pointer (if i+ is) in the caller, i.e. GOMP_target_enter_exit_data.

Note that there is a front-end/middle-end issue with regards to 'target exit data',
which is the reason that the exit data has been commented out. I plan to fix this
separately.* (It is a bug of its own - and this fix is to libgomp and the other is
to the FE/ME.)

OK for mainline?

Tobias

(*) Part of the 'alloc' issue has been discussed in the patch:
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604887.html
however, during discussion on IRC it turned out that this patch is incomplete.
This issue is next on my to-do list.


PS: Also *pending* *review* is a simple reverse-offload-only patch and
one '!$omp loop' "13 Regression" fix (with the review comments fixed):

"[v2] OpenMP/Fortran: Fix loop-iter var privatization with !$OMP LOOP [PR108512]"
https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611730.html

"[Patch] libgomp: Fix reverse-offload for GOMP_MAP_TO_PSET"
https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611617.html

(Other pending patches: "OpenMP Patch Ping – including "[13 Regression]",
https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611524.html )
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: omp-fix-always-ptr.diff --]
[-- Type: text/x-patch, Size: 2793 bytes --]

libgomp: Fix 'target enter data' with always pointer

As GOMP_MAP_ALWAYS_POINTER operates on the previous map item, ensure that
with 'target enter data' both are passed together to gomp_map_vars_internal.

libgomp/ChangeLog:

	* target.c (gomp_map_vars_internal): Add 'i > 0' before doing a
	kind check.
	(GOMP_target_enter_exit_data): If the next map item is
	GOMP_MAP_ALWAYS_POINTER map it together with the current item.
        * testsuite/libgomp.fortran/target-enter-data-3.f90: New test.

 target.c                                          |   17 +++++++++++++----
 testsuite/libgomp.fortran/target-enter-data-3.f90 |   22 ++++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/libgomp/target.c b/libgomp/target.c
index c1682caea13..cc8db85957c 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -1480,8 +1480,9 @@ gomp_map_vars_internal (struct gomp_device_descr *devicep,
 		    gomp_mutex_unlock (&devicep->lock);
 		    gomp_fatal ("always pointer not mapped");
 		  }
-		if ((get_kind (short_mapkind, kinds, i - 1) & typemask)
-		    != GOMP_MAP_ALWAYS_POINTER)
+		if (i > 0
+		    && ((get_kind (short_mapkind, kinds, i - 1) & typemask)
+			!= GOMP_MAP_ALWAYS_POINTER))
 		  cur_node.tgt_offset = gomp_map_val (tgt, hostaddrs, i - 1);
 		if (cur_node.tgt_offset)
 		  cur_node.tgt_offset -= sizes[i];
@@ -4085,7 +4086,10 @@ GOMP_target_enter_exit_data (int device, size_t mapnum, void **hostaddrs,
 			 GOMP_MAP_VARS_ENTER_DATA);
 	  i += j - i - 1;
 	}
-      else if (i + 1 < mapnum && (kinds[i + 1] & 0xff) == GOMP_MAP_ATTACH)
+      else if (i + 1 < mapnum
+	       && ((kinds[i + 1] & 0xff) == GOMP_MAP_ATTACH
+		   || ((kinds[i + 1] & 0xff) == GOMP_MAP_ALWAYS_POINTER
+		       && (kinds[i] & 0xff) != GOMP_MAP_ALWAYS_POINTER)))
 	{
 	  /* An attach operation must be processed together with the mapped
 	     base-pointer list item.  */
diff --git a/libgomp/testsuite/libgomp.fortran/target-enter-data-3.f90 b/libgomp/testsuite/libgomp.fortran/target-enter-data-3.f90
new file mode 100644
index 00000000000..5d97566c66c
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/target-enter-data-3.f90
@@ -0,0 +1,22 @@
+implicit none
+type t
+  integer :: dummy
+  integer, pointer :: p1(:), p2(:)
+  integer :: dummy2
+end type t
+type(t) :: var
+integer :: i
+allocate(var%p1(5),var%p2(2:4))
+var%p1 = [22,53,28,6,4]
+var%p2 = [46,679,54]
+
+!$omp target enter data map(to:var%p1, var%p2)
+!$omp target
+  if (.not.associated(var%p1).or.lbound(var%p1,1)/=1.or.ubound(var%p1,1)/=5) stop 1
+  if (.not.associated(var%p2).or.lbound(var%p2,1)/=2.or.ubound(var%p2,1)/=4) stop 2
+  if (any (var%p1 /= [22,53,28,6,4])) stop 3
+  if (any (var%p2 /= [46,679,54])) stop 4
+!$omp end target
+!!$omp target exit data map(from:var%p1, var%p2)
+end
+

Re: [Patch] libgomp: Fix 'target enter data' with always pointer

[Jakub Jelinek]

On Mon, Feb 13, 2023 at 09:28:15PM +0100, Tobias Burnus wrote:
> libgomp: Fix 'target enter data' with always pointer
> 
> As GOMP_MAP_ALWAYS_POINTER operates on the previous map item, ensure that
> with 'target enter data' both are passed together to gomp_map_vars_internal.
> 
> libgomp/ChangeLog:
> 
> 	* target.c (gomp_map_vars_internal): Add 'i > 0' before doing a
> 	kind check.
> 	(GOMP_target_enter_exit_data): If the next map item is
> 	GOMP_MAP_ALWAYS_POINTER map it together with the current item.
>         * testsuite/libgomp.fortran/target-enter-data-3.f90: New test.

8 spaces instead of tab, this won't get through the git pre-commit hook.

Otherwise LGTM.

	Jakub