public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Tobias Burnus <tobias@codesourcery.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>, fortran <fortran@gcc.gnu.org>
Subject: Re: [Patch] Fortran/openmp: Fix '!$omp end'
Date: Thu, 11 Nov 2021 19:01:14 +0100	[thread overview]
Message-ID: <20211111180114.GU2710@tucnak> (raw)
In-Reply-To: <e2a8a2ee-0f3a-3f56-7326-14da23189fec@codesourcery.com>

On Thu, Nov 11, 2021 at 06:11:23PM +0100, Tobias Burnus wrote:
> --- a/gcc/fortran/parse.c
> +++ b/gcc/fortran/parse.c
> @@ -915,15 +915,16 @@ decode_omp_directive (void)
>        matcho ("error", gfc_match_omp_error, ST_OMP_ERROR);
>        matcho ("end atomic", gfc_match_omp_eos_error, ST_OMP_END_ATOMIC);
>        matcho ("end critical", gfc_match_omp_end_critical, ST_OMP_END_CRITICAL);
> -      matchs ("end distribute parallel do simd", gfc_match_omp_eos_error,
> +      matchs ("end distribute parallel do simd", gfc_match_omp_end_nowait,
>  	      ST_OMP_END_DISTRIBUTE_PARALLEL_DO_SIMD);
> -      matcho ("end distribute parallel do", gfc_match_omp_eos_error,
> +      matcho ("end distribute parallel do", gfc_match_omp_end_nowait,

I think the above two changes are incorrect.
At least looking at 5.1 which is clearer than 5.2, 5.1 [221:17-23] says
for C/C++ that while nowait is allowed on worksharing-loop, it is not
allowed on combined parallel worksharing-loop, and Fortran has that
restriction through the syntax (no [nowait] on !$omp end parallel do).

> @@ -936,9 +937,12 @@ decode_omp_directive (void)
>  	      ST_OMP_END_MASTER_TASKLOOP);
>        matcho ("end master", gfc_match_omp_eos_error, ST_OMP_END_MASTER);
>        matchs ("end ordered", gfc_match_omp_eos_error, ST_OMP_END_ORDERED);
> -      matchs ("end parallel do simd", gfc_match_omp_eos_error,
> +      matchs ("end parallel do simd", gfc_match_omp_end_nowait,
>  	      ST_OMP_END_PARALLEL_DO_SIMD);
> -      matcho ("end parallel do", gfc_match_omp_eos_error, ST_OMP_END_PARALLEL_DO);
> +      matcho ("end parallel do", gfc_match_omp_end_nowait,
> +	      ST_OMP_END_PARALLEL_DO);

Likewise for the above two.

> @@ -951,46 +955,53 @@ decode_omp_directive (void)
>  	      ST_OMP_END_PARALLEL_MASTER_TASKLOOP);
>        matcho ("end parallel master", gfc_match_omp_eos_error,
>  	      ST_OMP_END_PARALLEL_MASTER);
> -      matcho ("end parallel sections", gfc_match_omp_eos_error,
> +      matcho ("end parallel sections", gfc_match_omp_end_nowait,
>  	      ST_OMP_END_PARALLEL_SECTIONS);
> -      matcho ("end parallel workshare", gfc_match_omp_eos_error,
> +      matcho ("end parallel workshare", gfc_match_omp_end_nowait,
>  	      ST_OMP_END_PARALLEL_WORKSHARE);

Ditto for the above two.

>        matcho ("end parallel", gfc_match_omp_eos_error, ST_OMP_END_PARALLEL);
>        matcho ("end scope", gfc_match_omp_end_nowait, ST_OMP_END_SCOPE);
>        matcho ("end sections", gfc_match_omp_end_nowait, ST_OMP_END_SECTIONS);
>        matcho ("end single", gfc_match_omp_end_single, ST_OMP_END_SINGLE);
>        matcho ("end target data", gfc_match_omp_eos_error, ST_OMP_END_TARGET_DATA);
> -      matchs ("end target parallel do simd", gfc_match_omp_eos_error,
> +      matchs ("end target parallel do simd", gfc_match_omp_end_nowait,

The above seems like a bug in 5.1 standard, haven't checked 5.2.
!$omp end target parallel do simd nowait
should be IMO valid, but [241:16] mistakenly doesn't list it.

>  	      ST_OMP_END_TARGET_PARALLEL_DO_SIMD);
> -      matcho ("end target parallel do", gfc_match_omp_eos_error,
> +      matcho ("end target parallel do", gfc_match_omp_end_nowait,

Similarly.

>  	      ST_OMP_END_TARGET_PARALLEL_DO);
> -      matcho ("end target parallel", gfc_match_omp_eos_error,
> +      matcho ("end target parallel loop", gfc_match_omp_end_nowait,
> +	      ST_OMP_END_TARGET_PARALLEL_LOOP);

Similarly.

> +      matcho ("end target parallel", gfc_match_omp_end_nowait,
>  	      ST_OMP_END_TARGET_PARALLEL);

Similarly.

> -      matchs ("end target simd", gfc_match_omp_eos_error, ST_OMP_END_TARGET_SIMD);
> +      matchs ("end target simd", gfc_match_omp_end_nowait, ST_OMP_END_TARGET_SIMD);

Similarly.

>        matchs ("end target teams distribute parallel do simd",
> -	      gfc_match_omp_eos_error,
> +	      gfc_match_omp_end_nowait,
>  	      ST_OMP_END_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD);
> -      matcho ("end target teams distribute parallel do", gfc_match_omp_eos_error,
> +      matcho ("end target teams distribute parallel do",
> +	      gfc_match_omp_end_nowait,
>  	      ST_OMP_END_TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO);
> -      matchs ("end target teams distribute simd", gfc_match_omp_eos_error,
> +      matchs ("end target teams distribute simd", gfc_match_omp_end_nowait,
>  	      ST_OMP_END_TARGET_TEAMS_DISTRIBUTE_SIMD);
> -      matcho ("end target teams distribute", gfc_match_omp_eos_error,
> +      matcho ("end target teams distribute", gfc_match_omp_end_nowait,
>  	      ST_OMP_END_TARGET_TEAMS_DISTRIBUTE);
> -      matcho ("end target teams", gfc_match_omp_eos_error, ST_OMP_END_TARGET_TEAMS);
> -      matcho ("end target", gfc_match_omp_eos_error, ST_OMP_END_TARGET);
> +      matcho ("end target teams loop", gfc_match_omp_end_nowait,
> +	      ST_OMP_END_TARGET_TEAMS_LOOP);
> +      matcho ("end target teams", gfc_match_omp_end_nowait,
> +	      ST_OMP_END_TARGET_TEAMS);
> +      matcho ("end target", gfc_match_omp_end_nowait, ST_OMP_END_TARGET);

Similarly all the above.  !$omp end target
followed by anything should accept nowait.

>        matcho ("end taskgroup", gfc_match_omp_eos_error, ST_OMP_END_TASKGROUP);
>        matchs ("end taskloop simd", gfc_match_omp_eos_error,
>  	      ST_OMP_END_TASKLOOP_SIMD);
>        matcho ("end taskloop", gfc_match_omp_eos_error, ST_OMP_END_TASKLOOP);
>        matcho ("end task", gfc_match_omp_eos_error, ST_OMP_END_TASK);
> -      matchs ("end teams distribute parallel do simd", gfc_match_omp_eos_error,
> +      matchs ("end teams distribute parallel do simd", gfc_match_omp_end_nowait,
>  	      ST_OMP_END_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD);
> -      matcho ("end teams distribute parallel do", gfc_match_omp_eos_error,
> +      matcho ("end teams distribute parallel do", gfc_match_omp_end_nowait,

These again shouldn't allow nowait, the outermost leaf construct
that accepts nowait is do and when do is combined with parallel, nowait
shouldn't be specified.

	Jakub


  reply	other threads:[~2021-11-11 18:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-11 17:11 Tobias Burnus
2021-11-11 18:01 ` Jakub Jelinek [this message]
2021-11-12 11:01   ` Tobias Burnus
2021-11-12 12:02     ` Jakub Jelinek
2021-11-12 15:56       ` Tobias Burnus
2021-11-12 16:07         ` Jakub Jelinek

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=20211111180114.GU2710@tucnak \
    --to=jakub@redhat.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=tobias@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).