Re: For libgomp OpenACC entry points, redefine the "device" argument to "flags"

[Thomas Schwinge <thomas@codesourcery.com>]

Hi!

On Wed, 19 Dec 2018 22:56:05 +0100, I wrote:
> On Wed, 19 Dec 2018 15:18:12 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Wed, Dec 19, 2018 at 03:03:42PM +0100, Jakub Jelinek wrote:
> > > On Wed, Dec 19, 2018 at 02:59:54PM +0100, Thomas Schwinge wrote:
> > > > Right.  For OpenACC, there's no "device" clause, so we only ever passed
> > > > in "GOMP_DEVICE_ICV" (default), or "GOMP_DEVICE_HOST_FALLBACK" ("if
> > > > (false)" clause).  Therefore, the libgomp "resolve_legacy_flags" function
> > > > added to make sure that these two values (as used by old executables)
> > > > continue to work as before (with new libgomp).  (And, we have to make
> > > > sure that no (new) "GOACC_FLAG_*" combination ever results in these
> > > > values; will document that.)
> 
> > > LGTM then in principle.

> Is the attached OK for trunk?

No further comments received; committed the following to trunk in
r267448:

commit 813421cdb9712da2a4f3804c35b4a7328e81f88e
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri Dec 28 11:34:14 2018 +0000

    For libgomp OpenACC entry points, redefine the "device" argument to "flags"
    
    ... so that we're then able to use this for other flags in addition to
    "GOACC_FLAG_HOST_FALLBACK".
    
            gcc/
            * omp-expand.c (expand_omp_target): Restructure OpenACC vs. OpenMP
            code paths.  Update for libgomp OpenACC entry points change.
            include/
            * gomp-constants.h (GOACC_FLAG_HOST_FALLBACK)
            (GOACC_FLAGS_MARSHAL_OP, GOACC_FLAGS_UNMARSHAL): Define.
            libgomp/
            * oacc-parallel.c (GOACC_parallel_keyed, GOACC_parallel)
            (GOACC_data_start, GOACC_enter_exit_data, GOACC_update)
            (GOACC_declare): Redefine the "device" argument to "flags".
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@267448 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog              |   5 ++
 gcc/omp-expand.c           | 115 +++++++++++++++++++++++++++++----------------
 gcc/tree-ssa-structalias.c |   4 +-
 include/ChangeLog          |   5 ++
 include/gomp-constants.h   |  12 +++++
 libgomp/ChangeLog          |   6 +++
 libgomp/oacc-parallel.c    |  52 ++++++++++++--------
 7 files changed, 137 insertions(+), 62 deletions(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index feaa4251815a..741c02d6cdae 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,8 @@
+2018-12-28  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* omp-expand.c (expand_omp_target): Restructure OpenACC vs. OpenMP
+	code paths.  Update for libgomp OpenACC entry points change.
+
 2018-12-28  Thomas Schwinge  <thomas@codesourcery.com>
 	    Julian Brown  <julian@codesourcery.com>
 
diff --git gcc/omp-expand.c gcc/omp-expand.c
index b5cb4241508e..79bc9acb7711 100644
--- gcc/omp-expand.c
+++ gcc/omp-expand.c
@@ -7496,9 +7496,8 @@ expand_omp_target (struct omp_region *region)
 
   /* Emit a library call to launch the offloading region, or do data
      transfers.  */
-  tree t1, t2, t3, t4, device, cond, depend, c, clauses;
+  tree t1, t2, t3, t4, depend, c, clauses;
   enum built_in_function start_ix;
-  location_t clause_loc;
   unsigned int flags_i = 0;
 
   switch (gimple_omp_target_kind (entry_stmt))
@@ -7542,49 +7541,63 @@ expand_omp_target (struct omp_region *region)
 
   clauses = gimple_omp_target_clauses (entry_stmt);
 
-  /* By default, the value of DEVICE is GOMP_DEVICE_ICV (let runtime
-     library choose) and there is no conditional.  */
-  cond = NULL_TREE;
-  device = build_int_cst (integer_type_node, GOMP_DEVICE_ICV);
+  tree device = NULL_TREE;
+  location_t device_loc = UNKNOWN_LOCATION;
+  tree goacc_flags = NULL_TREE;
+  if (is_gimple_omp_oacc (entry_stmt))
+    {
+      /* By default, no GOACC_FLAGs are set.  */
+      goacc_flags = integer_zero_node;
+    }
+  else
+    {
+      c = omp_find_clause (clauses, OMP_CLAUSE_DEVICE);
+      if (c)
+	{
+	  device = OMP_CLAUSE_DEVICE_ID (c);
+	  device_loc = OMP_CLAUSE_LOCATION (c);
+	}
+      else
+	{
+	  /* By default, the value of DEVICE is GOMP_DEVICE_ICV (let runtime
+	     library choose).  */
+	  device = build_int_cst (integer_type_node, GOMP_DEVICE_ICV);
+	  device_loc = gimple_location (entry_stmt);
+	}
 
+      c = omp_find_clause (clauses, OMP_CLAUSE_NOWAIT);
+      if (c)
+	flags_i |= GOMP_TARGET_FLAG_NOWAIT;
+    }
+
+  /* By default, there is no conditional.  */
+  tree cond = NULL_TREE;
   c = omp_find_clause (clauses, OMP_CLAUSE_IF);
   if (c)
     cond = OMP_CLAUSE_IF_EXPR (c);
-
-  c = omp_find_clause (clauses, OMP_CLAUSE_DEVICE);
-  if (c)
-    {
-      /* Even if we pass it to all library function calls, it is currently only
-	 defined/used for the OpenMP target ones.  */
-      gcc_checking_assert (start_ix == BUILT_IN_GOMP_TARGET
-			   || start_ix == BUILT_IN_GOMP_TARGET_DATA
-			   || start_ix == BUILT_IN_GOMP_TARGET_UPDATE
-			   || start_ix == BUILT_IN_GOMP_TARGET_ENTER_EXIT_DATA);
-
-      device = OMP_CLAUSE_DEVICE_ID (c);
-      clause_loc = OMP_CLAUSE_LOCATION (c);
-    }
-  else
-    clause_loc = gimple_location (entry_stmt);
-
-  c = omp_find_clause (clauses, OMP_CLAUSE_NOWAIT);
-  if (c)
-    flags_i |= GOMP_TARGET_FLAG_NOWAIT;
-
-  /* Ensure 'device' is of the correct type.  */
-  device = fold_convert_loc (clause_loc, integer_type_node, device);
-
-  /* If we found the clause 'if (cond)', build
-     (cond ? device : GOMP_DEVICE_HOST_FALLBACK).  */
+  /* If we found the clause 'if (cond)', build:
+     OpenACC: goacc_flags = (cond ? goacc_flags : flags | GOACC_FLAG_HOST_FALLBACK)
+     OpenMP: device = (cond ? device : GOMP_DEVICE_HOST_FALLBACK) */
   if (cond)
     {
+      tree *tp;
+      if (is_gimple_omp_oacc (entry_stmt))
+	tp = &goacc_flags;
+      else
+	{
+	  /* Ensure 'device' is of the correct type.  */
+	  device = fold_convert_loc (device_loc, integer_type_node, device);
+
+	  tp = &device;
+	}
+
       cond = gimple_boolify (cond);
 
       basic_block cond_bb, then_bb, else_bb;
       edge e;
       tree tmp_var;
 
-      tmp_var = create_tmp_var (TREE_TYPE (device));
+      tmp_var = create_tmp_var (TREE_TYPE (*tp));
       if (offloaded)
 	e = split_block_after_labels (new_bb);
       else
@@ -7607,13 +7620,20 @@ expand_omp_target (struct omp_region *region)
       gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
 
       gsi = gsi_start_bb (then_bb);
-      stmt = gimple_build_assign (tmp_var, device);
+      stmt = gimple_build_assign (tmp_var, *tp);
       gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
 
       gsi = gsi_start_bb (else_bb);
-      stmt = gimple_build_assign (tmp_var,
-				  build_int_cst (integer_type_node,
-						 GOMP_DEVICE_HOST_FALLBACK));
+      if (is_gimple_omp_oacc (entry_stmt))
+	stmt = gimple_build_assign (tmp_var,
+				    BIT_IOR_EXPR,
+				    *tp,
+				    build_int_cst (integer_type_node,
+						   GOACC_FLAG_HOST_FALLBACK));
+      else
+	stmt = gimple_build_assign (tmp_var,
+				    build_int_cst (integer_type_node,
+						   GOMP_DEVICE_HOST_FALLBACK));
       gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
 
       make_edge (cond_bb, then_bb, EDGE_TRUE_VALUE);
@@ -7623,14 +7643,17 @@ expand_omp_target (struct omp_region *region)
       make_edge (then_bb, new_bb, EDGE_FALLTHRU);
       make_edge (else_bb, new_bb, EDGE_FALLTHRU);
 
-      device = tmp_var;
+      *tp = tmp_var;
+
       gsi = gsi_last_nondebug_bb (new_bb);
     }
   else
     {
       gsi = gsi_last_nondebug_bb (new_bb);
-      device = force_gimple_operand_gsi (&gsi, device, true, NULL_TREE,
-					 true, GSI_SAME_STMT);
+
+      if (device != NULL_TREE)
+	device = force_gimple_operand_gsi (&gsi, device, true, NULL_TREE,
+					   true, GSI_SAME_STMT);
     }
 
   t = gimple_omp_target_data_arg (entry_stmt);
@@ -7654,7 +7677,17 @@ expand_omp_target (struct omp_region *region)
   bool tagging = false;
   /* The maximum number used by any start_ix, without varargs.  */
   auto_vec<tree, 11> args;
-  args.quick_push (device);
+  if (is_gimple_omp_oacc (entry_stmt))
+    {
+      tree goacc_flags_m = fold_build1 (GOACC_FLAGS_MARSHAL_OP,
+					TREE_TYPE (goacc_flags), goacc_flags);
+      goacc_flags_m = force_gimple_operand_gsi (&gsi, goacc_flags_m, true,
+						NULL_TREE, true,
+						GSI_SAME_STMT);
+      args.quick_push (goacc_flags_m);
+    }
+  else
+    args.quick_push (device);
   if (offloaded)
     args.quick_push (build_fold_addr_expr (child_fn));
   args.quick_push (t1);
diff --git gcc/tree-ssa-structalias.c gcc/tree-ssa-structalias.c
index fc85e9ded5e8..cfdfb50854f0 100644
--- gcc/tree-ssa-structalias.c
+++ gcc/tree-ssa-structalias.c
@@ -4697,7 +4697,7 @@ find_func_aliases_for_builtin_call (struct function *fn, gcall *t)
 		  argpos = 1;
 		  break;
 		case BUILT_IN_GOACC_PARALLEL:
-		  /* __builtin_GOACC_parallel (device, fn, mapnum, hostaddrs,
+		  /* __builtin_GOACC_parallel (flags_m, fn, mapnum, hostaddrs,
 					       sizes, kinds, ...).  */
 		  fnpos = 1;
 		  argpos = 3;
@@ -5255,7 +5255,7 @@ find_func_clobbers (struct function *fn, gimple *origt)
 		  argpos = 1;
 		  break;
 		case BUILT_IN_GOACC_PARALLEL:
-		  /* __builtin_GOACC_parallel (device, fn, mapnum, hostaddrs,
+		  /* __builtin_GOACC_parallel (flags_m, fn, mapnum, hostaddrs,
 					       sizes, kinds, ...).  */
 		  fnpos = 1;
 		  argpos = 3;
diff --git include/ChangeLog include/ChangeLog
index 74b5a1f1dd45..886e9a6ca35c 100644
--- include/ChangeLog
+++ include/ChangeLog
@@ -1,3 +1,8 @@
+2018-12-28  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* gomp-constants.h (GOACC_FLAG_HOST_FALLBACK)
+	(GOACC_FLAGS_MARSHAL_OP, GOACC_FLAGS_UNMARSHAL): Define.
+
 2018-12-22  Jason Merrill  <jason@redhat.com>
 
 	* demangle.h: Remove support for ancient GNU (pre-3.0), Lucid,
diff --git include/gomp-constants.h include/gomp-constants.h
index d3e64d4e352a..b6b6733bd875 100644
--- include/gomp-constants.h
+++ include/gomp-constants.h
@@ -197,6 +197,18 @@ enum gomp_map_kind
 /* Internal to libgomp.  */
 #define GOMP_TARGET_FLAG_UPDATE		(1U << 31)
 
+
+/* OpenACC construct flags.  */
+
+/* Force host fallback execution.  */
+#define GOACC_FLAG_HOST_FALLBACK	(1 << 0)
+
+/* For legacy reasons, in the ABI, the GOACC_FLAGs are encoded as an inverted
+   bitmask.  */
+#define GOACC_FLAGS_MARSHAL_OP		BIT_NOT_EXPR
+#define GOACC_FLAGS_UNMARSHAL(X)	(~(X))
+
+
 /* Versions of libgomp and device-specific plugins.  GOMP_VERSION
    should be incremented whenever an ABI-incompatible change is introduced
    to the plugin interface defined in libgomp/libgomp.h.  */
diff --git libgomp/ChangeLog libgomp/ChangeLog
index 5b014b032beb..aa69f42d3ca0 100644
--- libgomp/ChangeLog
+++ libgomp/ChangeLog
@@ -1,3 +1,9 @@
+2018-12-28  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* oacc-parallel.c (GOACC_parallel_keyed, GOACC_parallel)
+	(GOACC_data_start, GOACC_enter_exit_data, GOACC_update)
+	(GOACC_declare): Redefine the "device" argument to "flags".
+
 2018-12-28  Thomas Schwinge  <thomas@codesourcery.com>
 	    Cesar Philippidis  <cesar@codesourcery.com>
 
diff --git libgomp/oacc-parallel.c libgomp/oacc-parallel.c
index e2405d73d5a9..57bd484ba8a1 100644
--- libgomp/oacc-parallel.c
+++ libgomp/oacc-parallel.c
@@ -38,6 +38,16 @@
 #include <stdarg.h>
 #include <assert.h>
 
+
+/* In the ABI, the GOACC_FLAGs are encoded as an inverted bitmask, so that we
+   continue to support the following two legacy values.  */
+_Static_assert (GOACC_FLAGS_UNMARSHAL (GOMP_DEVICE_ICV) == 0,
+		"legacy GOMP_DEVICE_ICV broken");
+_Static_assert (GOACC_FLAGS_UNMARSHAL (GOMP_DEVICE_HOST_FALLBACK)
+		== GOACC_FLAG_HOST_FALLBACK,
+		"legacy GOMP_DEVICE_HOST_FALLBACK broken");
+
+
 /* Returns the number of mappings associated with the pointer or pset. PSET
    have three mappings, whereas pointer have two.  */
 
@@ -105,17 +115,18 @@ handle_ftn_pointers (size_t mapnum, void **hostaddrs, size_t *sizes,
 static void goacc_wait (int async, int num_waits, va_list *ap);
 
 
-/* Launch a possibly offloaded function on DEVICE.  FN is the host fn
+/* Launch a possibly offloaded function with FLAGS.  FN is the host fn
    address.  MAPNUM, HOSTADDRS, SIZES & KINDS  describe the memory
    blocks to be copied to/from the device.  Varadic arguments are
    keyed optional parameters terminated with a zero.  */
 
 void
-GOACC_parallel_keyed (int device, void (*fn) (void *),
+GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
 		      size_t mapnum, void **hostaddrs, size_t *sizes,
 		      unsigned short *kinds, ...)
 {
-  bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK;
+  int flags = GOACC_FLAGS_UNMARSHAL (flags_m);
+
   va_list ap;
   struct goacc_thread *thr;
   struct gomp_device_descr *acc_dev;
@@ -145,7 +156,7 @@ GOACC_parallel_keyed (int device, void (*fn) (void *),
 
   /* Host fallback if "if" clause is false or if the current device is set to
      the host.  */
-  if (host_fallback)
+  if (flags & GOACC_FLAG_HOST_FALLBACK)
     {
       goacc_save_and_set_bind (acc_device_host);
       fn (hostaddrs);
@@ -269,7 +280,7 @@ GOACC_parallel_keyed (int device, void (*fn) (void *),
 /* Legacy entry point, only provide host execution.  */
 
 void
-GOACC_parallel (int device, void (*fn) (void *),
+GOACC_parallel (int flags_m, void (*fn) (void *),
 		size_t mapnum, void **hostaddrs, size_t *sizes,
 		unsigned short *kinds,
 		int num_gangs, int num_workers, int vector_length,
@@ -281,10 +292,11 @@ GOACC_parallel (int device, void (*fn) (void *),
 }
 
 void
-GOACC_data_start (int device, size_t mapnum,
+GOACC_data_start (int flags_m, size_t mapnum,
 		  void **hostaddrs, size_t *sizes, unsigned short *kinds)
 {
-  bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK;
+  int flags = GOACC_FLAGS_UNMARSHAL (flags_m);
+
   struct target_mem_desc *tgt;
 
 #ifdef HAVE_INTTYPES_H
@@ -302,7 +314,7 @@ GOACC_data_start (int device, size_t mapnum,
 
   /* Host fallback or 'do nothing'.  */
   if ((acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
-      || host_fallback)
+      || (flags & GOACC_FLAG_HOST_FALLBACK))
     {
       tgt = gomp_map_vars (NULL, 0, NULL, NULL, NULL, NULL, true,
 			   GOMP_MAP_VARS_OPENACC);
@@ -333,13 +345,14 @@ GOACC_data_end (void)
 }
 
 void
-GOACC_enter_exit_data (int device, size_t mapnum,
+GOACC_enter_exit_data (int flags_m, size_t mapnum,
 		       void **hostaddrs, size_t *sizes, unsigned short *kinds,
 		       int async, int num_waits, ...)
 {
+  int flags = GOACC_FLAGS_UNMARSHAL (flags_m);
+
   struct goacc_thread *thr;
   struct gomp_device_descr *acc_dev;
-  bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK;
   bool data_enter = false;
   size_t i;
 
@@ -349,7 +362,7 @@ GOACC_enter_exit_data (int device, size_t mapnum,
   acc_dev = thr->dev;
 
   if ((acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
-      || host_fallback)
+      || (flags & GOACC_FLAG_HOST_FALLBACK))
     return;
 
   if (num_waits)
@@ -523,11 +536,12 @@ goacc_wait (int async, int num_waits, va_list *ap)
 }
 
 void
-GOACC_update (int device, size_t mapnum,
+GOACC_update (int flags_m, size_t mapnum,
 	      void **hostaddrs, size_t *sizes, unsigned short *kinds,
 	      int async, int num_waits, ...)
 {
-  bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK;
+  int flags = GOACC_FLAGS_UNMARSHAL (flags_m);
+
   size_t i;
 
   goacc_lazy_initialize ();
@@ -536,7 +550,7 @@ GOACC_update (int device, size_t mapnum,
   struct gomp_device_descr *acc_dev = thr->dev;
 
   if ((acc_dev->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
-      || host_fallback)
+      || (flags & GOACC_FLAG_HOST_FALLBACK))
     return;
 
   if (num_waits)
@@ -643,7 +657,7 @@ GOACC_get_thread_num (void)
 }
 
 void
-GOACC_declare (int device, size_t mapnum,
+GOACC_declare (int flags_m, size_t mapnum,
 	       void **hostaddrs, size_t *sizes, unsigned short *kinds)
 {
   int i;
@@ -663,7 +677,7 @@ GOACC_declare (int device, size_t mapnum,
 	  case GOMP_MAP_POINTER:
 	  case GOMP_MAP_RELEASE:
 	  case GOMP_MAP_DELETE:
-	    GOACC_enter_exit_data (device, 1, &hostaddrs[i], &sizes[i],
+	    GOACC_enter_exit_data (flags_m, 1, &hostaddrs[i], &sizes[i],
 				   &kinds[i], GOMP_ASYNC_SYNC, 0);
 	    break;
 
@@ -672,18 +686,18 @@ GOACC_declare (int device, size_t mapnum,
 
 	  case GOMP_MAP_ALLOC:
 	    if (!acc_is_present (hostaddrs[i], sizes[i]))
-	      GOACC_enter_exit_data (device, 1, &hostaddrs[i], &sizes[i],
+	      GOACC_enter_exit_data (flags_m, 1, &hostaddrs[i], &sizes[i],
 				     &kinds[i], GOMP_ASYNC_SYNC, 0);
 	    break;
 
 	  case GOMP_MAP_TO:
-	    GOACC_enter_exit_data (device, 1, &hostaddrs[i], &sizes[i],
+	    GOACC_enter_exit_data (flags_m, 1, &hostaddrs[i], &sizes[i],
 				   &kinds[i], GOMP_ASYNC_SYNC, 0);
 
 	    break;
 
 	  case GOMP_MAP_FROM:
-	    GOACC_enter_exit_data (device, 1, &hostaddrs[i], &sizes[i],
+	    GOACC_enter_exit_data (flags_m, 1, &hostaddrs[i], &sizes[i],
 				   &kinds[i], GOMP_ASYNC_SYNC, 0);
 	    break;
 


Grüße
 Thomas