This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [OpenACC] declare directive
- From: James Norris <jnorris at codesourcery dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Thomas Schwinge <thomas at codesourcery dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 9 Nov 2015 10:31:13 -0600
- Subject: Re: [OpenACC] declare directive
- Authentication-results: sourceware.org; auth=none
- References: <562FDBF8 dot 1040105 at mentor dot com> <5638E164 dot 5010207 at codesourcery dot com> <87611h1zi7 dot fsf at kepler dot schwinge dot homeip dot net> <563CD07A dot 3000404 at codesourcery dot com> <20151106190352 dot GG5675 at tucnak dot redhat dot com> <563F6BA5 dot 9020606 at codesourcery dot com> <5640C35C dot 9030907 at codesourcery dot com> <20151109162117 dot GU5675 at tucnak dot redhat dot com>
Jakub,
On 11/09/2015 10:21 AM, Jakub Jelinek wrote:
On Mon, Nov 09, 2015 at 10:01:32AM -0600, James Norris wrote:
+ if (lookup_attribute ("omp declare target", DECL_ATTRIBUTES (decl)))
Here you only look up "omp declare target", not "omp declare target link".
So, what happens if you mix that (once in some copy clause, once in link),
or mention twice in link, etc.? Needs testsuite coverage and clear rules.
Will fix.
+ DECL_ATTRIBUTES (decl) =
+ tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (decl));
Incorrect formatting, = goes already on the following line, no whitespace
at end of line, and next line is indented below CL from DECL.
Will fix.
+ t = build_omp_clause (OMP_CLAUSE_LOCATION (c) , OMP_CLAUSE_MAP);
Wrong formatting, no space before ,.
Will fix.
+ if (ret_clauses)
+ {
+ tree fndecl = current_function_decl;
+ tree attrs = lookup_attribute ("oacc declare returns",
+ DECL_ATTRIBUTES (fndecl));
Why do you use an attribute for this? I think adding the automatic
vars to hash_map during gimplification of the OACC_DECLARE is best.
See below (This doesn't scale...)
+ tree id = get_identifier ("oacc declare returns");
+ DECL_ATTRIBUTES (fndecl) =
+ tree_cons (id, ret_clauses, DECL_ATTRIBUTES (fndecl));
Formatting error.
Will fix.
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1065,6 +1065,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
gimple_seq body, cleanup;
gcall *stack_save;
location_t start_locus = 0, end_locus = 0;
+ tree ret_clauses = NULL;
tree temp = voidify_wrapper_expr (bind_expr, NULL);
@@ -1166,9 +1167,56 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
clobber_stmt = gimple_build_assign (t, clobber);
gimple_set_location (clobber_stmt, end_locus);
gimplify_seq_add_stmt (&cleanup, clobber_stmt);
+
+ if (flag_openacc)
+ {
+ tree attrs = lookup_attribute ("oacc declare returns",
+ DECL_ATTRIBUTES (current_function_decl));
+ tree clauses, c, c_next = NULL, c_prev = NULL;
+
+ if (!attrs)
+ break;
+
+ clauses = TREE_VALUE (attrs);
+
+ for (c = clauses; c; c_prev = c, c = c_next)
+ {
+ c_next = OMP_CLAUSE_CHAIN (c);
+
+ if (t == OMP_CLAUSE_DECL (c))
+ {
+ if (ret_clauses)
+ OMP_CLAUSE_CHAIN (c) = ret_clauses;
+
+ ret_clauses = c;
+
+ if (c_prev == NULL)
+ clauses = c_next;
+ else
+ OMP_CLAUSE_CHAIN (c_prev) = c_next;
+ }
+ }
This doesn't really scale. Consider 10000 clauses on various
oacc declare constructs in a single function, and 1000000 automatic
variables in such a function.
So, what I'm suggesting is during gimplification of OACC_DECLARE,
if you find a clause on an automatic variable in the current function
that you want to unmap afterwards, have a
static hash_map<tree, tree> *oacc_declare_returns;
and you just add into the hash map the VAR_DECL -> the clause you want,
then in this spot you check
if (oacc_declare_returns)
{
clause = lookup in hash_map (t);
if (clause)
{
...
}
}
Now I see what you were getting at in using the hash_map. I didn't
consider creating a static hash_map and populating it as you suggest.
Thank you!
+
+ if (clauses == NULL)
+ {
+ DECL_ATTRIBUTES (current_function_decl) =
+ remove_attribute ("oacc declare returns",
+ DECL_ATTRIBUTES (current_function_decl));
Wrong formatting.
Will fix.
Jakub
Thanks for taking the time to review.
Jim