public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug fortran/98011] New: [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)?
@ 2020-11-26 10:59 tschwinge at gcc dot gnu.org
  2020-11-26 11:41 ` [Bug fortran/98011] " burnus at gcc dot gnu.org
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: tschwinge at gcc dot gnu.org @ 2020-11-26 10:59 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98011

            Bug ID: 98011
           Summary: [OpenACC] 'gcc/fortran/scanner.c:load_line' should
                    consider 'flag_openacc' in addition to 'flag_openmp'
                    (and vice versa?)?
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: openacc
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tschwinge at gcc dot gnu.org
                CC: burnus at gcc dot gnu.org, jakub at gcc dot gnu.org
  Target Milestone: ---

It's not obvious (to me, at least), why in 'gcc/fortran/scanner.c:load_line'
'flag_openmp' and 'flag_openacc' are handled differently:

    /* For truncation and tab warnings, set seen_comment to false if one has
       either an OpenMP or OpenACC directive - or a !GCC$ attribute.  If
       OpenMP is enabled, use '!$' as as conditional compilation sentinel
       and OpenMP directive ('!$omp').  */
    if (seen_comment && first_comment && flag_openmp && comment_ix + 1 == i
        && c == '$')
      first_comment = seen_comment = false;
    if (seen_comment && first_comment && comment_ix + 4 == i)
      {
        if (((*pbuf)[comment_ix+1] == 'g' || (*pbuf)[comment_ix+1] == 'G')
            && ((*pbuf)[comment_ix+2] == 'c' || (*pbuf)[comment_ix+2] == 'C')
            && ((*pbuf)[comment_ix+3] == 'c' || (*pbuf)[comment_ix+3] == 'C')
            && c == '$')
          first_comment = seen_comment = false;
        if (flag_openacc
            && (*pbuf)[comment_ix+1] == '$'
            && ((*pbuf)[comment_ix+2] == 'a' || (*pbuf)[comment_ix+2] == 'A')
            && ((*pbuf)[comment_ix+3] == 'c' || (*pbuf)[comment_ix+3] == 'C')
            && (c == 'c' || c == 'C'))
          first_comment = seen_comment = false;
      }

Shouldn't this also be handled vice versa?

If that indeed is meant to be different, then let's please add some "dummy
handling"/commentary to make this explicit, to show that we did consider this.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug fortran/98011] [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)?
  2020-11-26 10:59 [Bug fortran/98011] New: [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)? tschwinge at gcc dot gnu.org
@ 2020-11-26 11:41 ` burnus at gcc dot gnu.org
  2020-11-26 11:59 ` burnus at gcc dot gnu.org
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: burnus at gcc dot gnu.org @ 2020-11-26 11:41 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98011

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> ---
The OpenMP version is a bit crude, but OpenMP has besides
  !$omp <directive>
also
  !$ <something>
as "conditional compilation sentinel".

In free-form source code: "Initial lines must have a space after the sentinel".
Still, it could/should be improved.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug fortran/98011] [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)?
  2020-11-26 10:59 [Bug fortran/98011] New: [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)? tschwinge at gcc dot gnu.org
  2020-11-26 11:41 ` [Bug fortran/98011] " burnus at gcc dot gnu.org
@ 2020-11-26 11:59 ` burnus at gcc dot gnu.org
  2020-11-30 14:37 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: burnus at gcc dot gnu.org @ 2020-11-26 11:59 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98011

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Regarding OpenACC: There is something going wrong here (-fopenacc):

../testsuite/gfortran.dg/goacc/sentinel-free-form.f95:13:6:

   13 |   !$ acc parallel ! { dg-error "Unclassifiable statement" }
      |      1
Error: Unclassifiable statement at (1)

I do not understand why it parses '!$'. It should regard it as comment; that's
not OpenMP!

 * * *

I wrote (for OpenMP's conditional compilation sentinel):
> In free-form source code: "Initial lines must have a space after the sentinel".

I wonder whether checking this will break any real-world code.
Otherwise, the following patch would work.

We could also add a warning similar to the OpenACC warning:
  Warning: !$ACC at (1) starts a commented line as it neither is followed by a
space nor is a continuation line
to mitigate the real-world code issue.

--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -1866,10 +1866,11 @@ load_line (FILE *input, gfc_char_t **pbuf, int
*pbuflen, 

       /* For truncation and tab warnings, set seen_comment to false if one has
         either an OpenMP or OpenACC directive - or a !GCC$ attribute.  If
-        OpenMP is enabled, use '!$' as as conditional compilation sentinel
-        and OpenMP directive ('!$omp').  */
-      if (seen_comment && first_comment && flag_openmp && comment_ix + 1 == i
-         && c == '$')
+        OpenMP is enabled, use '!$' as as conditional compilation sentinel. 
*/
+      if (seen_comment && first_comment && flag_openmp
+         && ((gfc_current_form != FORM_FREE && comment_ix + 1 == i && c ==
'$')
+             || (gfc_current_form == FORM_FREE && comment_ix + 2 == i
+                 && (*pbuf)[comment_ix+1] == '$' && c == ' ')))
        first_comment = seen_comment = false;
       if (seen_comment && first_comment && comment_ix + 4 == i)
        {
@@ -1884,6 +1885,12 @@ load_line (FILE *input, gfc_char_t **pbuf, int *pbuflen, 
              && ((*pbuf)[comment_ix+3] == 'c' || (*pbuf)[comment_ix+3] == 'C')
              && (c == 'c' || c == 'C'))
            first_comment = seen_comment = false;
+         if (flag_openmp
+             && (*pbuf)[comment_ix+1] == '$'
+             && ((*pbuf)[comment_ix+2] == 'o' || (*pbuf)[comment_ix+2] == 'O')
+             && ((*pbuf)[comment_ix+3] == 'm' || (*pbuf)[comment_ix+3] == 'M')
+             && (c == 'p' || c == 'P'))
+           first_comment = seen_comment = false;
        }

       /* Vendor extension: "<tab>1" marks a continuation line.  */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug fortran/98011] [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)?
  2020-11-26 10:59 [Bug fortran/98011] New: [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)? tschwinge at gcc dot gnu.org
  2020-11-26 11:41 ` [Bug fortran/98011] " burnus at gcc dot gnu.org
  2020-11-26 11:59 ` burnus at gcc dot gnu.org
@ 2020-11-30 14:37 ` cvs-commit at gcc dot gnu.org
  2020-11-30 14:48 ` burnus at gcc dot gnu.org
  2023-10-15 21:12 ` cvs-commit at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-11-30 14:37 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98011

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:1d6f6ac693a8601bef9fe4ba72eb6fbf7b60b5cd

commit r11-5572-g1d6f6ac693a8601bef9fe4ba72eb6fbf7b60b5cd
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Mon Nov 30 15:30:51 2020 +0100

    Fortran: With OpenACC, ignore OpenMP's cond comp sentinels

    gcc/fortran/ChangeLog:

            PR fortran/98011
            * scanner.c (skip_free_comments, skip_fixed_comments): If only
            -fopenacc but not -fopenmp is used, ignore OpenMP's conditional
            compilation sentinels. Fix indentation, use 'else if' for
readability.

    gcc/testsuite/ChangeLog:

            PR fortran/98011
            * gfortran.dg/goacc/sentinel-free-form.f95:
            * gfortran.dg/goacc-gomp/fixed-1.f: New test.
            * gfortran.dg/goacc-gomp/free-1.f90: New test.
            * gfortran.dg/goacc/fixed-5.f: New test.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug fortran/98011] [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)?
  2020-11-26 10:59 [Bug fortran/98011] New: [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)? tschwinge at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2020-11-30 14:37 ` cvs-commit at gcc dot gnu.org
@ 2020-11-30 14:48 ` burnus at gcc dot gnu.org
  2023-10-15 21:12 ` cvs-commit at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: burnus at gcc dot gnu.org @ 2020-11-30 14:48 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98011

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Actually, the code in gcc/fortran/scanner.c load_line is fine. It does load too
much for OpenMP – but that only wastes memory.

The actual check for '!$omp' vs. '!$ ' is done in skip_free_comments and
skip_fixed_comments (which has to do some additional checking). And that check
works well.

However, as mentioned in comment 2, 'gfortran -fopenacc' did process OpenMP's
'!$ ' conditional-compilation sentinels (and OpenACC does not have its own
sentinels).

That issue has now been FIXED for mainline.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug fortran/98011] [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)?
  2020-11-26 10:59 [Bug fortran/98011] New: [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)? tschwinge at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2020-11-30 14:48 ` burnus at gcc dot gnu.org
@ 2023-10-15 21:12 ` cvs-commit at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-10-15 21:12 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98011

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:643a5223f1a1860a88c215d715f079d7f846843c

commit r14-4651-g643a5223f1a1860a88c215d715f079d7f846843c
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Sun Oct 15 23:10:17 2023 +0200

    libgomp.texi: Update "Enabling OpenMP" + OpenACC / invoke.texi:
-fopenacc/-fopenmp update

    The OpenACC specification does not mention the '!$ ' sentinel for
conditional
    compilation and the feature was removed in r11-5572-g1d6f6ac693a860
    for PR fortran/98011; update libgomp.texi for this and update a leftover
    comment. - Additionally, some other updates are done as well.

    libgomp/
            * libgomp.texi (Enabling OpenMP): Update for C/C++ attributes;
            improve wording especially for Fortran; mention -fopenmp-simd.
            (Enabling OpenACC): Minor cleanup; remove conditional compilation
            sentinel.

    gcc/
            * doc/invoke.texi (-fopenacc, -fopenmp, -fopenmp-simd): Use @samp
not
            @code; document more completely the supported Fortran sentinels.

    gcc/fortran
            * scanner.cc (skip_free_comments, skip_fixed_comments): Remove
            leftover 'OpenACC' from comments about OpenMP's conditional
            compilation sentinel.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-15 21:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-26 10:59 [Bug fortran/98011] New: [OpenACC] 'gcc/fortran/scanner.c:load_line' should consider 'flag_openacc' in addition to 'flag_openmp' (and vice versa?)? tschwinge at gcc dot gnu.org
2020-11-26 11:41 ` [Bug fortran/98011] " burnus at gcc dot gnu.org
2020-11-26 11:59 ` burnus at gcc dot gnu.org
2020-11-30 14:37 ` cvs-commit at gcc dot gnu.org
2020-11-30 14:48 ` burnus at gcc dot gnu.org
2023-10-15 21:12 ` cvs-commit at gcc dot gnu.org

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