public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* OpenACC 2.6 "host_data" construct, "if_present" clause
       [not found]   ` <yxfpefaesaov.fsf@hertz.schwinge.homeip.net>
@ 2018-12-18 23:24     ` Thomas Schwinge
  2018-12-19 12:46       ` For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" (was: OpenACC 2.6 "host_data" construct, "if_present" clause) Thomas Schwinge
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Schwinge @ 2018-12-18 23:24 UTC (permalink / raw)
  To: Jakub Jelinek, gcc; +Cc: Gergö Barany

Hi Jakub!

OpenACC 2.6 adds a new clause to the "host_data" construct:
2.8.3. "if_present clause".  Gergő (in CC) is working on that.

    When an 'if_present' clause appears on the directive, the compiler
    will only change the address of any variable or array which appears
    in _var-list_ that is present on the current device.

So, basically:

    --- a/libgomp/target.c
    +++ b/libgomp/target.c
    @@ -1130,13 +1130,17 @@ gomp_map_vars_async (struct gomp_device_descr *devicep,
           else if ((kind & typemask) == GOMP_MAP_USE_DEVICE_PTR)
             {
               cur_node.host_start = (uintptr_t) hostaddrs[i];
               cur_node.host_end = cur_node.host_start;
               splay_tree_key n = gomp_map_lookup (mem_map, &cur_node);
               if (n == NULL)
                 {
    +              if ([...])
    +                /* No error, continue using the host address.  */
    +                continue;
                   gomp_mutex_unlock (&devicep->lock);
                   gomp_fatal ("use_device_ptr pointer wasn't mapped");
                 }

Note that this clause applies to *all* "use_device"
("GOMP_MAP_USE_DEVICE_PTR") clauses present on the "host_data" construct,
so it's just a single bit flag for the construct.

Do you suggest we yet add a new mapping kind
"GOMP_MAP_USE_DEVICE_PTR_IF_PRESENT" for that?  And, any preference about
the specific value to use?  Gergő proposed:

    --- a/include/gomp-constants.h
    +++ b/include/gomp-constants.h
    @@ -80,6 +80,10 @@ enum gomp_map_kind
         GOMP_MAP_DEVICE_RESIDENT =		(GOMP_MAP_FLAG_SPECIAL_1 | 1),
         /* OpenACC link.  */
         GOMP_MAP_LINK =			(GOMP_MAP_FLAG_SPECIAL_1 | 2),
    +    /* Like GOMP_MAP_USE_DEVICE_PTR below, translate a host to a device
    +       address.  If translation fails because the target is not mapped,
    +       continue using the host address. */
    +    GOMP_MAP_USE_DEVICE_PTR_IF_PRESENT =		(GOMP_MAP_FLAG_SPECIAL_1 | 3),
         /* Allocate.  */
         GOMP_MAP_FIRSTPRIVATE =		(GOMP_MAP_FLAG_SPECIAL | 0),
         /* Similarly, but store the value in the pointer rather than

Or, I had the idea that we could avoid that, instead continue using
"GOMP_MAP_USE_DEVICE_PTR", and transmit the "if_present" flag through the
"int device" argument of "GOACC_data_start" (making sure that old
executables continue to function as before).  For OpenACC, that argument
is only ever set to "GOMP_DEVICE_ICV" or "GOMP_DEVICE_HOST_FALLBACK" (for
"if" clause evaluating to "false"), so has some bits to spare for that.
However, I've not been able to convince myself that this solution would
be any much prettier than adding a new mapping kind...  ;-)


Grüße
 Thomas

^ permalink raw reply	[flat|nested] 8+ messages in thread

* For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" (was: OpenACC 2.6 "host_data" construct, "if_present" clause)
  2018-12-18 23:24     ` OpenACC 2.6 "host_data" construct, "if_present" clause Thomas Schwinge
@ 2018-12-19 12:46       ` Thomas Schwinge
  2018-12-19 13:38         ` Jakub Jelinek
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Schwinge @ 2018-12-19 12:46 UTC (permalink / raw)
  To: Jakub Jelinek, gcc, Gergö Barany

[-- Attachment #1: Type: text/plain, Size: 3476 bytes --]

Hi!

On Wed, 19 Dec 2018 00:24:00 +0100, I wrote:
> OpenACC 2.6 adds a new clause to the "host_data" construct:
> 2.8.3. "if_present clause".  Gergő (in CC) is working on that.
> 
>     When an 'if_present' clause appears on the directive, the compiler
>     will only change the address of any variable or array which appears
>     in _var-list_ that is present on the current device.
> 
> So, basically:
> 
>     --- a/libgomp/target.c
>     +++ b/libgomp/target.c
>     @@ -1130,13 +1130,17 @@ gomp_map_vars_async (struct gomp_device_descr *devicep,
>            else if ((kind & typemask) == GOMP_MAP_USE_DEVICE_PTR)
>              {
>                cur_node.host_start = (uintptr_t) hostaddrs[i];
>                cur_node.host_end = cur_node.host_start;
>                splay_tree_key n = gomp_map_lookup (mem_map, &cur_node);
>                if (n == NULL)
>                  {
>     +              if ([...])
>     +                /* No error, continue using the host address.  */
>     +                continue;
>                    gomp_mutex_unlock (&devicep->lock);
>                    gomp_fatal ("use_device_ptr pointer wasn't mapped");
>                  }
> 
> Note that this clause applies to *all* "use_device"
> ("GOMP_MAP_USE_DEVICE_PTR") clauses present on the "host_data" construct,
> so it's just a single bit flag for the construct.
> 
> Do you suggest we yet add a new mapping kind
> "GOMP_MAP_USE_DEVICE_PTR_IF_PRESENT" for that?  And, any preference about
> the specific value to use?  Gergő proposed:
> 
>     --- a/include/gomp-constants.h
>     +++ b/include/gomp-constants.h
>     @@ -80,6 +80,10 @@ enum gomp_map_kind
>          GOMP_MAP_DEVICE_RESIDENT =		(GOMP_MAP_FLAG_SPECIAL_1 | 1),
>          /* OpenACC link.  */
>          GOMP_MAP_LINK =			(GOMP_MAP_FLAG_SPECIAL_1 | 2),
>     +    /* Like GOMP_MAP_USE_DEVICE_PTR below, translate a host to a device
>     +       address.  If translation fails because the target is not mapped,
>     +       continue using the host address. */
>     +    GOMP_MAP_USE_DEVICE_PTR_IF_PRESENT =		(GOMP_MAP_FLAG_SPECIAL_1 | 3),
>          /* Allocate.  */
>          GOMP_MAP_FIRSTPRIVATE =		(GOMP_MAP_FLAG_SPECIAL | 0),
>          /* Similarly, but store the value in the pointer rather than
> 
> Or, I had the idea that we could avoid that, instead continue using
> "GOMP_MAP_USE_DEVICE_PTR", and transmit the "if_present" flag through the
> "int device" argument of "GOACC_data_start" (making sure that old
> executables continue to function as before).  For OpenACC, that argument
> is only ever set to "GOMP_DEVICE_ICV" or "GOMP_DEVICE_HOST_FALLBACK" (for
> "if" clause evaluating to "false"), so has some bits to spare for that.
> However, I've not been able to convince myself that this solution would
> be any much prettier than adding a new mapping kind...  ;-)

Having thought about it some more, the idea doesn't actually seem so bad
anymore.  :-) Just don't think of it as 'merging stuff into "int
device"', but rather 'for OpenACC libgomp entry points, redefine the "int
device" argument to "unsigned int flags"' -- see attached WIP (for GCC
trunk, testing).

Jakub, what do you think?


For the "if_present" clause, we'd then initialize "tree flags" not to
zero but to "omp_find_clause (OMP_CLAUSE_IF_PRESENT) ?
GOACC_FLAG_IF_PRESENT : 0" or similar, and then handle that in libgomp.


Grüße
 Thomas



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-For-OpenACC-libgomp-entry-points-redefine-the-int-de.patch --]
[-- Type: text/x-diff, Size: 16492 bytes --]

From 1a45ffa3da47f1ec2e62badc31ae31454f4351d9 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 19 Dec 2018 13:37:00 +0100
Subject: [PATCH] For OpenACC libgomp entry points, redefine the "int device"
 argument to "unsigned int flags"

---
 gcc/builtin-types.def      | 10 +++---
 gcc/fortran/types.def      | 10 +++---
 gcc/omp-builtins.def       | 10 +++---
 gcc/omp-expand.c           | 68 +++++++++++++++++++++++++++++++++++++-
 gcc/tree-ssa-structalias.c |  4 +--
 include/gomp-constants.h   |  3 ++
 libgomp/libgomp_g.h        | 12 +++----
 libgomp/oacc-parallel.c    | 53 ++++++++++++++++++++---------
 8 files changed, 133 insertions(+), 37 deletions(-)

diff --git a/gcc/builtin-types.def b/gcc/builtin-types.def
index 685b22f975a1..f92580d5c649 100644
--- a/gcc/builtin-types.def
+++ b/gcc/builtin-types.def
@@ -684,6 +684,8 @@ DEF_FUNCTION_TYPE_5 (BT_FN_BOOL_VPTR_PTR_I16_INT_INT,
 		     BT_BOOL, BT_VOLATILE_PTR, BT_PTR, BT_I16, BT_INT, BT_INT)
 DEF_FUNCTION_TYPE_5 (BT_FN_VOID_INT_SIZE_PTR_PTR_PTR,
 		     BT_VOID, BT_INT, BT_SIZE, BT_PTR, BT_PTR, BT_PTR)
+DEF_FUNCTION_TYPE_5 (BT_FN_VOID_UINT_SIZE_PTR_PTR_PTR,
+		     BT_VOID, BT_UINT, BT_SIZE, BT_PTR, BT_PTR, BT_PTR)
 DEF_FUNCTION_TYPE_5 (BT_FN_VOID_OMPFN_PTR_UINT_UINT_UINT,
 		     BT_VOID, BT_PTR_FN_VOID_PTR, BT_PTR, BT_UINT, BT_UINT,
 		     BT_UINT)
@@ -822,12 +824,12 @@ DEF_FUNCTION_TYPE_VAR_5 (BT_FN_INT_STRING_SIZE_INT_SIZE_CONST_STRING_VAR,
 DEF_FUNCTION_TYPE_VAR_5 (BT_FN_INT_INT_INT_INT_INT_INT_VAR,
 			 BT_INT, BT_INT, BT_INT, BT_INT, BT_INT, BT_INT)
 
-DEF_FUNCTION_TYPE_VAR_6 (BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_VAR,
-			 BT_VOID, BT_INT, BT_PTR_FN_VOID_PTR, BT_SIZE,
+DEF_FUNCTION_TYPE_VAR_6 (BT_FN_VOID_UINT_OMPFN_SIZE_PTR_PTR_PTR_VAR,
+			 BT_VOID, BT_UINT, BT_PTR_FN_VOID_PTR, BT_SIZE,
 			 BT_PTR, BT_PTR, BT_PTR)
 
-DEF_FUNCTION_TYPE_VAR_7 (BT_FN_VOID_INT_SIZE_PTR_PTR_PTR_INT_INT_VAR,
-			 BT_VOID, BT_INT, BT_SIZE, BT_PTR, BT_PTR,
+DEF_FUNCTION_TYPE_VAR_7 (BT_FN_VOID_UINT_SIZE_PTR_PTR_PTR_INT_INT_VAR,
+			 BT_VOID, BT_UINT, BT_SIZE, BT_PTR, BT_PTR,
 			 BT_PTR, BT_INT, BT_INT)
 
 DEF_POINTER_TYPE (BT_PTR_FN_VOID_VAR, BT_FN_VOID_VAR)
diff --git a/gcc/fortran/types.def b/gcc/fortran/types.def
index 0eabc3f4d993..593d18c0cfd2 100644
--- a/gcc/fortran/types.def
+++ b/gcc/fortran/types.def
@@ -178,6 +178,8 @@ DEF_FUNCTION_TYPE_5 (BT_FN_VOID_SIZE_VPTR_PTR_PTR_INT, BT_VOID, BT_SIZE,
 		     BT_VOLATILE_PTR, BT_PTR, BT_PTR, BT_INT)
 DEF_FUNCTION_TYPE_5 (BT_FN_VOID_INT_SIZE_PTR_PTR_PTR,
 		     BT_VOID, BT_INT, BT_SIZE, BT_PTR, BT_PTR, BT_PTR)
+DEF_FUNCTION_TYPE_5 (BT_FN_VOID_UINT_SIZE_PTR_PTR_PTR,
+		     BT_VOID, BT_UINT, BT_SIZE, BT_PTR, BT_PTR, BT_PTR)
 DEF_FUNCTION_TYPE_5 (BT_FN_BOOL_UINT_LONGPTR_LONG_LONGPTR_LONGPTR,
 		     BT_BOOL, BT_UINT, BT_PTR_LONG, BT_LONG, BT_PTR_LONG,
 		     BT_PTR_LONG)
@@ -265,10 +267,10 @@ DEF_FUNCTION_TYPE_VAR_1 (BT_FN_VOID_ULL_VAR,
 
 DEF_FUNCTION_TYPE_VAR_2 (BT_FN_VOID_INT_INT_VAR, BT_VOID, BT_INT, BT_INT)
 
-DEF_FUNCTION_TYPE_VAR_7 (BT_FN_VOID_INT_SIZE_PTR_PTR_PTR_INT_INT_VAR,
-			 BT_VOID, BT_INT, BT_SIZE, BT_PTR, BT_PTR,
+DEF_FUNCTION_TYPE_VAR_7 (BT_FN_VOID_UINT_SIZE_PTR_PTR_PTR_INT_INT_VAR,
+			 BT_VOID, BT_UINT, BT_SIZE, BT_PTR, BT_PTR,
 			 BT_PTR, BT_INT, BT_INT)
 
-DEF_FUNCTION_TYPE_VAR_6 (BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_VAR,
-			  BT_VOID, BT_INT, BT_PTR_FN_VOID_PTR, BT_SIZE,
+DEF_FUNCTION_TYPE_VAR_6 (BT_FN_VOID_UINT_OMPFN_SIZE_PTR_PTR_PTR_VAR,
+			  BT_VOID, BT_UINT, BT_PTR_FN_VOID_PTR, BT_SIZE,
 			  BT_PTR, BT_PTR, BT_PTR)
diff --git a/gcc/omp-builtins.def b/gcc/omp-builtins.def
index 187268097bca..8c7a0c204846 100644
--- a/gcc/omp-builtins.def
+++ b/gcc/omp-builtins.def
@@ -32,17 +32,17 @@ along with GCC; see the file COPYING3.  If not see
 DEF_GOACC_BUILTIN (BUILT_IN_ACC_GET_DEVICE_TYPE, "acc_get_device_type",
 		   BT_FN_INT, ATTR_NOTHROW_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_DATA_START, "GOACC_data_start",
-		   BT_FN_VOID_INT_SIZE_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
+		   BT_FN_VOID_UINT_SIZE_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_DATA_END, "GOACC_data_end",
 		   BT_FN_VOID, ATTR_NOTHROW_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_ENTER_EXIT_DATA, "GOACC_enter_exit_data",
-		   BT_FN_VOID_INT_SIZE_PTR_PTR_PTR_INT_INT_VAR,
+		   BT_FN_VOID_UINT_SIZE_PTR_PTR_PTR_INT_INT_VAR,
 		   ATTR_NOTHROW_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_PARALLEL, "GOACC_parallel_keyed",
-		   BT_FN_VOID_INT_OMPFN_SIZE_PTR_PTR_PTR_VAR,
+		   BT_FN_VOID_UINT_OMPFN_SIZE_PTR_PTR_PTR_VAR,
 		   ATTR_NOTHROW_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_UPDATE, "GOACC_update",
-		   BT_FN_VOID_INT_SIZE_PTR_PTR_PTR_INT_INT_VAR,
+		   BT_FN_VOID_UINT_SIZE_PTR_PTR_PTR_INT_INT_VAR,
 		   ATTR_NOTHROW_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_WAIT, "GOACC_wait",
 		   BT_FN_VOID_INT_INT_VAR,
@@ -445,4 +445,4 @@ DEF_GOMP_BUILTIN (BUILT_IN_GOMP_WORKSHARE_TASK_REDUCTION_UNREGISTER,
 		  "GOMP_workshare_task_reduction_unregister",
 		  BT_FN_VOID_BOOL, ATTR_NOTHROW_LEAF_LIST)
 DEF_GOACC_BUILTIN (BUILT_IN_GOACC_DECLARE, "GOACC_declare",
-		   BT_FN_VOID_INT_SIZE_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
+		   BT_FN_VOID_UINT_SIZE_PTR_PTR_PTR, ATTR_NOTHROW_LIST)
diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
index b6ff119f5446..77edbd79c95e 100644
--- a/gcc/omp-expand.c
+++ b/gcc/omp-expand.c
@@ -7540,6 +7540,8 @@ expand_omp_target (struct omp_region *region)
      library choose) and there is no conditional.  */
   cond = NULL_TREE;
   device = build_int_cst (integer_type_node, GOMP_DEVICE_ICV);
+  /* No flags set by default.  */
+  tree flags = build_int_cstu (unsigned_type_node, 0);
 
   c = omp_find_clause (clauses, OMP_CLAUSE_IF);
   if (c)
@@ -7565,6 +7567,66 @@ expand_omp_target (struct omp_region *region)
   if (c)
     flags_i |= GOMP_TARGET_FLAG_NOWAIT;
 
+  if (is_gimple_omp_oacc (entry_stmt))
+    {
+      //TODO Copied from below.  Merge code as much as feasible.
+      /* If we found the clause 'if (cond)', build
+	 TODO flags = (cond ? flags : GOACC_FLAG_HOST_FALLBACK).
+	 TODO Should be: flags = (cond ? flags : flags | GOACC_FLAG_HOST_FALLBACK).  */
+      if (cond)
+	{
+	  cond = gimple_boolify (cond);
+
+	  basic_block cond_bb, then_bb, else_bb;
+	  edge e;
+	  tree tmp_var;
+
+	  tmp_var = create_tmp_var (TREE_TYPE (flags));
+	  if (offloaded)
+	    e = split_block_after_labels (new_bb);
+	  else
+	    {
+	      gsi = gsi_last_nondebug_bb (new_bb);
+	      gsi_prev (&gsi);
+	      e = split_block (new_bb, gsi_stmt (gsi));
+	    }
+	  cond_bb = e->src;
+	  new_bb = e->dest;
+	  remove_edge (e);
+
+	  then_bb = create_empty_bb (cond_bb);
+	  else_bb = create_empty_bb (then_bb);
+	  set_immediate_dominator (CDI_DOMINATORS, then_bb, cond_bb);
+	  set_immediate_dominator (CDI_DOMINATORS, else_bb, cond_bb);
+
+	  stmt = gimple_build_cond_empty (cond);
+	  gsi = gsi_last_bb (cond_bb);
+	  gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
+
+	  gsi = gsi_start_bb (then_bb);
+	  stmt = gimple_build_assign (tmp_var, flags);
+	  gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
+
+	  gsi = gsi_start_bb (else_bb);
+	  stmt = gimple_build_assign (tmp_var,
+				      build_int_cst (unsigned_type_node,
+						     GOACC_FLAG_HOST_FALLBACK));
+	  gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
+
+	  make_edge (cond_bb, then_bb, EDGE_TRUE_VALUE);
+	  make_edge (cond_bb, else_bb, EDGE_FALSE_VALUE);
+	  add_bb_to_loop (then_bb, cond_bb->loop_father);
+	  add_bb_to_loop (else_bb, cond_bb->loop_father);
+	  make_edge (then_bb, new_bb, EDGE_FALLTHRU);
+	  make_edge (else_bb, new_bb, EDGE_FALLTHRU);
+
+	  flags = tmp_var;
+	}
+      gsi = gsi_last_nondebug_bb (new_bb);
+    }
+  else
+    //TODO Indentation.
+    {
   /* Ensure 'device' is of the correct type.  */
   device = fold_convert_loc (clause_loc, integer_type_node, device);
 
@@ -7626,6 +7688,7 @@ expand_omp_target (struct omp_region *region)
       device = force_gimple_operand_gsi (&gsi, device, true, NULL_TREE,
 					 true, GSI_SAME_STMT);
     }
+    }
 
   t = gimple_omp_target_data_arg (entry_stmt);
   if (t == NULL)
@@ -7648,7 +7711,10 @@ 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))
+    args.quick_push (flags);
+  else
+    args.quick_push (device);
   if (offloaded)
     args.quick_push (build_fold_addr_expr (child_fn));
   args.quick_push (t1);
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index fc85e9ded5e8..65cc47b22c5b 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/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, 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, fn, mapnum, hostaddrs,
 					       sizes, kinds, ...).  */
 		  fnpos = 1;
 		  argpos = 3;
diff --git a/include/gomp-constants.h b/include/gomp-constants.h
index add3896a4aa1..fa58ecd2c3c9 100644
--- a/include/gomp-constants.h
+++ b/include/gomp-constants.h
@@ -197,6 +197,9 @@ enum gomp_map_kind
 /* Internal to libgomp.  */
 #define GOMP_TARGET_FLAG_UPDATE		(1U << 31)
 
+/* TODO */
+#define GOACC_FLAG_HOST_FALLBACK	(1 << 0)
+
 /* 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 a/libgomp/libgomp_g.h b/libgomp/libgomp_g.h
index 5b54839b29e5..97f9dc023a81 100644
--- a/libgomp/libgomp_g.h
+++ b/libgomp/libgomp_g.h
@@ -359,20 +359,20 @@ extern void GOMP_teams_reg (void (*) (void *), void *, unsigned, unsigned,
 
 /* oacc-parallel.c */
 
-extern void GOACC_parallel_keyed (int, void (*) (void *), size_t,
+extern void GOACC_parallel_keyed (unsigned int, void (*) (void *), size_t,
 				  void **, size_t *, unsigned short *, ...);
-extern void GOACC_parallel (int, void (*) (void *), size_t, void **, size_t *,
+extern void GOACC_parallel (unsigned int, void (*) (void *), size_t, void **, size_t *,
 			    unsigned short *, int, int, int, int, int, ...);
-extern void GOACC_data_start (int, size_t, void **, size_t *,
+extern void GOACC_data_start (unsigned int, size_t, void **, size_t *,
 			      unsigned short *);
 extern void GOACC_data_end (void);
-extern void GOACC_enter_exit_data (int, size_t, void **,
+extern void GOACC_enter_exit_data (unsigned int, size_t, void **,
 				   size_t *, unsigned short *, int, int, ...);
-extern void GOACC_update (int, size_t, void **, size_t *,
+extern void GOACC_update (unsigned int, size_t, void **, size_t *,
 			  unsigned short *, int, int, ...);
 extern void GOACC_wait (int, int, ...);
 extern int GOACC_get_num_threads (void);
 extern int GOACC_get_thread_num (void);
-extern void GOACC_declare (int, size_t, void **, size_t *, unsigned short *);
+extern void GOACC_declare (unsigned int, size_t, void **, size_t *, unsigned short *);
 
 #endif /* LIBGOMP_G_H */
diff --git a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
index c60e1a12f4dc..0f798b0a78fb 100644
--- a/libgomp/oacc-parallel.c
+++ b/libgomp/oacc-parallel.c
@@ -105,17 +105,33 @@ 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
+/* Legacy support: TODO.  */
+
+static unsigned int
+resolve_legacy_flags (unsigned int flags, bool legacy_only)
+{
+  if ((int) flags == GOMP_DEVICE_ICV)
+    flags = 0;
+  else if ((int) flags == GOMP_DEVICE_HOST_FALLBACK)
+    flags = GOACC_FLAG_HOST_FALLBACK;
+  else if (legacy_only)
+    gomp_fatal ("unexpected device argument");
+  return flags;
+}
+
+
+/* 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 (unsigned int flags, void (*fn) (void *),
 		      size_t mapnum, void **hostaddrs, size_t *sizes,
 		      unsigned short *kinds, ...)
 {
-  bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK;
+  flags = resolve_legacy_flags (flags, /* TODO */ true);
+  bool host_fallback = !!(flags & GOACC_FLAG_HOST_FALLBACK);
   va_list ap;
   struct goacc_thread *thr;
   struct gomp_device_descr *acc_dev;
@@ -256,22 +272,24 @@ 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 (unsigned int flags, void (*fn) (void *),
 		size_t mapnum, void **hostaddrs, size_t *sizes,
 		unsigned short *kinds,
 		int num_gangs, int num_workers, int vector_length,
 		int async, int num_waits, ...)
 {
+  flags = resolve_legacy_flags (flags, true);
   goacc_save_and_set_bind (acc_device_host);
   fn (hostaddrs);
   goacc_restore_bind ();
 }
 
 void
-GOACC_data_start (int device, size_t mapnum,
+GOACC_data_start (unsigned int flags, size_t mapnum,
 		  void **hostaddrs, size_t *sizes, unsigned short *kinds)
 {
-  bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK;
+  flags = resolve_legacy_flags (flags, /* TODO */ true);
+  bool host_fallback = !!(flags & GOACC_FLAG_HOST_FALLBACK);
   struct target_mem_desc *tgt;
 
 #ifdef HAVE_INTTYPES_H
@@ -320,13 +338,14 @@ GOACC_data_end (void)
 }
 
 void
-GOACC_enter_exit_data (int device, size_t mapnum,
+GOACC_enter_exit_data (unsigned int flags, size_t mapnum,
 		       void **hostaddrs, size_t *sizes, unsigned short *kinds,
 		       int async, int num_waits, ...)
 {
+  flags = resolve_legacy_flags (flags, /* TODO */ true);
+  bool host_fallback = !!(flags & GOACC_FLAG_HOST_FALLBACK);
   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;
 
@@ -514,11 +533,12 @@ goacc_wait (int async, int num_waits, va_list *ap)
 }
 
 void
-GOACC_update (int device, size_t mapnum,
+GOACC_update (unsigned int flags, size_t mapnum,
 	      void **hostaddrs, size_t *sizes, unsigned short *kinds,
 	      int async, int num_waits, ...)
 {
-  bool host_fallback = device == GOMP_DEVICE_HOST_FALLBACK;
+  flags = resolve_legacy_flags (flags, /* TODO */ true);
+  bool host_fallback = !!(flags & GOACC_FLAG_HOST_FALLBACK);
   size_t i;
 
   goacc_lazy_initialize ();
@@ -634,9 +654,12 @@ GOACC_get_thread_num (void)
 }
 
 void
-GOACC_declare (int device, size_t mapnum,
+GOACC_declare (unsigned int flags, size_t mapnum,
 	       void **hostaddrs, size_t *sizes, unsigned short *kinds)
 {
+  /* This does not "resolve_legacy_flags", as we're only passing through that
+     argument to "GOACC_enter_exit_data".  */
+
   int i;
 
   for (i = 0; i < mapnum; i++)
@@ -654,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, 1, &hostaddrs[i], &sizes[i],
 				   &kinds[i], GOMP_ASYNC_SYNC, 0);
 	    break;
 
@@ -663,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, 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, 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, 1, &hostaddrs[i], &sizes[i],
 				   &kinds[i], GOMP_ASYNC_SYNC, 0);
 	    break;
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" (was: OpenACC 2.6 "host_data" construct, "if_present" clause)
  2018-12-19 12:46       ` For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" (was: OpenACC 2.6 "host_data" construct, "if_present" clause) Thomas Schwinge
@ 2018-12-19 13:38         ` Jakub Jelinek
  2018-12-19 14:00           ` For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" Thomas Schwinge
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2018-12-19 13:38 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc, Gergö Barany

On Wed, Dec 19, 2018 at 01:46:06PM +0100, Thomas Schwinge wrote:
> > Or, I had the idea that we could avoid that, instead continue using
> > "GOMP_MAP_USE_DEVICE_PTR", and transmit the "if_present" flag through the
> > "int device" argument of "GOACC_data_start" (making sure that old
> > executables continue to function as before).  For OpenACC, that argument
> > is only ever set to "GOMP_DEVICE_ICV" or "GOMP_DEVICE_HOST_FALLBACK" (for
> > "if" clause evaluating to "false"), so has some bits to spare for that.
> > However, I've not been able to convince myself that this solution would
> > be any much prettier than adding a new mapping kind...  ;-)
> 
> Having thought about it some more, the idea doesn't actually seem so bad
> anymore.  :-) Just don't think of it as 'merging stuff into "int
> device"', but rather 'for OpenACC libgomp entry points, redefine the "int
> device" argument to "unsigned int flags"' -- see attached WIP (for GCC
> trunk, testing).
> 
> Jakub, what do you think?

So, what values you were passing in before as the argument?  Just 0 or -1
or something similar and nothing else?  Just wondering if the change isn't
an ABI change.
In OpenMP we are passing a device number (from device clause), or -1, if
no device clause was used and so ICV should be checked and -2 if it is
if (false) and therefore should be always host fallback.

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags"
  2018-12-19 13:38         ` Jakub Jelinek
@ 2018-12-19 14:00           ` Thomas Schwinge
  2018-12-19 14:03             ` Jakub Jelinek
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Schwinge @ 2018-12-19 14:00 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc, Gergö Barany

Hi Jakub!

On Wed, 19 Dec 2018 14:38:38 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Dec 19, 2018 at 01:46:06PM +0100, Thomas Schwinge wrote:
> > > Or, I had the idea that we could avoid that, instead continue using
> > > "GOMP_MAP_USE_DEVICE_PTR", and transmit the "if_present" flag through the
> > > "int device" argument of "GOACC_data_start" (making sure that old
> > > executables continue to function as before).  For OpenACC, that argument
> > > is only ever set to "GOMP_DEVICE_ICV" or "GOMP_DEVICE_HOST_FALLBACK" (for
> > > "if" clause evaluating to "false"), so has some bits to spare for that.
> > > However, I've not been able to convince myself that this solution would
> > > be any much prettier than adding a new mapping kind...  ;-)
> > 
> > Having thought about it some more, the idea doesn't actually seem so bad
> > anymore.  :-) Just don't think of it as 'merging stuff into "int
> > device"', but rather 'for OpenACC libgomp entry points, redefine the "int
> > device" argument to "unsigned int flags"' -- see attached WIP (for GCC
> > trunk, testing).
> > 
> > Jakub, what do you think?
> 
> So, what values you were passing in before as the argument?  Just 0 or -1
> or something similar and nothing else?  Just wondering if the change isn't
> an ABI change.
> In OpenMP we are passing a device number (from device clause), or -1, if
> no device clause was used and so ICV should be checked and -2 if it is
> if (false) and therefore should be always host fallback.

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.)

I'm currently doing a verification run of the libgomp testsuite (had one
detail botched up in the patch that I sent).

And just to make sure: as recently discussed in a different thread, we
don't have to support the case of new executables built that are
dynamically linking against old libgomp versions (where the latter won't
understand the new "flags" value "GOACC_FLAG_HOST_FALLBACK", would ignore
that flag).


Grüße
 Thomas

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags"
  2018-12-19 14:00           ` For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" Thomas Schwinge
@ 2018-12-19 14:03             ` Jakub Jelinek
  2018-12-19 14:18               ` Jakub Jelinek
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2018-12-19 14:03 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc, Gergö Barany

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.)
> 
> I'm currently doing a verification run of the libgomp testsuite (had one
> detail botched up in the patch that I sent).
> 
> And just to make sure: as recently discussed in a different thread, we
> don't have to support the case of new executables built that are
> dynamically linking against old libgomp versions (where the latter won't
> understand the new "flags" value "GOACC_FLAG_HOST_FALLBACK", would ignore
> that flag).

LGTM then in principle.

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags"
  2018-12-19 14:03             ` Jakub Jelinek
@ 2018-12-19 14:18               ` Jakub Jelinek
  2018-12-19 21:56                 ` For libgomp OpenACC entry points, redefine the "device" argument to "flags" Thomas Schwinge
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2018-12-19 14:18 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc, Gergö Barany

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.)
> > 
> > I'm currently doing a verification run of the libgomp testsuite (had one
> > detail botched up in the patch that I sent).
> > 
> > And just to make sure: as recently discussed in a different thread, we
> > don't have to support the case of new executables built that are
> > dynamically linking against old libgomp versions (where the latter won't
> > understand the new "flags" value "GOACC_FLAG_HOST_FALLBACK", would ignore
> > that flag).
> 
> LGTM then in principle.

Or keep it int and use inverted bitmask, thus when bit is 1, it represents
the default state and when bit is 0, it is something different from it.
If you passed before just -1 and -2 and because we are only supporting two's
complement, the host fallback test would be (flags & 1) == 0.
Then you don't need to at runtime transform from legacy to non-legacy.

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* For libgomp OpenACC entry points, redefine the "device" argument to "flags"
  2018-12-19 14:18               ` Jakub Jelinek
@ 2018-12-19 21:56                 ` Thomas Schwinge
  2018-12-28 11:44                   ` Thomas Schwinge
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Schwinge @ 2018-12-19 21:56 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches; +Cc: gcc, Gergö Barany

[-- Attachment #1: Type: text/plain, Size: 6870 bytes --]

Hi Jakub!

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.
> 
> Or keep it int and use inverted bitmask, thus when bit is 1, it represents
> the default state and when bit is 0, it is something different from it.

Ha, I too had that idea after thinking some more about the "-1" and "-2"
values/representations...  :-)

> If you passed before just -1 and -2 and because we are only supporting two's
> complement, the host fallback test would be (flags & 1) == 0.

I structured that a bit more conveniently.  That's especially useful once
additional flags added, where you want to just do "flags |= [flag]", etc.

> Then you don't need to at runtime transform from legacy to non-legacy.

Right.

Is the attached OK for trunk?  If approving this patch, please respond
with "Reviewed-by: NAME <EMAIL>" so that your effort will be recorded in
the commit log, see <https://gcc.gnu.org/wiki/Reviewed-by>.

For your review convenience, here's the "gcc/omp-expand.c" changes with
"--ignore-space-change" (as I slightly restructured OpenACC vs. OpenMP
code paths):

    @@ -7536,49 +7536,62 @@ 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);
    -
    -  c = omp_find_clause (clauses, OMP_CLAUSE_IF);
    -  if (c)
    -    cond = OMP_CLAUSE_IF_EXPR (c);
    -
    +  device = NULL_TREE;
    +  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)
     	{
    -      /* 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
    +	{
    +	  /* By default, the value of DEVICE is GOMP_DEVICE_ICV (let runtime
    +	     library choose).  */
    +	  device = build_int_cst (integer_type_node, GOMP_DEVICE_ICV);
     	  clause_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.  */
    +  cond = NULL_TREE;
    +  c = omp_find_clause (clauses, OMP_CLAUSE_IF);
    +  if (c)
    +    cond = OMP_CLAUSE_IF_EXPR (c);
    +  /* 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 (clause_loc, integer_type_node, device);
     
    -  /* If we found the clause 'if (cond)', build
    -     (cond ? device : GOMP_DEVICE_HOST_FALLBACK).  */
    -  if (cond)
    -    {
    +	  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
    @@ -7601,10 +7614,17 @@ 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);
    +      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));
    @@ -7617,12 +7637,15 @@ 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);
    +
    +      if (device != NULL_TREE)
     	device = force_gimple_operand_gsi (&gsi, device, true, NULL_TREE,
     					   true, GSI_SAME_STMT);
         }
    @@ -7648,6 +7671,16 @@ 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;
    +  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));


Grüße
 Thomas



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-For-libgomp-OpenACC-entry-points-redefine-the-device.patch --]
[-- Type: text/x-diff, Size: 13961 bytes --]

From 2bde6a91bbdbd6aa3e4ca784eeab6f4e0cf77434 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 19 Dec 2018 20:04:18 +0100
Subject: [PATCH] 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".
---
 gcc/omp-expand.c           | 109 ++++++++++++++++++++++++-------------
 gcc/tree-ssa-structalias.c |   4 +-
 include/gomp-constants.h   |  12 ++++
 libgomp/oacc-parallel.c    |  52 +++++++++++-------
 4 files changed, 118 insertions(+), 59 deletions(-)

diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c
index 76c09c5883b7..d3626a6f1c5f 100644
--- a/gcc/omp-expand.c
+++ b/gcc/omp-expand.c
@@ -7536,49 +7536,62 @@ 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);
-
-  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)
+  device = NULL_TREE;
+  tree goacc_flags = NULL_TREE;
+  if (is_gimple_omp_oacc (entry_stmt))
     {
-      /* 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);
+      /* By default, no GOACC_FLAGs are set.  */
+      goacc_flags = integer_zero_node;
     }
   else
-    clause_loc = gimple_location (entry_stmt);
-
-  c = omp_find_clause (clauses, OMP_CLAUSE_NOWAIT);
-  if (c)
-    flags_i |= GOMP_TARGET_FLAG_NOWAIT;
+    {
+      c = omp_find_clause (clauses, OMP_CLAUSE_DEVICE);
+      if (c)
+	{
+	  device = OMP_CLAUSE_DEVICE_ID (c);
+	  clause_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);
+	  clause_loc = gimple_location (entry_stmt);
+	}
 
-  /* Ensure 'device' is of the correct type.  */
-  device = fold_convert_loc (clause_loc, integer_type_node, device);
+      c = omp_find_clause (clauses, OMP_CLAUSE_NOWAIT);
+      if (c)
+	flags_i |= GOMP_TARGET_FLAG_NOWAIT;
+    }
 
-  /* If we found the clause 'if (cond)', build
-     (cond ? device : GOMP_DEVICE_HOST_FALLBACK).  */
+  /* By default, there is no conditional.  */
+  cond = NULL_TREE;
+  c = omp_find_clause (clauses, OMP_CLAUSE_IF);
+  if (c)
+    cond = OMP_CLAUSE_IF_EXPR (c);
+  /* 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 (clause_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
@@ -7601,13 +7614,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);
@@ -7617,14 +7637,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);
@@ -7648,7 +7671,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 a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index fc85e9ded5e8..cfdfb50854f0 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/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 a/include/gomp-constants.h b/include/gomp-constants.h
index d3e64d4e352a..b6b6733bd875 100644
--- a/include/gomp-constants.h
+++ b/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 a/libgomp/oacc-parallel.c b/libgomp/oacc-parallel.c
index e2405d73d5a9..57bd484ba8a1 100644
--- a/libgomp/oacc-parallel.c
+++ b/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;
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: For libgomp OpenACC entry points, redefine the "device" argument to "flags"
  2018-12-19 21:56                 ` For libgomp OpenACC entry points, redefine the "device" argument to "flags" Thomas Schwinge
@ 2018-12-28 11:44                   ` Thomas Schwinge
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Schwinge @ 2018-12-28 11:44 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jakub Jelinek, gcc, Gergö Barany

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-12-28 11:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <0fe4cc25-8ed3-831f-337f-c61fd54eb7f0@mentor.com>
     [not found] ` <6a1ccffb-b139-3bde-6e3d-198464051cbf@mentor.com>
     [not found]   ` <yxfpefaesaov.fsf@hertz.schwinge.homeip.net>
2018-12-18 23:24     ` OpenACC 2.6 "host_data" construct, "if_present" clause Thomas Schwinge
2018-12-19 12:46       ` For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" (was: OpenACC 2.6 "host_data" construct, "if_present" clause) Thomas Schwinge
2018-12-19 13:38         ` Jakub Jelinek
2018-12-19 14:00           ` For OpenACC libgomp entry points, redefine the "int device" argument to "unsigned int flags" Thomas Schwinge
2018-12-19 14:03             ` Jakub Jelinek
2018-12-19 14:18               ` Jakub Jelinek
2018-12-19 21:56                 ` For libgomp OpenACC entry points, redefine the "device" argument to "flags" Thomas Schwinge
2018-12-28 11:44                   ` Thomas Schwinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).