Re: [PATCH 6/7] [OpenACC] Reference count self-checking (dynamic_refcount version)
On Fri, 22 May 2020 15:21:44 -0700
Julian Brown wrote:
> This is a new version of the reference count self-checking code,
> adjusted to work with the new (old) dynamic_refcount counting scheme.
> The key observation is that a target_mem_desc that was created from
> a dynamic data lifetime should not contribute to the structured
> refcount for splay tree keys in its variable list. We can figure out
> which target_mem_descs that applies to using the information recorded
> in the previous patch.
>
> In a sense, this takes the "awkward corner cases" from the
> virtual_refcount ("overhaul") patch, and moves them to the optional
> self-test code, where they can potentially do less harm. With this,
> we still have a formal-ish model of what refcounts mean and some
> confidence that they remain consistent (at least throughout execution
> of a test run), which I think is a good thing.
>
> OK? (We probably want a way of configuring-in this testing
> automatically, as mentioned previously.)
This is a new version of the self-checking patch that works with the
recent patch to stop attach/detach operations from affecting reference
counts:
https://gcc.gnu.org/pipermail/gcc-patches/2020-June/548529.html
This patch depends on the previous patch to distinguish structural from
dynamic reference counts:
https://gcc.gnu.org/pipermail/gcc-patches/2020-May/546334.html
There still isn't a way of neatly configuring self-checking behaviour
on/off, so this is mostly just useful for development (or as a
proof-of-concept).
Thanks,
Julian
commit 920a44da7b74ddbe4e6d908a56a67e98d2078756
Author: Julian Brown
Date: Thu May 21 02:41:32 2020 -0700
[OpenACC] Reference count self-checking (dynamic_refcount version)
libgomp/
* libgomp.h (RC_CHECKING): New macro, disabled by default, guarding all
hunks in this patch.
(target_mem_desc): Add refcount_chk, mark fields.
(splay_tree_key_s): Add refcount_chk field.
(dump_tgt, gomp_rc_check): Add prototypes.
* oacc-mem.c (GOACC_enter_exit_data): Add refcount self-check code.
* oacc-parallel.c (GOACC_parallel_keyed_internal): Add refcount
self-check code.
(GOACC_data_start, GOACC_data_end, GOACC_enter_exit_data): Likewise.
* target.c (stdio.h): Include.
(dump_tgt, rc_check_clear, rc_check_count, rc_check_verify,
gomp_rc_check): New functions to consistency-check reference counts.
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 0d1978ffb13..eaa7c6ebb4c 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -960,9 +960,17 @@ struct target_var_desc {
uintptr_t length;
};
+/* Uncomment to enable reference-count consistency checking (for development
+ use only). */
+//#define RC_CHECKING 1
+
struct target_mem_desc {
/* Reference count. */
uintptr_t refcount;
+#ifdef RC_CHECKING
+ uintptr_t refcount_chk;
+ bool mark;
+#endif
/* All the splay nodes allocated together. */
splay_tree_node array;
/* Start of the target region. */
@@ -1019,6 +1027,10 @@ struct splay_tree_key_s {
uintptr_t refcount;
/* Dynamic reference count. */
uintptr_t dynamic_refcount;
+#ifdef RC_CHECKING
+ /* The recalculated reference count, for verification. */
+ uintptr_t refcount_chk;
+#endif
struct splay_tree_aux *aux;
};
@@ -1174,6 +1186,12 @@ extern void gomp_detach_pointer (struct gomp_device_descr *,
struct goacc_asyncqueue *, splay_tree_key,
uintptr_t, bool, struct gomp_coalesce_buf *);
+#ifdef RC_CHECKING
+extern void dump_tgt (const char *, struct target_mem_desc *);
+extern void gomp_rc_check (struct gomp_device_descr *,
+ struct target_mem_desc *);
+#endif
+
extern struct target_mem_desc *gomp_map_vars (struct gomp_device_descr *,
size_t, void **, void **,
size_t *, void *, bool,
diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index a9682e832be..1816b06bf2d 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -1522,4 +1522,10 @@ GOACC_enter_exit_data (int flags_m, size_t mapnum, void **hostaddrs,
thr->prof_info = NULL;
thr->api_info = NULL;
}
+
+#ifdef RC_CHECKING
+ gomp_mutex_lock (_dev->lock);
+ gomp_rc_check (acc_dev, thr->mapped_data);
+ gomp_mutex_unlock (_dev->lock);
+#endif
}
diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
index c7e46e35bd6..0774cdc7e4f 100644
--- a/libgomp/oacc-parallel.c
+++ b/libgomp/oacc-parallel.c
@@ -301,6 +301,15 @@ GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
_info);
}
+#ifdef RC_CHECKING
+ gomp_mutex_lock (_dev->lock);
+ assert (tgt);
+ dump_tgt (__FUNCTION__, tgt);
+ tgt->prev = thr->mapped_data;
+ gomp_rc_check (acc_dev, tgt);
+ gomp_mutex_unlock (_dev->lock);
+#endif
+
devaddrs = gomp_alloca (sizeof (void *) * mapnum);
for (i = 0; i < mapnum; i++)
devaddrs[i] = (void *) gomp_map_val (tgt, hostaddrs, i);
@@ -347,6
[PATCH 6/7] [OpenACC] Reference count self-checking (dynamic_refcount version)
This is a new version of the reference count self-checking code, adjusted
to work with the new (old) dynamic_refcount counting scheme. The key
observation is that a target_mem_desc that was created from a dynamic
data lifetime should not contribute to the structured refcount for splay
tree keys in its variable list. We can figure out which target_mem_descs
that applies to using the information recorded in the previous patch.
In a sense, this takes the "awkward corner cases" from the
virtual_refcount ("overhaul") patch, and moves them to the optional
self-test code, where they can potentially do less harm. With this, we
still have a formal-ish model of what refcounts mean and some confidence
that they remain consistent (at least throughout execution of a test run),
which I think is a good thing.
OK? (We probably want a way of configuring-in this testing automatically,
as mentioned previously.)
Julian
ChangeLog
libgomp/
* libgomp.h (RC_CHECKING): New macro, disabled by default, guarding all
hunks in this patch.
(target_mem_desc): Add refcount_chk, mark fields.
(splay_tree_key_s): Add refcount_chk field.
(dump_tgt, gomp_rc_check): Add prototypes.
* oacc-mem.c (GOACC_enter_exit_data): Add refcount self-check code.
* oacc-parallel.c (GOACC_parallel_keyed_internal): Add refcount
self-check code.
(GOACC_data_start, GOACC_data_end, GOACC_enter_exit_data): Likewise.
* target.c (stdio.h): Include.
(dump_tgt, rc_check_clear, rc_check_count, rc_check_verify,
gomp_rc_check): New functions to consistency-check reference counts.
---
libgomp/libgomp.h | 18
libgomp/oacc-mem.c | 6 ++
libgomp/oacc-parallel.c | 27 ++
libgomp/target.c| 185
4 files changed, 236 insertions(+)
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 0d1978ffb13..eaa7c6ebb4c 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -960,9 +960,17 @@ struct target_var_desc {
uintptr_t length;
};
+/* Uncomment to enable reference-count consistency checking (for development
+ use only). */
+//#define RC_CHECKING 1
+
struct target_mem_desc {
/* Reference count. */
uintptr_t refcount;
+#ifdef RC_CHECKING
+ uintptr_t refcount_chk;
+ bool mark;
+#endif
/* All the splay nodes allocated together. */
splay_tree_node array;
/* Start of the target region. */
@@ -1019,6 +1027,10 @@ struct splay_tree_key_s {
uintptr_t refcount;
/* Dynamic reference count. */
uintptr_t dynamic_refcount;
+#ifdef RC_CHECKING
+ /* The recalculated reference count, for verification. */
+ uintptr_t refcount_chk;
+#endif
struct splay_tree_aux *aux;
};
@@ -1174,6 +1186,12 @@ extern void gomp_detach_pointer (struct
gomp_device_descr *,
struct goacc_asyncqueue *, splay_tree_key,
uintptr_t, bool, struct gomp_coalesce_buf *);
+#ifdef RC_CHECKING
+extern void dump_tgt (const char *, struct target_mem_desc *);
+extern void gomp_rc_check (struct gomp_device_descr *,
+ struct target_mem_desc *);
+#endif
+
extern struct target_mem_desc *gomp_map_vars (struct gomp_device_descr *,
size_t, void **, void **,
size_t *, void *, bool,
diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c
index 038ab68e8a2..c8ec3c9a7dd 100644
--- a/libgomp/oacc-mem.c
+++ b/libgomp/oacc-mem.c
@@ -1450,4 +1450,10 @@ GOACC_enter_exit_data (int flags_m, size_t mapnum, void
**hostaddrs,
thr->prof_info = NULL;
thr->api_info = NULL;
}
+
+#ifdef RC_CHECKING
+ gomp_mutex_lock (_dev->lock);
+ gomp_rc_check (acc_dev, thr->mapped_data);
+ gomp_mutex_unlock (_dev->lock);
+#endif
}
diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
index c7e46e35bd6..0774cdc7e4f 100644
--- a/libgomp/oacc-parallel.c
+++ b/libgomp/oacc-parallel.c
@@ -301,6 +301,15 @@ GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
_info);
}
+#ifdef RC_CHECKING
+ gomp_mutex_lock (_dev->lock);
+ assert (tgt);
+ dump_tgt (__FUNCTION__, tgt);
+ tgt->prev = thr->mapped_data;
+ gomp_rc_check (acc_dev, tgt);
+ gomp_mutex_unlock (_dev->lock);
+#endif
+
devaddrs = gomp_alloca (sizeof (void *) * mapnum);
for (i = 0; i < mapnum; i++)
devaddrs[i] = (void *) gomp_map_val (tgt, hostaddrs, i);
@@ -347,6 +356,12 @@ GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
thr->prof_info = NULL;
thr->api_info = NULL;
}
+
+#ifdef RC_CHECKING
+ gomp_mutex_lock (_dev->lock);
+ gomp_rc_check (acc_dev, thr->mapped_data);
+ gomp_mutex_unlock (_dev->lock);
+#endif
}
/* Legacy entry point (GCC 5). Only provide host fallback execution. */
@@ -481,6 +496,12 @@ GOACC_data_start (int flags_m, size_t mapnum,
thr->prof_info = NULL;
