Paul-Antoine Arras wrote:
Regarding (1), the OpenMP spec has the following restriction: "If multiple list
items are explicitly mapped on the same construct and have the same containing
array or have base pointers that share original storage, and if any of the list
items do not have corresponding list items that are present in the device data
environment prior to a task encountering the construct, then the list items must
refer to *the same array elements* of either the containing array or the
implicit array of the base pointers."
Because tiles is allocatable, we cannot prove at compile time that array
elements are the same, so the check is deferred to libgomp. But there the
condition enforcing that all addresses are the same is too strict, so this patch
relaxes it to only check that addresses are sorted in increasing order.
Yes, better false negative than false positives when doing error checks.
For allocatables (for which deep mapping applies), they are contiguous
and the code walks them increasing index/memory position.
The OpenMP spec allows (2) as long as it is implicit, without extending the
original mapping. So this patch sets the GOMP_MAP_IMPLICIT flag appropriately
on deep maps at compile time to let libgomp know that it is fine.
PR fortran/120505
gcc/ChangeLog:
* omp-low.cc (lower_omp_target): Set GOMP_MAP_IMPLICIT flag.
libgomp/ChangeLog:
* target.c (gomp_map_vars_internal): Allow struct mapping from different
containing array elements as long as adresses are in increasing order.
* testsuite/libgomp.c-c++-common/map-arrayofstruct-2.c: Adjust
dg-output.
* testsuite/libgomp.c-c++-common/map-arrayofstruct-3.c: Likewise.
* testsuite/libgomp.fortran/map-subarray-5.f90: Likewise.
* testsuite/libgomp.fortran/map-subarray-10.f90: New test.
* testsuite/libgomp.fortran/map-subarray-9.f90: New test.
(The two testcases differ by one using tiles(1)%...and tiles(1)%...
the other tiles(1)%..., tiles(2)%...)
* * *
--- a/gcc/omp-low.cc
+++ b/gcc/omp-low.cc
@@ -13240,7 +13240,20 @@ lower_omp_target (gimple_stmt_iterator *gsi_p,
omp_context *ctx)
- case OMP_CLAUSE_MAP: tkind2 = OMP_CLAUSE_MAP_KIND (c); break;
+ case OMP_CLAUSE_MAP:
For bystanders: This for the deep mappying code only → if (deep_map_cnt)
and lang_hooks.decls.omp_deep_mapping.
* * *
+ gomp_fatal ("Mapped array elements must be in "
+ "increasing address order (got %p > %p)",
+ hostaddrs[j], hostaddrs[j + 1]);
Shouldn't this be "must be the same or in increasing …" to match the
check and also to match the specification?
+++ b/libgomp/testsuite/libgomp.fortran/map-subarray-10.f90
@@ -0,0 +1,33 @@
+! { dg-do run }
+
"! PR fortran/120505" – just as crossref?
+!$omp target
+ if (any (var%tiles(1)%den1 /= reshape([1,2,3,4],[2,2]))) stop 1
+ if (any (var%tiles(2)%den2 /= reshape([11,22,33,44],[2,2]))) stop 1
+ var%tiles(1)%den1 = var%tiles(1)%den1 + 5
+ var%tiles(2)%den2 = var%tiles(2)%den2 + 7
+!$omp end target
Check whether the result is fine, e.g.
if (any (var%tiles(1)%den1 /= 5 + reshape([1,2,3,4],[2,2]))) stop 1
if (any (var%tiles(2)%den2 /= 7 + reshape([11,22,33,44],[2,2]))) stop 1
* * *
+++ b/libgomp/testsuite/libgomp.fortran/map-subarray-9.f90
...
+!$omp target
+ if (any (var%tiles(1)%den1 /= reshape([1,2,3,4],[2,2]))) stop 1
+ if (any (var%tiles(1)%den2 /= reshape([11,22,33,44],[2,2]))) stop 1
+ var%tiles(1)%den1 = var%tiles(1)%den1 + 5
+ var%tiles(1)%den2 = var%tiles(1)%den2 + 7
+!$omp end target
check whether it has been updated:
if (any (var%tiles(1)%den1 /= 5 + reshape([1,2,3,4],[2,2]))) stop 1
if (any (var%tiles(1)%den2 /= 7 + reshape([11,22,33,44],[2,2]))) stop 1
Note: needs 'target update' or 'target exit data' before the check.
(both cases).
* * *
Otherwise, LGTM.
Thanks for the patch!
Tobias
