[committed] Fix OpenMP parsing of the specification part in functions (PR fortran/71704)

[Cesar Philippidis]

On 07/08/2016 09:31 AM, Jakub Jelinek wrote:
> On Fri, Jul 08, 2016 at 09:19:01AM -0700, Cesar Philippidis wrote:
>> 2016-07-08  Cesar Philippidis  <cesar@codesourcery.com>
>>
>> 	gcc/fortran/
>> 	* parse.c (matcha): Define.
>> 	(decode_oacc_directive): Add spec_only local var and set it.  Use
>> 	matcha to parse acc data, enter data, exit data, host_data, parallel,
>> 	kernels, update and wait directives.  Return ST_GET_FCN_CHARACTERISTICS
>> 	if a non-declarative directive could be matched.
>>
>> 	gcc/testsuite/
>> 	* gfortran.dg/goacc/pr71704-acc.f90: New test.
> 
> I'd drop the -acc suffix, the directory is enough to differentiate the gomp
> vs. goacc test.

Done.

>> --- a/gcc/fortran/parse.c
>> +++ b/gcc/fortran/parse.c
>> @@ -589,11 +589,25 @@ decode_statement (void)
>>    return ST_NONE;
>>  }
>>  
>> +/* Like match, but don't match anything if not -fopenacc
>> +   and if spec_only, goto do_spec_only without actually matching.  */
> 
> The comment doesn't match what the macro does.  The whole
> decode_oacc_directive function is only called if -fopenacc, so
> it is really "Like a match and if spec_only, ..."

The intent behind that comment was to note it shouldn't be used with the
OpenMP clauses. But I agree that the -fopenacc stuff isn't necessary.
This patch updates the comment.

>> +#define matcha(keyword, subr, st)				\
>> +    do {							\
>> +      if (spec_only && gfc_match (keyword) == MATCH_YES)	\
>> +	goto do_spec_only;					\
>> +      else if (match_word (keyword, subr, &old_locus)		\
>> +	       == MATCH_YES)					\
>> +	return st;						\
>> +      else							\
>> +	undo_new_statement ();				  	\
>> +    } while (0);
>> +
>>  static gfc_statement
>>  decode_oacc_directive (void)
>>  {
>>    locus old_locus;
>>    char c;
>> +  bool spec_only = false;
>>  
>>    gfc_enforce_clean_symbol_state ();
>>  
>> @@ -608,6 +622,10 @@ decode_oacc_directive (void)
>>        return ST_NONE;
>>      }
>>  
>> +  if (gfc_current_state () == COMP_FUNCTION
>> +      && gfc_current_block ()->result->ts.kind == -1)
>> +    spec_only = true;
>> +
>>    gfc_unset_implicit_pure (NULL);
>>  
>>    old_locus = gfc_current_locus;
>> @@ -627,7 +645,7 @@ decode_oacc_directive (void)
>>        match ("cache", gfc_match_oacc_cache, ST_OACC_CACHE);
> 
> Why isn't ST_OACC_ATOMIC matcha?
> At least from the case_executable/case_exec_markers vs.
> case_decl defines, all directives but "routine" and "declare" should
> be matcha IMHO.

Because the atomic directive must operate on a sequence of instructions,
otherwise it should generate a syntax error.

> Also, can you figure out in the OpenACC standard and/or discuss on lang
> committee whether acc declare and/or acc routine can appear anywhere in the
> specification part, or need to be ordered certain way?
> If like in OpenMP they can appear anywhere, then
> case ST_OACC_ROUTINE: case ST_OACC_DECLARE
> should move from case_decl to case_omp_decl macro.

OK, I'll check with them. Can that be a follow up patch, or would you
like to see it resolved in one patch?

Is this patch OK for trunk and gcc6?

Cesar

-------------- next part --------------
A non-text attachment was scrubbed...
Name: fortran-oacc-directives-a.diff
Type: text/x-patch
Size: 4918 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20160708/fa7bfbf2/attachment.bin>