[PR fortran/63858] Fix mix of OpenACC and OpenMP sentinels in continuations

Jakub Jelinek jakub@redhat.com
Tue Oct 20 09:41:00 GMT 2015


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.

> +{
> +  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?
Looking around, we probably don't have a testcase coverage for say
fixed form:

C*OMP+PARALLEL DO
do ...

(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).

> +	  while (gfc_is_whitespace (c));
> +	  if (c != '\n' && c != '!')
> +	    {
> +	      /* Canonicalize to *$omp.  */

The comment has a pasto, by storing * you canonicalize to *$acc.

> -	  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.

	Jakub



More information about the Gcc-patches mailing list