As of Tobias' recent r277631 "Fortran/OpenMP] Don't create "alloc:" for 'target exit data'", <http://mid.mail-archive.com/7a5f39e8-a33b-048a-f9c1-1355b941771e@codesourcery.com>, this is done for Fortran OpenMP '!$omp target exit data'/'!$omp target update', but not for OpenACC '!$acc exit data'/'!$acc update'; see 'gcc/gimplify.c:gimplify_scan_omp_clauses': /* For Fortran, not only the pointer to the data is mapped but also the address of the pointer, the array descriptor etc.; for 'exit data' - and in particular for 'delete:' - having an 'alloc:' does not make sense. Likewise, for 'update' only transferring the data itself is needed as the rest has been handled in previous directives. */ if ((code == OMP_TARGET_EXIT_DATA || code == OMP_TARGET_UPDATE) && (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET)) remove = true; Note that I'm not proposing to also do that for OpenACC at this point; I have one doubt about that I need to look into first, but don't have time for right now.
Relatedly, since r228777 "Merge from gomp-4_1-branch to trunk" ("which brings in most of the OpenMP 4.5 support for C and C++"), the following optimization is done: case OMP_TARGET_ENTER_DATA: case OMP_TARGET_EXIT_DATA: case OACC_ENTER_DATA: case OACC_EXIT_DATA: case OACC_HOST_DATA: if (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER || (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_REFERENCE)) /* For target {,enter ,exit }data only the array slice is mapped, but not the pointer to it. */ remove = true; break; Why is this not applicable for OpenACC '!$acc update'/OpenMP '!$omp target update', too? (This is now also relevant for C/C++.)
(In reply to myself from comment #1) > if (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER > || (OMP_CLAUSE_MAP_KIND (c) > == GOMP_MAP_FIRSTPRIVATE_REFERENCE)) > /* For target {,enter ,exit }data only the array slice is > mapped, but not the pointer to it. */ > remove = true; > Why is this not applicable for OpenACC '!$acc update'/OpenMP '!$omp target > update', too? (This is now also relevant for C/C++.) I see I asked the same question before, and in <http://mid.mail-archive.com/574F1C6C.5020704@codesourcery.com> Cesar answered: "I suppose they can be added here, but lower_omp_target already ignores GOMP_MAP_FIRSTPRIVATE_POINTER for non-offloaded and data_region regions."
(In reply to myself from comment #1) > /* For target {,enter ,exit }data only the array slice is > mapped, but not the pointer to it. */ Some handling for OpenACC 'data' got added in r236678 "use firstprivate pointers for subarrays in c and c++", <http://mid.mail-archive.com/573244BE.5010708@codesourcery.com>, but is somewhat spread out, and not easy to follow: case OACC_DATA: if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE) break; /* FALLTHRU */ case OMP_TARGET_DATA: case [...]: if (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER || (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_REFERENCE)) /* For target {,enter ,exit }data only the array slice is mapped, but not the pointer to it. */ remove = true; [...] if (code == OACC_DATA && OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP && (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_REFERENCE)) remove = true; if (remove) *list_p = OMP_CLAUSE_CHAIN (c); ..., and not sure if this also relates to the following: 'omp_notice_variable': if (octx->region_type == ORT_ACC_DATA && (n2->value & GOVD_MAP_0LEN_ARRAY)) nflags |= GOVD_MAP_0LEN_ARRAY; goto found_outer; 'gimplify_scan_omp_clauses': if (code == OACC_DATA && OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER) flags |= GOVD_MAP_0LEN_ARRAY; (Maybe the idea there is to preserve 'GOMP_MAP_FIRSTPRIVATE_POINTER', 'GOMP_MAP_FIRSTPRIVATE_REFERENCE' until some other processing ('GOVD_MAP_0LEN_ARRAY') has been done, and only then remove them, right at the end. But that wouldn't be quite obvious.)
The recent r279626 "OpenACC 2.6 deep copy: middle-end parts" contains changes related to this (which caused PR93026). Would've been good to first sort out the desired behavior (this PR92929) before introducing new functionality (OpenACC 2.6 manual deep copy) that supposedly depends on these optimizations? In particular, this needs careful attention, because we have to stay ABI compatible with existing GCC releases: libgomp needs to do the right things for both the "old" and the "new" way. (I'm not claiming that it isn't -- just that it needs careful attention.)
Author: tschwinge Date: Sat Dec 21 21:32:36 2019 New Revision: 279700 URL: https://gcc.gnu.org/viewcvs?rev=279700&root=gcc&view=rev Log: [PR93026, PR92929] Adjust 'gfortran.dg/goacc/finalize-1.f' for r279626 changes gcc/testsuite/ PR fortran/93026 PR middle-end/92929 * gfortran.dg/goacc/finalize-1.f: Adjust. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gfortran.dg/goacc/finalize-1.f
Apologies for breakage. This part of the patch was originally from the og9 patch supporting Fortran polymorphic class pointers posted at https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00752.html. The rationale was as follows: [...] OpenACC "enter data" and "exit data" now have GOMP_MAP_POINTER and GOMP_MAP_PSET mappings removed during gimplification. In some circumstances, passing an array to a function/subroutine and then doing an "enter data" on it could leave dangling references to the function's stack, although the actual array data is defined outside the function. In any case, the pointer/pointer-set mappings don't seem to be necessary for OpenACC "enter data". I observed the described problem with a large test program, and unfortunately did not manage to come up with a minimised test case at the time. Someone more familiar with Fortran might be able to do so more easily than me! In terms of backwards compatibility, we can't do anything about the "parasitic binding" to a function's expired stack frame as generated by an older version of the compiler in this case, I don't think. That's most likely going to manifest as an intermittent crash.
Test case & further discussion in: https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547424.html
In addition to r277631 "Fortran/OpenMP] Don't create "alloc:" for 'target exit data'", as shown in the PR94635 commit af557050fd011a03d21dc26b31959033061a0443 "[OpenMP] Fix 'omp exit data' for Fortran arrays (PR 94635)", and commit 9f0f7da9aa98eec28b4e5e34ade0aa0028df161d "[OpenMP] Fix 'omp exit data' for Fortran arrays (PR 94635)", this is not only an optimization but also a correctness issue. The OpenMP cases now presumably have been addressed, but OpenACC not yet?
(In reply to Thomas Schwinge from comment #0) > As of Tobias' recent r277631 "Fortran/OpenMP] Don't create "alloc:" for > 'target exit data'", > <http://mid.mail-archive.com/7a5f39e8-a33b-048a-f9c1- > 1355b941771e@codesourcery.com>, this is done for Fortran OpenMP '!$omp > target exit data'/'!$omp target update', but not for OpenACC '!$acc exit > data'/'!$acc update'; see 'gcc/gimplify.c:gimplify_scan_omp_clauses': > > /* For Fortran, not only the pointer to the data is mapped but also > the address of the pointer, the array descriptor etc.; for > 'exit data' - and in particular for 'delete:' - having an 'alloc:' > does not make sense. Likewise, for 'update' only transferring the > data itself is needed as the rest has been handled in previous > directives. */ > if ((code == OMP_TARGET_EXIT_DATA || code == OMP_TARGET_UPDATE) > && (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER > || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET)) > remove = true; Is it actually correct to remove 'GOMP_MAP_TO_PSET' for 'OMP_TARGET_UPDATE'? Untested -- I'm thinking of the following Fortran scenario: arrays a1, a2 pointer p [target] enter data(a1, a2) p => a1 [target] enter data(p) ! creates persistent, visible device copy of 'p', initially pointing to 'a1' p => a2 [target] update to(p) ! do we now still have 'p => a1' on the device, or 'p => a2'? What is this '[target] update' to do? Just update the data pointed to, and also/or only update the array descriptor on the device? If the latter, then didn't get this broken by r277631 (cited above)? Again: unverified. But (also unverified), would 'libgomp/target.c:gomp_update' actually do the right thing for 'GOMP_MAP_TO_PSET', given the current 'GOMP_MAP_COPY_TO_P' definition? Am I confused, or the code?
(In reply to Thomas Schwinge from comment #4) > The recent r279626 "OpenACC 2.6 deep copy: middle-end parts" contains > changes related to this ... some of which have now gotten reverted in recent commit g:b20097c65d2e74b1901fba1c55c77f0407e542d2 "openacc: Don't strip TO_PSET/POINTER for enter/exit data".