Use "oacc kernels" attribute for OpenACC kernels (was: Mark oacc kernels fns)

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Mon, 25 Jan 2016 16:09:14 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Jan 25, 2016 at 10:06:50AM -0500, Nathan Sidwell wrote:
> > On 01/04/16 10:39, Nathan Sidwell wrote:
> > >There's currently no robust predicate to determine whether an oacc offload
> > >function is for a kernels region (as opposed to a parallel region).
> > >[...]
> > >
> > >This patch marks TREE_PUBLIC on the offload attribute values, to note kernels
> > >regions,  and adds a predicate to check that.  [...]
> > >
> > >Using these predicates improves the dump output of the openacc device lowering
> > >pass too.

I just submitted a patch adding "Test cases to check OpenACC offloaded
function's attributes and classification",
<http://news.gmane.org/find-root.php?message_id=%3C87zip3jw2x.fsf%40hertz.schwinge.homeip.net%3E>,
to actually check the dump output of "oaccdevlow" -- it works.  ;-)

> > https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00092.html
> > ping?
> 
> Ok, thanks.

It's conceptually and code-wise simpler to just use a "oacc kernels"
attribute for that.  (And, that will make another patch I'm working on
less convoluted.)

I'm open to suggestions if there is a better place to set the "oacc
kernels" attribute -- I put it into expand_omp_target, where another
special thing for GF_OMP_TARGET_KIND_OACC_KERNELS is already being done,
and before "rewriting" GF_OMP_TARGET_KIND_OACC_KERNELS (and
GF_OMP_TARGET_KIND_OACC_PARALLEL) into BUILT_IN_GOACC_PARALLEL.  My
reasoning for not setting the attribute earlier (like, in the front
ends), is that at that point in/before expand_omp_target, we still have
the distrinction between OACC_PARALLEL/OACC_KERNELS (tree codes), and
later GF_OMP_TARGET_KIND_OACC_PARALLEL/GF_OMP_TARGET_KIND_OACC_KERNELS
(GIMPLE_OMP_TARGET subcodes).  Another question/possibly cleanup of
course might be to actually do set the "oacc kernels" attribute in the
front end and merge OACC_KERNELS into OACC_PARALLEL, and
GF_OMP_TARGET_KIND_OACC_KERNELS into GF_OMP_TARGET_KIND_OACC_PARALLEL?

But anyway, as a first step: OK for trunk?

commit 2e6dc8dfd679d8dae814e325afa2547b502827ef
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Tue Jul 26 17:44:31 2016 +0200

    Use "oacc kernels" attribute for OpenACC kernels
    
    	gcc/
    	* omp-low.c (expand_omp_target) <GF_OMP_TARGET_KIND_OACC_KERNELS>:
    	Set "oacc kernels" attribute.
    	(set_oacc_fn_attrib): Remove is_kernel formal parameter.  Adjust
    	all users.
    	(oacc_fn_attrib_kernels_p): Remove function.
    	(execute_oacc_device_lower): Look for "oacc kernels" attribute
    	instead of calling oacc_fn_attrib_kernels_p.
    	* tree-ssa-loop.c (gate_oacc_kernels): Likewise.
    	* tree-parloops.c (create_parallel_loop): If oacc_kernels_p,
    	assert "oacc kernels" attribute is set.
---
 gcc/omp-low.c                                      | 53 ++++++++--------------
 gcc/omp-low.h                                      |  3 +-
 gcc/tree-parloops.c                                |  5 +-
 gcc/tree-ssa-loop.c                                |  5 +-
 10 files changed, 34 insertions(+), 48 deletions(-)

diff --git gcc/omp-low.c gcc/omp-low.c
index c75452c..a35556d 100644
--- gcc/omp-low.c
+++ gcc/omp-low.c
@@ -12552,11 +12552,10 @@ replace_oacc_fn_attrib (tree fn, tree dims)
 
 /* Scan CLAUSES for launch dimensions and attach them to the oacc
    function attribute.  Push any that are non-constant onto the ARGS
-   list, along with an appropriate GOMP_LAUNCH_DIM tag.  IS_KERNEL is
-   true, if these are for a kernels region offload function.  */
+   list, along with an appropriate GOMP_LAUNCH_DIM tag.  */
 
 void
-set_oacc_fn_attrib (tree fn, tree clauses, bool is_kernel, vec<tree> *args)
+set_oacc_fn_attrib (tree fn, tree clauses, vec<tree> *args)
 {
   /* Must match GOMP_DIM ordering.  */
   static const omp_clause_code ids[]
@@ -12581,9 +12580,6 @@ set_oacc_fn_attrib (tree fn, tree clauses, bool is_kernel, vec<tree> *args)
 	  non_const |= GOMP_DIM_MASK (ix);
 	}
       attr = tree_cons (NULL_TREE, dim, attr);
-      /* Note kernelness with TREE_PUBLIC.  */
-      if (is_kernel)
-	TREE_PUBLIC (attr) = 1;
     }
 
   replace_oacc_fn_attrib (fn, attr);
@@ -12652,16 +12648,6 @@ get_oacc_fn_attrib (tree fn)
   return lookup_attribute (OACC_FN_ATTRIB, DECL_ATTRIBUTES (fn));
 }
 
-/* Return true if this oacc fn attrib is for a kernels offload
-   region.  We use the TREE_PUBLIC flag of each dimension -- only
-   need to check the first one.  */
-
-bool
-oacc_fn_attrib_kernels_p (tree attr)
-{
-  return TREE_PUBLIC (TREE_VALUE (attr));
-}
-
 /* Return level at which oacc routine may spawn a partitioned loop, or
    -1 if it is not a routine (i.e. is an offload fn).  */
 
@@ -13044,7 +13030,12 @@ expand_omp_target (struct omp_region *region)
   exit_bb = region->exit;
 
   if (gimple_omp_target_kind (entry_stmt) == GF_OMP_TARGET_KIND_OACC_KERNELS)
-    mark_loops_in_oacc_kernels_region (region->entry, region->exit);
+    {
+      DECL_ATTRIBUTES (child_fn)
+	= tree_cons (get_identifier ("oacc kernels"),
+		     NULL_TREE, DECL_ATTRIBUTES (child_fn));
+      mark_loops_in_oacc_kernels_region (region->entry, region->exit);
+    }
 
   if (offloaded)
     {
@@ -13224,7 +13215,6 @@ expand_omp_target (struct omp_region *region)
   enum built_in_function start_ix;
   location_t clause_loc;
   unsigned int flags_i = 0;
-  bool oacc_kernels_p = false;
 
   switch (gimple_omp_target_kind (entry_stmt))
     {
@@ -13245,8 +13235,6 @@ expand_omp_target (struct omp_region *region)
       flags_i |= GOMP_TARGET_FLAG_EXIT_DATA;
       break;
     case GF_OMP_TARGET_KIND_OACC_KERNELS:
-      oacc_kernels_p = true;
-      /* FALLTHROUGH */
     case GF_OMP_TARGET_KIND_OACC_PARALLEL:
       start_ix = BUILT_IN_GOACC_PARALLEL;
       break;
@@ -13409,7 +13397,7 @@ expand_omp_target (struct omp_region *region)
       break;
     case BUILT_IN_GOACC_PARALLEL:
       {
-	set_oacc_fn_attrib (child_fn, clauses, oacc_kernels_p, &args);
+	set_oacc_fn_attrib (child_fn, clauses, &args);
 	tagging = true;
       }
       /* FALLTHRU */
@@ -18851,7 +18839,6 @@ oacc_validate_dims (tree fn, tree attrs, int *dims, int level, unsigned used)
   tree purpose[GOMP_DIM_MAX];
   unsigned ix;
   tree pos = TREE_VALUE (attrs);
-  bool is_kernel = oacc_fn_attrib_kernels_p (attrs);
 
   /* Make sure the attribute creator attached the dimension
      information.  */
@@ -18898,13 +18885,9 @@ oacc_validate_dims (tree fn, tree attrs, int *dims, int level, unsigned used)
       /* Replace the attribute with new values.  */
       pos = NULL_TREE;
       for (ix = GOMP_DIM_MAX; ix--;)
-	{
-	  pos = tree_cons (purpose[ix],
-			   build_int_cst (integer_type_node, dims[ix]),
-			   pos);
-	  if (is_kernel)
-	    TREE_PUBLIC (pos) = 1;
-	}
+	pos = tree_cons (purpose[ix],
+			 build_int_cst (integer_type_node, dims[ix]),
+			 pos);
       replace_oacc_fn_attrib (fn, pos);
     }
 }
@@ -19644,10 +19627,14 @@ execute_oacc_device_lower ()
   int fn_level = oacc_fn_attrib_level (attrs);
 
   if (dump_file)
-    fprintf (dump_file, oacc_fn_attrib_kernels_p (attrs)
-	     ? "Function is kernels offload\n"
-	     : fn_level < 0 ? "Function is parallel offload\n"
-	     : "Function is routine level %d\n", fn_level);
+    {
+      if (lookup_attribute ("oacc kernels", DECL_ATTRIBUTES (current_function_decl)))
+	fprintf (dump_file, "Function is OpenACC kernels offload\n");
+      else if (fn_level < 0)
+	fprintf (dump_file, "Function is OpenACC parallel offload\n");
+      else
+	fprintf (dump_file, "Function is OpenACC routine level %d\n", fn_level);
+    }
 
   unsigned outer_mask = fn_level >= 0 ? GOMP_DIM_MASK (fn_level) - 1 : 0;
   unsigned used_mask = oacc_loop_partition (loops, outer_mask);
diff --git gcc/omp-low.h gcc/omp-low.h
index b1f7885..64caef8 100644
--- gcc/omp-low.h
+++ gcc/omp-low.h
@@ -33,8 +33,7 @@ extern tree omp_member_access_dummy_var (tree);
 extern void replace_oacc_fn_attrib (tree, tree);
 extern tree build_oacc_routine_dims (tree);
 extern tree get_oacc_fn_attrib (tree);
-extern void set_oacc_fn_attrib (tree, tree, bool, vec<tree> *);
-extern bool oacc_fn_attrib_kernels_p (tree);
+extern void set_oacc_fn_attrib (tree, tree, vec<tree> *);
 extern int get_oacc_ifn_dim_arg (const gimple *);
 extern int get_oacc_fn_dim_size (tree, int);
 
diff --git gcc/tree-parloops.c gcc/tree-parloops.c
index a160152..5706b0c 100644
--- gcc/tree-parloops.c
+++ gcc/tree-parloops.c
@@ -2042,10 +2042,13 @@ create_parallel_loop (struct loop *loop, tree loop_fn, tree data,
   /* Prepare the GIMPLE_OMP_PARALLEL statement.  */
   if (oacc_kernels_p)
     {
+      gcc_checking_assert (lookup_attribute ("oacc kernels",
+					     DECL_ATTRIBUTES (cfun->decl)));
+
       tree clause = build_omp_clause (loc, OMP_CLAUSE_NUM_GANGS);
       OMP_CLAUSE_NUM_GANGS_EXPR (clause)
 	= build_int_cst (integer_type_node, n_threads);
-      set_oacc_fn_attrib (cfun->decl, clause, true, NULL);
+      set_oacc_fn_attrib (cfun->decl, clause, NULL);
     }
   else
     {
diff --git gcc/tree-ssa-loop.c gcc/tree-ssa-loop.c
index 06354e3..da9a806 100644
--- gcc/tree-ssa-loop.c
+++ gcc/tree-ssa-loop.c
@@ -151,10 +151,7 @@ gate_oacc_kernels (function *fn)
   if (!flag_openacc)
     return false;
 
-  tree oacc_function_attr = get_oacc_fn_attrib (fn->decl);
-  if (oacc_function_attr == NULL_TREE)
-    return false;
-  if (!oacc_fn_attrib_kernels_p (oacc_function_attr))
+  if (!lookup_attribute ("oacc kernels", DECL_ATTRIBUTES (fn->decl)))
     return false;
 
   struct loop *loop;


Grüße
 Thomas

Attachment: signature.asc
Description: PGP signature