Re: [1/2] OpenACC routine support

[Cesar Philippidis <cesar_philippidis at mentor dot com>]

On 12/01/2015 06:49 AM, Cesar Philippidis wrote:
> On 12/01/2015 06:40 AM, Thomas Schwinge wrote:
> 
>> I noticed while working on other test cases:
>>
>> On Wed, 18 Nov 2015 11:02:01 -0800, Cesar Philippidis <cesar@codesourcery.com> wrote:
>>> --- a/gcc/cp/parser.c
>>> +++ b/gcc/cp/parser.c
>>
>>> @@ -1318,13 +1318,21 @@ cp_finalize_omp_declare_simd (cp_parser *parser, tree fndecl)
>>>      }
>>>  }
>>>  
>>> -/* Diagnose if #pragma omp routine isn't followed immediately
>>> -   by function declaration or definition.   */
>>> +/* Diagnose if #pragma acc routine isn't followed immediately by function
>>> +   declaration or definition.  */
>>>  
>>>  static inline void
>>>  cp_ensure_no_oacc_routine (cp_parser *parser)
>>>  {
>>> -  cp_finalize_oacc_routine (parser, NULL_TREE, false, true);
>>> +  if (parser->oacc_routine && !parser->oacc_routine->error_seen)
>>> +    {
>>> +      tree clauses = parser->oacc_routine->clauses;
>>> +      location_t loc = OMP_CLAUSE_LOCATION (TREE_PURPOSE(clauses));
>>> +
>>> +      error_at (loc, "%<#pragma oacc routine%> not followed by function "
>>> +		"declaration or definition");
>>> +      parser->oacc_routine = NULL;
>>> +    }
>>>  }
>>
>> "#pragma acc routine", not "oacc".  Also in a few other places.
> 
> Good eyes. Thanks for catching that.
> 
>> Next, in the function quoted above, you use "not followed by function
>> declaration or definition", but you use "not followed by a single
>> function declaration or definition" in a lot of (but not all) other
>> places -- is that intentional?
> 
> I probably wasn't being consistent. Which error message do you prefer?
> I'll take a look at what the c front end does.
> 
>> For example:
>>
>>>  cp_parser_oacc_routine (cp_parser *parser, cp_token *pragma_tok,
>>>  			enum pragma_context context)
>>>  {
>>> [...]
>>> +	  error_at (OMP_CLAUSE_LOCATION (parser->oacc_routine->clauses),
>>> +		    "%<#pragma oacc routine%> not followed by a single "
>>> +		    "function declaration or definition");
>>
>> "a single".
>>
>>> [...]
>>> +	  if (parser->oacc_routine
>>> +	      && !parser->oacc_routine->error_seen
>>> +	      && !parser->oacc_routine->fndecl_seen)
>>> +	    error_at (loc, "%<#pragma acc routine%> not followed by "
>>> +		      "function declaration or definition");
>>
>> Not "a single".
>>
>>> +
>>> +	  data.tokens.release ();
>>> +	  parser->oacc_routine = NULL;
>>> +	}
>>> +    }
>>> +}
>>> +
>>> +/* Finalize #pragma acc routine clauses after direct declarator has
>>> +   been parsed, and put that into "oacc routine" attribute.  */
>>
>> There is no "oacc routine" attribute (anymore)?
> 
> You're right, it was renamed to 'oacc function'.
> 
>>> +static tree
>>> +cp_parser_late_parsing_oacc_routine (cp_parser *parser, tree attrs)
>>> +{
>>> [...]
>>> +  if ((!data->error_seen && data->fndecl_seen)
>>> +      || data->tokens.length () != 1)
>>> +    {
>>> +      error_at (loc, "%<#pragma oacc routine%> not followed by a single "
>>> +		"function declaration or definition");
>>
>> "a single".
>>
>> (I have not verified all of the parser(s) source code.)
> 
> Thanks. I'll go through and update the comments and error messages.

Here's the updated patch. The test cases were written in a way such that
none of them needed to be updated with these changes.

I'm tempted to commit this as obvious, but I want to make sure you're ok
with these new messages. The major change is to report these errors as
"pragma acc routine not followed by a function declaration or
definition". I think that's more descriptive then "not followed by a
single function". That said, it looks like the c front end uses the
latter error message.

Is this OK or do you prefer the "not followed by a single function" message?

Cesar