Using the ability to distinguish structural from dynamic mappings' target_mem_descs, we can adjust how the assertions in goacc_exit_datum and goacc_exit_data_internal work. This is possibly a slightly stronger test than the one introduced earlier in this patch series -- though actually I haven't quite convinced myself of that.
Anyway, this passes a regression run, with the refcount self-checking code enabled also. OK, or any comments? Julian ChangeLog libgomp/ * oacc-mem.c (goacc_exit_datum): Adjust self-test code. (goacc_exit_data_internal): Likewise. * target.c (gomp_unmap_vars_internal): Clear target_mem_desc variable list keys on unmapping. --- libgomp/oacc-mem.c | 43 ++++++++++++++++++++++++------------------- libgomp/target.c | 8 +++++++- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c index c8ec3c9a7dd..d7a1d87c9ef 100644 --- a/libgomp/oacc-mem.c +++ b/libgomp/oacc-mem.c @@ -755,16 +755,19 @@ goacc_exit_datum (void *h, size_t s, unsigned short kind, int async) gomp_remove_var_async (acc_dev, n, aq); else { - int num_mappings = 0; - /* If the target_mem_desc represents a single data mapping, we can - check that it is freed when this splay tree key's refcount - reaches zero. Otherwise (e.g. for a struct mapping with multiple - members), fall back to skipping the test. */ - for (int i = 0; i < n->tgt->list_count; i++) - if (n->tgt->list[i].key) - num_mappings++; + int remaining_mappings = 0; + bool dynamic = goacc_marked_dynamic_p (n->tgt); + if (dynamic) + { + /* For dynamic mappings, we may have more than one live splay + tree in the target_mem_desc's variable list. That's not an + error. */ + for (int i = 0; i < n->tgt->list_count; i++) + if (n->tgt->list[i].key) + remaining_mappings++; + } bool is_tgt_unmapped = gomp_remove_var (acc_dev, n); - assert (num_mappings > 1 || is_tgt_unmapped); + assert ((dynamic && remaining_mappings > 0) || is_tgt_unmapped); } } @@ -1283,17 +1286,19 @@ goacc_exit_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum, gomp_fatal ("synchronize failed"); } } - int num_mappings = 0; - /* If the target_mem_desc represents a single data mapping, we - can check that it is freed when this splay tree key's - refcount reaches zero. Otherwise (e.g. for a struct - mapping with multiple members), fall back to skipping the - test. */ - for (int j = 0; j < n->tgt->list_count; j++) - if (n->tgt->list[j].key) - num_mappings++; + int remaining_mappings = 0; + bool dynamic = goacc_marked_dynamic_p (n->tgt); + if (dynamic) + { + /* For dynamic mappings, we may have more than one live + splay tree in the target_mem_desc's variable list. + That's not an error. */ + for (int j = 0; j < n->tgt->list_count; j++) + if (n->tgt->list[j].key) + remaining_mappings++; + } bool is_tgt_unmapped = gomp_remove_var (acc_dev, n); - assert (num_mappings > 1 || is_tgt_unmapped); + assert ((dynamic && remaining_mappings > 0) || is_tgt_unmapped); } } break; diff --git a/libgomp/target.c b/libgomp/target.c index 9a51e1c70f6..f072e050cc1 100644 --- a/libgomp/target.c +++ b/libgomp/target.c @@ -1630,6 +1630,7 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom, if (k == NULL) continue; + bool clear_mapping = true; bool do_unmap = false; if (k->refcount > 1 && k->refcount != REFCOUNT_INFINITY) { @@ -1637,7 +1638,10 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom, /* If we only have dynamic references left, mark the tgt_mem_desc appropriately. */ if (k->refcount == k->dynamic_refcount) - goacc_mark_dynamic (k->tgt); + { + goacc_mark_dynamic (k->tgt); + clear_mapping = false; + } } else if (k->refcount == 1) { @@ -1662,6 +1666,8 @@ gomp_unmap_vars_internal (struct target_mem_desc *tgt, bool do_copyfrom, assert (!is_tgt_unmapped || k_tgt != tgt); } + if (clear_mapping) + tgt->list[i].key = NULL; } if (aq) -- 2.23.0