On 10/20/2015 02:37 AM, Jakub Jelinek wrote:
> On Fri, Oct 09, 2015 at 12:15:24PM +0200, Thomas Schwinge wrote:
>> diff --git gcc/fortran/scanner.c gcc/fortran/scanner.c
>> index bfb7d45..1e1ea84 100644
>> --- gcc/fortran/scanner.c
>> +++ gcc/fortran/scanner.c
>> @@ -935,6 +935,63 @@ skip_free_comments (void)
>>    return false;
>>  }
>>  
>> +/* Return true if MP was matched in fixed form.  */
>> +static bool
>> +skip_omp_attribute_fixed (locus *start)
> 
> Technically, this isn't attribute, but sentinel.
> So, skip_fixed_omp_sentinel?  I know the free functions
> are called attribute, perhaps we should rename them too,
> patch to do so preapproved.

I've renamed those functions in this patch. The free variants are named
skip_free_*_sentinel.

>> +{
>> +  gfc_char_t c;
>> +  if (((c = next_char ()) == 'm' || c == 'M')
>> +      && ((c = next_char ()) == 'p' || c == 'P'))
>> +    {
>> +      c = next_char ();
>> +      if (c != '\n'
>> +	  && (continue_flag
> 
> The original code checked here
> 	(openmp_flag && continue_flag)
> instead.  Is that change intentional?

I think so. Without it, continuations won't work with -fopenmp-simd.
Note how that function call is guarded by

  if ((flag_openmp || flag_openmp_simd) && !flag_openacc)

> Looking around, we probably don't have a testcase coverage for say
> fixed form:
> 
> C*OMP+PARALLEL DO
> do ...

That's going to be tricky. In fixed mode, the only way that we can tell
if there is an omp/acc continuation is by inspecting the character at
position 6. So, there could be a comment like

C*ACCELERATOR

which would get picked up as an acc continuation.

> (i.e. where it starts with an OpenMP (or OpenACC) continuation, without
> non-continued line first), or for free form where:
> 
> something &
> !$omp & parallel
> 
> (ditto for OpenACC).

What type of error should this be reporting? Right now it does report an
error because this gets expanded to

something parallel

That's clearly not correct. But at the same time, it would still be an
error if the user placed !$omp/acc between a continuation.

>> +	  while (gfc_is_whitespace (c));
>> +	  if (c != '\n' && c != '!')
>> +	    {
>> +	      /* Canonicalize to *$omp.  */
> 
> The comment has a pasto, by storing * you canonicalize to *$acc.

Fixed.

>> -	  if (flag_openacc)
>> +	  if (flag_openacc || (flag_openmp || flag_openmp_simd))
> 
> I'd just write
> 	if (flag_openacc || flag_openmp || flag_openmp_simd)
> the ()s around are just misleading.
> 
> Anyway, if the removal of "openmp_flag &&" is intentional, then
> the patch is ok with the above mentioned changes.  We can deal with the
> cases I've mentioned up above in a follow-up.

I'll apply this patch shortly.

Cesar