public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Thomas Schwinge <thomas@codesourcery.com>
Cc: gcc-patches@gcc.gnu.org, fortran@gcc.gnu.org,
	i.usmanov@samsung.com,        Ilmir Usmanov <me@ilmir.us>,
	cesar@codesourcery.com
Subject: Re: [PR fortran/63858] Fix mix of OpenACC and OpenMP sentinels in continuations
Date: Tue, 20 Oct 2015 09:41:00 -0000	[thread overview]
Message-ID: <20151020093733.GV478@tucnak.redhat.com> (raw)
In-Reply-To: <878u7cny9v.fsf@kepler.schwinge.homeip.net>

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

  parent reply	other threads:[~2015-10-20  9:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <3008431435623821@web14j.yandex.ru>
2015-06-30  1:00 ` [gomp4, fortran] Patch to fix continuation checks of OpenACC and OpenMP directives Ilmir Usmanov
     [not found]   ` <650751436268444@web22m.yandex.ru>
2015-07-14 21:24     ` Ilmir Usmanov
2015-07-14 21:26       ` Cesar Philippidis
2015-07-27 14:17   ` Thomas Schwinge
2015-10-09 10:15     ` [PR fortran/63858] Fix mix of OpenACC and OpenMP sentinels in continuations Thomas Schwinge
2015-10-19 17:12       ` Thomas Schwinge
2015-10-20  9:41       ` Jakub Jelinek [this message]
2015-11-25 14:35         ` Cesar Philippidis
2015-11-25 19:14           ` Bernhard Reutner-Fischer
2015-06-07 21:06 [gomp4, fortran] Patch to fix continuation checks of OpenACC and OpenMP directives Ilmir Usmanov
2015-06-07 21:40 ` Ilmir Usmanov
2015-06-08 15:01   ` Cesar Philippidis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151020093733.GV478@tucnak.redhat.com \
    --to=jakub@redhat.com \
    --cc=cesar@codesourcery.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=i.usmanov@samsung.com \
    --cc=me@ilmir.us \
    --cc=thomas@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).