| Message ID | 87twfb9yyk.fsf@kepler.schwinge.homeip.net |
|---|---|
| State | New |
| Headers | show |
Hi! Ping. On Wed, 27 Jul 2016 12:06:59 +0200, I wrote: > 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
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;