Libgomp magic offset value self-documentation

Message ID 20191003163505.49997-1-julian@codesourcery.com
State New
Headers show
Series
  • Libgomp magic offset value self-documentation
Related show

Commit Message

Julian Brown Oct. 3, 2019, 4:35 p.m.
This patch provides some macros naming special offset field values used
in libgomp/target.c.  It was previously posted as part of the series
supporting OpenACC 2.6 manual deep copy here:

  https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00825.html

Parts of that patch were committed already by Thomas in:

  https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01723.html

This patch comprises the remaining parts.  Tested (alongside other
patches) with offloading to NVPTX.  OK for trunk?

Thanks,

Julian

2019-10-02  Julian Brown  <julian@codesourcery.com>
	    Cesar Philippidis  <cesar@codesourcery.com>

	libgomp/
	* libgomp.h (OFFSET_INLINED, OFFSET_POINTER, OFFSET_STRUCT): Define.
	* target.c (FIELD_TGT_EMPTY): Define.
	(gomp_map_val): Use OFFSET_* macros instead of magic constants.  Write
	as switch instead of list of ifs.
	(gomp_map_vars_internal): Use OFFSET_* and FIELD_TGT_EMPTY macros.
---
 libgomp/libgomp.h |  5 +++++
 libgomp/target.c  | 44 +++++++++++++++++++++++++++-----------------
 2 files changed, 32 insertions(+), 17 deletions(-)

-- 
2.23.0

Comments

Jakub Jelinek Oct. 3, 2019, 5:08 p.m. | #1
On Thu, Oct 03, 2019 at 09:35:03AM -0700, Julian Brown wrote:
> This patch provides some macros naming special offset field values used

> in libgomp/target.c.  It was previously posted as part of the series

> supporting OpenACC 2.6 manual deep copy here:

> 

>   https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00825.html

> 

> Parts of that patch were committed already by Thomas in:

> 

>   https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01723.html

> 

> This patch comprises the remaining parts.  Tested (alongside other

> patches) with offloading to NVPTX.  OK for trunk?

> 

> Thanks,

> 

> Julian

> 

> 2019-10-02  Julian Brown  <julian@codesourcery.com>

> 	    Cesar Philippidis  <cesar@codesourcery.com>

> 

> 	libgomp/

> 	* libgomp.h (OFFSET_INLINED, OFFSET_POINTER, OFFSET_STRUCT): Define.

> 	* target.c (FIELD_TGT_EMPTY): Define.

> 	(gomp_map_val): Use OFFSET_* macros instead of magic constants.  Write

> 	as switch instead of list of ifs.

> 	(gomp_map_vars_internal): Use OFFSET_* and FIELD_TGT_EMPTY macros.


Ok.

	Jakub

Patch

diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index abb5c2263c4..178eb600ccd 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -903,6 +903,11 @@  struct target_mem_desc {
    artificial pointer to "omp declare target link" object.  */
 #define REFCOUNT_LINK (~(uintptr_t) 1)
 
+/* Special offset values.  */
+#define OFFSET_INLINED (~(uintptr_t) 0)
+#define OFFSET_POINTER (~(uintptr_t) 1)
+#define OFFSET_STRUCT (~(uintptr_t) 2)
+
 struct splay_tree_key_s {
   /* Address of the host object.  */
   uintptr_t host_start;
diff --git a/libgomp/target.c b/libgomp/target.c
index 1c9ca68ba10..a83cb48108a 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -44,6 +44,8 @@ 
 #include "plugin-suffix.h"
 #endif
 
+#define FIELD_TGT_EMPTY (~(size_t) 0)
+
 static void gomp_target_init (void);
 
 /* The whole initialization code for offloading plugins is only run one.  */
@@ -497,17 +499,25 @@  gomp_map_val (struct target_mem_desc *tgt, void **hostaddrs, size_t i)
     return tgt->list[i].key->tgt->tgt_start
 	   + tgt->list[i].key->tgt_offset
 	   + tgt->list[i].offset;
-  if (tgt->list[i].offset == ~(uintptr_t) 0)
-    return (uintptr_t) hostaddrs[i];
-  if (tgt->list[i].offset == ~(uintptr_t) 1)
-    return 0;
-  if (tgt->list[i].offset == ~(uintptr_t) 2)
-    return tgt->list[i + 1].key->tgt->tgt_start
-	   + tgt->list[i + 1].key->tgt_offset
-	   + tgt->list[i + 1].offset
-	   + (uintptr_t) hostaddrs[i]
-	   - (uintptr_t) hostaddrs[i + 1];
-  return tgt->tgt_start + tgt->list[i].offset;
+
+  switch (tgt->list[i].offset)
+    {
+    case OFFSET_INLINED:
+      return (uintptr_t) hostaddrs[i];
+
+    case OFFSET_POINTER:
+      return 0;
+
+    case OFFSET_STRUCT:
+      return tgt->list[i + 1].key->tgt->tgt_start
+	     + tgt->list[i + 1].key->tgt_offset
+	     + tgt->list[i + 1].offset
+	     + (uintptr_t) hostaddrs[i]
+	     - (uintptr_t) hostaddrs[i + 1];
+
+    default:
+      return tgt->tgt_start + tgt->list[i].offset;
+    }
 }
 
 static inline __attribute__((always_inline)) struct target_mem_desc *
@@ -575,7 +585,7 @@  gomp_map_vars_internal (struct gomp_device_descr *devicep,
 	  || (kind & typemask) == GOMP_MAP_FIRSTPRIVATE_INT)
 	{
 	  tgt->list[i].key = NULL;
-	  tgt->list[i].offset = ~(uintptr_t) 0;
+	  tgt->list[i].offset = OFFSET_INLINED;
 	  continue;
 	}
       else if ((kind & typemask) == GOMP_MAP_USE_DEVICE_PTR)
@@ -596,7 +606,7 @@  gomp_map_vars_internal (struct gomp_device_descr *devicep,
 	  cur_node.host_end = (uintptr_t) hostaddrs[last]
 			      + sizes[last];
 	  tgt->list[i].key = NULL;
-	  tgt->list[i].offset = ~(uintptr_t) 2;
+	  tgt->list[i].offset = OFFSET_STRUCT;
 	  splay_tree_key n = splay_tree_lookup (mem_map, &cur_node);
 	  if (n == NULL)
 	    {
@@ -629,7 +639,7 @@  gomp_map_vars_internal (struct gomp_device_descr *devicep,
       else if ((kind & typemask) == GOMP_MAP_ALWAYS_POINTER)
 	{
 	  tgt->list[i].key = NULL;
-	  tgt->list[i].offset = ~(uintptr_t) 1;
+	  tgt->list[i].offset = OFFSET_POINTER;
 	  has_firstprivate = true;
 	  continue;
 	}
@@ -659,7 +669,7 @@  gomp_map_vars_internal (struct gomp_device_descr *devicep,
 	  if (!n)
 	    {
 	      tgt->list[i].key = NULL;
-	      tgt->list[i].offset = ~(uintptr_t) 1;
+	      tgt->list[i].offset = OFFSET_POINTER;
 	      continue;
 	    }
 	}
@@ -884,12 +894,12 @@  gomp_map_vars_internal (struct gomp_device_descr *devicep,
 		size_t align = (size_t) 1 << (kind >> rshift);
 		tgt->list[i].key = k;
 		k->tgt = tgt;
-		if (field_tgt_clear != ~(size_t) 0)
+		if (field_tgt_clear != FIELD_TGT_EMPTY)
 		  {
 		    k->tgt_offset = k->host_start - field_tgt_base
 				    + field_tgt_offset;
 		    if (i == field_tgt_clear)
-		      field_tgt_clear = ~(size_t) 0;
+		      field_tgt_clear = FIELD_TGT_EMPTY;
 		  }
 		else
 		  {