| Message ID | 5309b57f3707783cf65c2b6c5b9b784d8e61b760.1592343757.git.julian@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | Refcounting and manual deep copy improvements | expand |
Hi Julian! On 2020-06-16T15:39:45-0700, Julian Brown <julian@codesourcery.com> wrote: > This patch fixes a set of XFAILs The overall goal of couse is not to resolve XFAILs, but to implement the expected behavior. :-) > in some recently-added patches by > skipping a detach operation on "no-op" exit data operations for blocks > with zero dynamic refcount. So this is one aspect of <https://gcc.gnu.org/PR95203> "OpenACC 2.6 deep copy: attach/detach API routines: no-op behavior". > This takes advantage of the ordering of > detach clauses with respect to associated data-movement clauses: i.e., > they are grouped together adjacently. I'm not convinced that it's sufficient to just special-case these cases. Instead, per the OpenACC "Data Clause Actions" etc., shouldn't basically all 'gomp_fatal's that we have on 'attach'/'detach' code paths turn into no-ops ("no action is taken")? And I'd then again like to bring forward my idea from another review: | we may (rather easily?) add a flag variable (ICV; | initialized from an environment variable) to guard this checking | behavior? Here: to *keep* the 'gomp_fatal's. | I suppose we may now have a few libgomp testcases that | actually do [check things via expected 'gomp_fatal's], | which wouldn't work any longer [...]. Such testcases could then 'dg-set-target-env-var "GOMP_ATTACH_FATAL" "1"' (better name is desirable), and have one variant with and one variant without that enabled. Before you start re-working the patch, let's please first get agreement on what exactly we intend to achieve. Grüße Thomas > libgomp/ > * oacc-mem.c (find_group_last): Handle detach operations. > (goacc_exit_data_internal): Detect detachments that are part of copyout > operations, and suppress them if dynamic refcount is zero. > * testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90: Remove XFAILs. > * testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90: Fix typo. > * testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90: Remove XFAILs. > * testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90: Likewise. > * testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90: Likewise. > * testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90: Likewise. > * testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90: Likewise. > * testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90: Likewise. > --- > libgomp/oacc-mem.c | 54 ++++++++++++++++--- > .../mdc-refcount-1-1-1.f90 | 6 +-- > .../mdc-refcount-1-1-2.F90 | 2 +- > .../mdc-refcount-1-2-1.f90 | 6 +-- > .../mdc-refcount-1-2-2.f90 | 6 +-- > .../mdc-refcount-1-3-1.f90 | 6 +-- > .../mdc-refcount-1-3-2.f90 | 5 +- > .../mdc-refcount-1-4-1.f90 | 6 +-- > .../mdc-refcount-1-4-2.f90 | 5 +- > 9 files changed, 55 insertions(+), 41 deletions(-) > > diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c > index 745cb132621..f852652c048 100644 > --- a/libgomp/oacc-mem.c > +++ b/libgomp/oacc-mem.c > @@ -987,7 +987,9 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) > { > case GOMP_MAP_TO_PSET: > if (pos + 1 < mapnum > - && (kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH) > + && ((kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH > + || (kinds[pos + 1] & 0xff) == GOMP_MAP_DETACH > + || (kinds[pos + 1] & 0xff) == GOMP_MAP_FORCE_DETACH)) > return pos + 1; > > while (pos + 1 < mapnum > @@ -1010,6 +1012,8 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) > break; > > case GOMP_MAP_ATTACH: > + case GOMP_MAP_DETACH: > + case GOMP_MAP_FORCE_DETACH: > return pos; > > default: > @@ -1025,7 +1029,9 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) > /* We can have a single GOMP_MAP_ATTACH mapping after a to/from > mapping. */ > if (pos + 1 < mapnum > - && (kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH) > + && ((kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH > + || (kinds[pos + 1] & 0xff) == GOMP_MAP_DETACH > + || (kinds[pos + 1] & 0xff) == GOMP_MAP_FORCE_DETACH)) > return pos + 1; > > /* We can have zero or more GOMP_MAP_POINTER mappings after a to/from > @@ -1168,15 +1174,43 @@ goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum, > { > gomp_mutex_lock (&acc_dev->lock); > > - /* Handle "detach" before copyback/deletion of mapped data. */ > - for (size_t i = 0; i < mapnum; ++i) > + /* Handle "detach" before copyback/deletion of mapped data. If this isn't a > + standalone "detach" clause, take care to skip the "detach" operation if > + the dynamic refcount of the data to be detached is zero. */ > + for (size_t grp = 0; grp < mapnum; grp++) > { > - unsigned char kind = kinds[i] & 0xff; > + size_t i = grp, group_last = find_group_last (grp, mapnum, sizes, kinds); > + unsigned char kind = kinds[grp] & 0xff; > bool finalize = false; > + > switch (kind) > { > + case GOMP_MAP_TO_PSET: > + case GOMP_MAP_TOFROM: > + case GOMP_MAP_FROM: > + case GOMP_MAP_FORCE_FROM: > + case GOMP_MAP_RELEASE: > + case GOMP_MAP_DELETE: > + { > + if (i + 1 >= mapnum) > + break; > + kind = kinds[i + 1] & 0xff; > + if (kind != GOMP_MAP_FORCE_DETACH && kind != GOMP_MAP_DETACH) > + break; > + splay_tree_key n = lookup_host (acc_dev, hostaddrs[i], sizes[i]); > + if (n == NULL) > + { > + gomp_mutex_unlock (&acc_dev->lock); > + gomp_fatal ("target data not mapped for detach operation"); > + } > + i++; > + if (n->dynamic_refcount == 0) > + break; > + } > + /* Fallthrough. */ > + > case GOMP_MAP_FORCE_DETACH: > - finalize = true; > + finalize = (kind == GOMP_MAP_FORCE_DETACH); > /* Fallthrough. */ > > case GOMP_MAP_DETACH: > @@ -1197,9 +1231,15 @@ goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum, > gomp_detach_pointer (acc_dev, aq, n, hostaddr, finalize, NULL); > } > break; > + case GOMP_MAP_STRUCT: > + case GOMP_MAP_POINTER: > + /* Ignore. */ > + break; > default: > - ; > + gomp_fatal (">>>> goacc_exit_data_internal UNHANDLED kind 0x%.2x", > + kind); > } > + grp = group_last; > } > > for (size_t i = 0; i < mapnum; ++i) > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90 > index 445cbabb8ca..7171affb9f0 100644 > --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90 > @@ -24,12 +24,8 @@ program main > print *, "CheCKpOInT1" > ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } > !$acc exit data delete(var%a) finalize > - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. > - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). > - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. > - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. > print *, "CheCKpOInT2" > - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } > + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } > if (acc_is_present(var%a)) stop 3 > if (.not. acc_is_present(var)) stop 4 > > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90 > index 7b206ac2042..2aa46189e9a 100644 > --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90 > @@ -6,4 +6,4 @@ > #include "mdc-refcount-1-1-1.f90" > > ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } > -! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" } > +! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90 > index 8554534b2f2..9a10aa5a781 100644 > --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90 > @@ -26,12 +26,8 @@ program main > print *, "CheCKpOInT1" > ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } > !$acc exit data delete(var%a) finalize > - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. > - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). > - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. > - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. > print *, "CheCKpOInT2" > - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } > + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } > if (acc_is_present(var%a)) stop 3 > if (.not. acc_is_present(var)) stop 4 > > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90 > index 8e696cc70e8..f506adf8e91 100644 > --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90 > @@ -26,12 +26,8 @@ program main > print *, "CheCKpOInT1" > ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } > !$acc exit data delete(var%a) > - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. > - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). > - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. > - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. > print *, "CheCKpOInT2" > - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } > + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } > if (acc_is_present(var%a)) stop 3 > if (.not. acc_is_present(var)) stop 4 > > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90 > index 070a6f8e149..450d95d3686 100644 > --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90 > @@ -27,12 +27,8 @@ program main > print *, "CheCKpOInT1" > ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } > !$acc exit data delete(var%a) finalize > - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. > - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). > - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. > - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. > print *, "CheCKpOInT2" > - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } > + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } > if (acc_is_present(var%a)) stop 3 > if (.not. acc_is_present(var)) stop 4 > > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90 > index 3c4bbda7f66..35efad4138a 100644 > --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90 > @@ -27,11 +27,8 @@ program main > print *, "CheCKpOInT1" > ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } > !$acc exit data delete(var%a) > - !TODO { dg-output "(\n|\r\n|\r)libgomp: attach count underflow(\n|\r\n|\r)$" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). > - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. > - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. > print *, "CheCKpOInT2" > - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } > + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } > if (acc_is_present(var%a)) stop 3 > if (.not. acc_is_present(var)) stop 4 > > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90 > index b22e411567f..816562fc055 100644 > --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90 > @@ -26,12 +26,8 @@ program main > print *, "CheCKpOInT1" > ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } > !$acc exit data detach(var%a) finalize > - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. > - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). > - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. > - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. > print *, "CheCKpOInT2" > - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } > + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } > !$acc exit data delete(var%a) > if (acc_is_present(var%a)) stop 3 > if (.not. acc_is_present(var)) stop 4 > diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90 > index 476cd5c1bee..b98bfd74924 100644 > --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90 > +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90 > @@ -27,11 +27,8 @@ program main > print *, "CheCKpOInT1" > ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } > !$acc exit data delete(var%a) > - !TODO { dg-output "(\n|\r\n|\r)libgomp: attach count underflow(\n|\r\n|\r)$" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). > - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. > - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. > print *, "CheCKpOInT2" > - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } > + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } > if (acc_is_present(var%a)) stop 3 > if (.not. acc_is_present(var)) stop 4 > > -- > 2.23.0 ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
On Fri, 24 Jul 2020 16:18:34 +0200 Thomas Schwinge <thomas@codesourcery.com> wrote: > Hi Julian! > > On 2020-06-16T15:39:45-0700, Julian Brown <julian@codesourcery.com> > wrote: > > This patch fixes a set of XFAILs > > The overall goal of couse is not to resolve XFAILs, but to implement > the expected behavior. :-) Eh, details :-). > > in some recently-added patches by > > skipping a detach operation on "no-op" exit data operations for > > blocks with zero dynamic refcount. > > So this is one aspect of <https://gcc.gnu.org/PR95203> "OpenACC 2.6 > deep copy: attach/detach API routines: no-op behavior". Not quite, I don't think -- that's about pointers (or pointed-to blocks) that are not mapped on the target. With this patch, we have a detach operation with a block with a zero dynamic refcount, but a non-zero structured refcount -- i.e. it's still mapped. So I think the patch is necessary, but not sufficient for the other cases you mention. > > This takes advantage of the ordering of > > detach clauses with respect to associated data-movement clauses: > > i.e., they are grouped together adjacently. > > I'm not convinced that it's sufficient to just special-case these > cases. Instead, per the OpenACC "Data Clause Actions" etc., shouldn't > basically all 'gomp_fatal's that we have on 'attach'/'detach' code > paths turn into no-ops ("no action is taken")? > > > And I'd then again like to bring forward my idea from another review: > > | we may (rather easily?) add a flag variable (ICV; > | initialized from an environment variable) to guard this checking > | behavior? > > Here: to *keep* the 'gomp_fatal's. > > | I suppose we may now have a few libgomp testcases that > | actually do [check things via expected 'gomp_fatal's], > | which wouldn't work any longer [...]. > > Such testcases could then 'dg-set-target-env-var "GOMP_ATTACH_FATAL" > "1"' (better name is desirable), and have one variant with and one > variant without that enabled. > > > Before you start re-working the patch, let's please first get > agreement on what exactly we intend to achieve. Hm, you are probably right about the no-op behaviour for attach operations (but slightly ugh in terms of usability), but I don't think that's really the problem the patch addresses. As for the user-tweakable checking -- yeah maybe it could work, but I don't think I'm going to have time to work on that at the moment. Sorry! HTH, Julian
diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c index 745cb132621..f852652c048 100644 --- a/libgomp/oacc-mem.c +++ b/libgomp/oacc-mem.c @@ -987,7 +987,9 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) { case GOMP_MAP_TO_PSET: if (pos + 1 < mapnum - && (kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH) + && ((kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH + || (kinds[pos + 1] & 0xff) == GOMP_MAP_DETACH + || (kinds[pos + 1] & 0xff) == GOMP_MAP_FORCE_DETACH)) return pos + 1; while (pos + 1 < mapnum @@ -1010,6 +1012,8 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) break; case GOMP_MAP_ATTACH: + case GOMP_MAP_DETACH: + case GOMP_MAP_FORCE_DETACH: return pos; default: @@ -1025,7 +1029,9 @@ find_group_last (int pos, size_t mapnum, size_t *sizes, unsigned short *kinds) /* We can have a single GOMP_MAP_ATTACH mapping after a to/from mapping. */ if (pos + 1 < mapnum - && (kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH) + && ((kinds[pos + 1] & 0xff) == GOMP_MAP_ATTACH + || (kinds[pos + 1] & 0xff) == GOMP_MAP_DETACH + || (kinds[pos + 1] & 0xff) == GOMP_MAP_FORCE_DETACH)) return pos + 1; /* We can have zero or more GOMP_MAP_POINTER mappings after a to/from @@ -1168,15 +1174,43 @@ goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum, { gomp_mutex_lock (&acc_dev->lock); - /* Handle "detach" before copyback/deletion of mapped data. */ - for (size_t i = 0; i < mapnum; ++i) + /* Handle "detach" before copyback/deletion of mapped data. If this isn't a + standalone "detach" clause, take care to skip the "detach" operation if + the dynamic refcount of the data to be detached is zero. */ + for (size_t grp = 0; grp < mapnum; grp++) { - unsigned char kind = kinds[i] & 0xff; + size_t i = grp, group_last = find_group_last (grp, mapnum, sizes, kinds); + unsigned char kind = kinds[grp] & 0xff; bool finalize = false; + switch (kind) { + case GOMP_MAP_TO_PSET: + case GOMP_MAP_TOFROM: + case GOMP_MAP_FROM: + case GOMP_MAP_FORCE_FROM: + case GOMP_MAP_RELEASE: + case GOMP_MAP_DELETE: + { + if (i + 1 >= mapnum) + break; + kind = kinds[i + 1] & 0xff; + if (kind != GOMP_MAP_FORCE_DETACH && kind != GOMP_MAP_DETACH) + break; + splay_tree_key n = lookup_host (acc_dev, hostaddrs[i], sizes[i]); + if (n == NULL) + { + gomp_mutex_unlock (&acc_dev->lock); + gomp_fatal ("target data not mapped for detach operation"); + } + i++; + if (n->dynamic_refcount == 0) + break; + } + /* Fallthrough. */ + case GOMP_MAP_FORCE_DETACH: - finalize = true; + finalize = (kind == GOMP_MAP_FORCE_DETACH); /* Fallthrough. */ case GOMP_MAP_DETACH: @@ -1197,9 +1231,15 @@ goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum, gomp_detach_pointer (acc_dev, aq, n, hostaddr, finalize, NULL); } break; + case GOMP_MAP_STRUCT: + case GOMP_MAP_POINTER: + /* Ignore. */ + break; default: - ; + gomp_fatal (">>>> goacc_exit_data_internal UNHANDLED kind 0x%.2x", + kind); } + grp = group_last; } for (size_t i = 0; i < mapnum; ++i) diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90 index 445cbabb8ca..7171affb9f0 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-1.f90 @@ -24,12 +24,8 @@ program main print *, "CheCKpOInT1" ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } !$acc exit data delete(var%a) finalize - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. print *, "CheCKpOInT2" - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } if (acc_is_present(var%a)) stop 3 if (.not. acc_is_present(var)) stop 4 diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90 index 7b206ac2042..2aa46189e9a 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-1-2.F90 @@ -6,4 +6,4 @@ #include "mdc-refcount-1-1-1.f90" ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } -! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" } +! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90 index 8554534b2f2..9a10aa5a781 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-1.f90 @@ -26,12 +26,8 @@ program main print *, "CheCKpOInT1" ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } !$acc exit data delete(var%a) finalize - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. print *, "CheCKpOInT2" - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } if (acc_is_present(var%a)) stop 3 if (.not. acc_is_present(var)) stop 4 diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90 index 8e696cc70e8..f506adf8e91 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-2-2.f90 @@ -26,12 +26,8 @@ program main print *, "CheCKpOInT1" ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } !$acc exit data delete(var%a) - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. print *, "CheCKpOInT2" - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } if (acc_is_present(var%a)) stop 3 if (.not. acc_is_present(var)) stop 4 diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90 index 070a6f8e149..450d95d3686 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-1.f90 @@ -27,12 +27,8 @@ program main print *, "CheCKpOInT1" ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } !$acc exit data delete(var%a) finalize - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. print *, "CheCKpOInT2" - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } if (acc_is_present(var%a)) stop 3 if (.not. acc_is_present(var)) stop 4 diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90 index 3c4bbda7f66..35efad4138a 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-3-2.f90 @@ -27,11 +27,8 @@ program main print *, "CheCKpOInT1" ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } !$acc exit data delete(var%a) - !TODO { dg-output "(\n|\r\n|\r)libgomp: attach count underflow(\n|\r\n|\r)$" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. print *, "CheCKpOInT2" - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } if (acc_is_present(var%a)) stop 3 if (.not. acc_is_present(var)) stop 4 diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90 index b22e411567f..816562fc055 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-1.f90 @@ -26,12 +26,8 @@ program main print *, "CheCKpOInT1" ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } !$acc exit data detach(var%a) finalize - !TODO goacc_exit_data_internal: Assertion `is_tgt_unmapped || num_mappings > 1' failed. - !TODO { dg-output ".*\[Aa\]ssert.*is_tgt_unmapped" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. print *, "CheCKpOInT2" - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } !$acc exit data delete(var%a) if (acc_is_present(var%a)) stop 3 if (.not. acc_is_present(var)) stop 4 diff --git a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90 index 476cd5c1bee..b98bfd74924 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90 +++ b/libgomp/testsuite/libgomp.oacc-fortran/mdc-refcount-1-4-2.f90 @@ -27,11 +27,8 @@ program main print *, "CheCKpOInT1" ! { dg-output ".*CheCKpOInT1(\n|\r\n|\r)" } !$acc exit data delete(var%a) - !TODO { dg-output "(\n|\r\n|\r)libgomp: attach count underflow(\n|\r\n|\r)$" { target { ! openacc_host_selected } } } ! Scan for what we expect in the "XFAILed" case (without actually XFAILing). - !TODO { dg-shouldfail "XFAILed" { ! openacc_host_selected } } ! ... instead of 'dg-xfail-run-if' so that 'dg-output' is evaluated at all. - !TODO { dg-final { if { [dg-process-target { xfail { ! openacc_host_selected } }] == "F" } { xfail "[testname-for-summary] really is XFAILed" } } } ! ... so that we still get an XFAIL visible in the log. print *, "CheCKpOInT2" - ! { dg-output ".CheCKpOInT2(\n|\r\n|\r)" { target { openacc_host_selected } } } + ! { dg-output ".*CheCKpOInT2(\n|\r\n|\r)" } if (acc_is_present(var%a)) stop 3 if (.not. acc_is_present(var)) stop 4