[PATCH] Fix use of declare'd vars by routine procedures.

[Jakub Jelinek]

On Thu, Jan 28, 2016 at 12:26:38PM -0600, James Norris wrote:
> I think the attached change is what you had in mind with
> regard to doing the check at gimplification time.

Nope, this is still a wrong location for that.
If you look at the next line after the block you've added, you'll see
if (gimplify_omp_ctxp && omp_notice_variable (gimplify_omp_ctxp, decl, true))
And that function fairly early calls is_global_var (decl).  So you know
already gimplify_omp_ctxp && is_global_var (decl), just put the rest into
that block.
The TREE_CODE (decl) == VAR_DECL check is VAR_P (decl).
What do you want to achieve with

> +      && ((TREE_STATIC (decl) && !DECL_EXTERNAL (decl))  
> +       || (!TREE_STATIC (decl) && DECL_EXTERNAL (decl))))

?  is_global_var already guarantees you that it is either TREE_STATIC
or DECL_EXTERNAL, why is that not good enough?

> diff --git a/trunk/gcc/gimplify.c b/trunk/gcc/gimplify.c
> --- a/trunk/gcc/gimplify.c	(revision 232802)
> +++ b/trunk/gcc/gimplify.c	(working copy)
> @@ -1841,6 +1841,33 @@
>        return GS_ERROR;
>      }
>  
> +  /* Validate variable for use within routine function.  */
> +  if (gimplify_omp_ctxp && gimplify_omp_ctxp->region_type == ORT_TARGET
> +      && get_oacc_fn_attrib (current_function_decl)

If you only care about the implicit target region of acc routine, I think
you also want to check that gimplify_omp_ctxp->outer_context == NULL.

> +      && TREE_CODE (decl) == VAR_DECL
> +      && is_global_var (decl)
> +      && ((TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
> +	  || (!TREE_STATIC (decl) && DECL_EXTERNAL (decl))))
> +    {
> +      location_t loc = DECL_SOURCE_LOCATION (decl);
> +
> +      if (lookup_attribute ("omp declare target link", DECL_ATTRIBUTES (decl)))
> +	{
> +	  error_at (loc,
> +		    "%qE with %<link%> clause used in %<routine%> function",
> +		    DECL_NAME (decl));
> +	  return GS_ERROR;
> +	}
> +      else if (!lookup_attribute ("omp declare target", DECL_ATTRIBUTES (decl)))
> +	{
> +	  error_at (loc,
> +		    "storage class of %qE cannot be ", DECL_NAME (decl));
> +	  error_at (gimplify_omp_ctxp->location,
> +		    "used in enclosing %<routine%> function");

And I'm really confused by this error message.  If you are complaining that
the variable is not listed in acc declare clauses, why don't you say that?
What does the error have to do with its storage class?
Also, splitting one error into two is weird, usually there would be one
error message and perhaps inform after it.

	Jakub