[Bug fortran/94690] New: [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[Bug fortran/94690] New: [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

            Bug ID: 94690
           Summary: [OpenMP] omp ... distribute – lastprivate not
                    permitted and more issues
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Keywords: openmp, rejects-valid
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org
                CC: jakub at gcc dot gnu.org
  Target Milestone: ---

Created attachment 48323
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48323&action=edit
[Patch, see comment 0 for issues] Add LASTPRIVATE to OMP_DISTRIBUTE_CLAUSES,
add a bunch of test cases

sollve_vv's test_target_teams_distribute_lastprivate.F90 fails as gfortran
misses 'lastprivate' in 'distribute' (missing from OMP_DISTRIBUTE_CLAUSES).

However, when trying to fortranify the C/C++ testcases pr66199-[3-9].c, some
more issues showed up:

* pr66199-4.f90 + pr66199-5.f90 do not recognize 'collapse(2)'
* pr66199-7.f90 fails with ‘d2’ not specified in enclosing ‘teams’,
  which might be a test case (conversion) bug.
* pr66199-9.f90 – gives an ICE in omp_add_variable
  (if 'lastprivate' is permitted, otherwise, it is rejected.)

[The C/C++ test cases were added in r228777 alias
r6-3899-gd9a6bd32adc40a7e1e5c72692a330f14453ad7f0 ]

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> ---
However, adding LASTPRIVATE to OMP_DISTRIBUTE_CLAUSES (see patch) *is*
sufficient for sollve_vv's test_target_teams_distribute_lastprivate.F90.

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #0)
> * pr66199-4.f90 + pr66199-5.f90 do not recognize 'collapse(2)'

Turns out this is due to truncating of the line...

EXPECTED: There is some diagnostic for free-form Fortran and for
-Wline-truncation also for fixed-form Fortran source form.

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #2)
> EXPECTED: There is some diagnostic for free-form Fortran and for
> -Wline-truncation also for fixed-form Fortran source form.

That's fixed by the following patch. It is first set to disable tab warnings
but after exceeding the line length, it can be set again.

The too-long line warning itself is taken care of in gfc_next_char_literal,
which also knows about comments, OpenMP etc.

diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index f65594f8baa..d1c9379ba88 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -1902,2 +1902,3 @@ load_line (FILE *input, gfc_char_t **pbuf, int *pbuflen,
const int *first_char)
          bool trunc_warn = true;
+         seen_comment = false;

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #3)
> (In reply to Tobias Burnus from comment #2)
> > EXPECTED: There is some diagnostic for free-form Fortran and for
> > -Wline-truncation also for fixed-form Fortran source form.

That's now tracked in PR 94709. The simple patch is incomplete (see new PR).

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #0)
> * pr66199-9.f90 – gives an ICE in omp_add_variable
>   (if 'lastprivate' is permitted, otherwise, it is rejected.)

Reduced test case (needs patch to permit lastprivate in
OMP_DISTRIBUTE_CLAUSES):
  internal compiler error: in omp_add_variable, at gimplify.c:6958

integer function f2 ()
  implicit none
  integer :: d, a
! !$omp teams shared(d) shared(a)
  !$omp distribute lastprivate(d)
  do d = 1, 2
    a = d * 5
  end do

! !$omp end teams
  f2 = d
end

["omp teams" is not needed for the ICE, but without the C testcase fails with:
 "lastprivate variable ‘d’ is private in outer context"]

The problem is (original dump):
  #pragma omp distribute private(d) lastprivate(d)

Mixing 'private' + 'lastprivate' is bad.

 * * *

The private is added in gfc_trans_omp_do, as dovar_found == 0.

We have:
* clauses->lists[OMP_LIST_LASTPRIVATE]->sym->name
$41 = 0x7ffff78c0128 "d"

* clauses->lists[OMP_LIST_PRIVATE] == NULL

And the code (in gfc_trans_omp_do):

          if (op != EXEC_OMP_DISTRIBUTE)
            for (n = clauses->lists[(op == EXEC_OMP_SIMD && collapse == 1)
                                    ? OMP_LIST_LINEAR : OMP_LIST_LASTPRIVATE];
...
          else if (n == NULL && op != EXEC_OMP_SIMD)
            for (n = clauses->lists[OMP_LIST_PRIVATE]; n != NULL; n = n->next)

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Tobias Burnus from comment #5)
> The problem is (original dump):
>   #pragma omp distribute private(d) lastprivate(d)

If one does not add the 'private(d)', it works: 'private(d)' is added by the ME
only if 'lastprivate(d)' is not present.

Namely, the following patch works – but the question is, whence to add a
clause. (There is more such code.)

--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -4312,7 +4312,5 @@ gfc_trans_omp_do (gfc_code *code, gfc_exec_op op,
stmtblock_t *pblock,
                dovar_found = 2;
-           }
-         else
-           tmp = build_omp_clause (input_location, OMP_CLAUSE_PRIVATE);
          OMP_CLAUSE_DECL (tmp) = dovar_decl;
          omp_clauses = gfc_trans_add_clause (tmp, omp_clauses);
+           }
        }

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
If no lastprivate is available, it is added in gimplify.c's gimplify_omp_for,
line 11759: 

  for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt)); i++)
...
      if (is_doacross)
...
      else if (ort == ORT_SIMD)
...
      else if (omp_is_private (gimplify_omp_ctxp, decl, 0))
...
      else
        omp_add_variable (gimplify_omp_ctxp, decl, GOVD_PRIVATE | GOVD_SEEN);

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2020-04-23
           Assignee|unassigned at gcc dot gnu.org      |burnus at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #8 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Patch: https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544387.html

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 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:f884bef21cccc05d748fd7869cd641cbb4f6b6bb

commit r11-349-gf884bef21cccc05d748fd7869cd641cbb4f6b6bb
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Wed May 13 10:06:45 2020 +0200

    [Fortran] OpenMP - permit lastprivate in distribute + SIMD fixes (PR94690)

    gcc/fortran/
    2020-05-13  Tobias Burnus  <tobias@codesourcery.com>

            PR fortran/94690
            * openmp.c (OMP_DISTRIBUTE_CLAUSES): Add OMP_CLAUSE_LASTPRIVATE.
            (gfc_resolve_do_iterator): Skip the private handling for SIMD as
            that is handled by ME code.
            * trans-openmp.c (gfc_trans_omp_do): Don't add private/lastprivate
            for dovar_found == 0, unless !simple.

    libgomp/
    2020-05-13  Tobias Burnus  <tobias@codesourcery.com>

            PR fortran/94690
            * testsuite/libgomp.fortran/pr66199-3.f90: New.
            * testsuite/libgomp.fortran/pr66199-4.f90: New.
            * testsuite/libgomp.fortran/pr66199-5.f90: New.
            * testsuite/libgomp.fortran/pr66199-6.f90: New.
            * testsuite/libgomp.fortran/pr66199-7.f90: New.
            * testsuite/libgomp.fortran/pr66199-8.f90: New.
            * testsuite/libgomp.fortran/pr66199-9.f90: New.

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #10 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Created attachment 48522
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48522&action=edit
Patch to add OpenMP 5 feature (private + lastprivate permitted for 'simd')

The patch solves an independent issue, required for the test case below
(namely, the "private(j)").

The *committed* patch causes gfortran.dg/gomp/target1.f90 to FAIL.

The problem shows up with the following test case (reduced but modified):
-------------
  subroutine foo ()
    integer :: s, i, j
    !$omp target teams
    !$omp distribute parallel do default(shared) &
    !$omp collapse (2) private(j) lastprivate (s)
      do i = 1, 10
        do j = 1, 10
          s = i * 10 + j
        end do
      end do
    !$omp end target teams
  end subroutine
-------------

The problem is that this currently produces (-fdump-tree-original):
  #pragma omp teams private(i) private(j)

Which later causes problems as it doesn't add a  (last)private(i) when it
should 

(Search for "if (outer && lastprivate)" in gimplify_omp_for – and look through
the lines below. As "i" is present, omp_check_private() returns true and a
required omp_add_variable is not called. – Leading later to the ICE.)

That's solved by:

--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -5688,3 +5688,3 @@ gfc_resolve_do_iterator (gfc_code *code, gfc_symbol *sym,
bool add_clause)
       gfc_code *omp_code = omp_current_ctx->code->block;
-      for ( ; omp_code->next; omp_code = omp_code->next)
+      for ( ; omp_code; omp_code = omp_code->next)
        switch (omp_code->op)

However, that leads to the odd result that:

  !$omp target teams
    !$omp distribute parallel do default(shared) &  ! "do"
    !$omp collapse (2) private(j) lastprivate (s)
    ...
    !$omp distribute parallel do simd default(shared) & ! "do simd" (!)
    !$omp collapse (2) private(j) lastprivate (s)
    ...

has "#pragma omp teams" [w/o private(i) private(j)]

While removing the "simd" from the code above leads to no "private(i)
private(j)", which does not make sense.

The code (openmp.c's gfc_do_resolve_do_iterator) is supposed to do:
  /* !$omp do and !$omp parallel do iteration variable is predetermined
     private just in the !$omp do resp. !$omp parallel do construct,
     with no implications for the outer parallel constructs.  */

But here it adds it to 'teams', which looks wrong/inconsistent.

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 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:0ec52417fd9b3bef5227cdc9a18ff4f0247b0ea4

commit r11-421-g0ec52417fd9b3bef5227cdc9a18ff4f0247b0ea4
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Fri May 15 16:40:34 2020 +0200

    [Fortran] OpenMP 5 â permit more sharing clauses for SIMD (PR94690)

    gcc/fortran/
            PR fortran/94690
            * openmp.c (resolve_omp_do): Permit more clauses for SIMD
            iteration variables.

    gcc/testsuite/
            PR fortran/94690
            * gfortran.dg/gomp/openmp-simd-4.f90: New test.

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[schwab@linux-m68k.org]

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

--- Comment #12 from Andreas Schwab <schwab@linux-m68k.org> ---
This breaks gfortran.dg/gomp/target1.f90 on riscv.

/daten/src/gcc/gcc/gcc/testsuite/gfortran.dg/gomp/target1.f90:318:0: internal
compiler error: in lookup_decl_in_outer_ctx, at omp-low.c:3967
0xbb9b5a lookup_decl_in_outer_ctx
        ../../gcc/gcc/omp-low.c:3967
0xbe0b76 lower_send_clauses
        ../../gcc/gcc/omp-low.c:7290
0xbe0b76 lower_omp_taskreg
        ../../gcc/gcc/omp-low.c:11295
0xbca7ae lower_omp_1
        ../../gcc/gcc/omp-low.c:12878
0xbca7ae lower_omp
        ../../gcc/gcc/omp-low.c:13015
0xbc9bbd lower_omp_1
        ../../gcc/gcc/omp-low.c:12807
0xbc9bbd lower_omp
        ../../gcc/gcc/omp-low.c:13015
0xbe2367 lower_omp_for
        ../../gcc/gcc/omp-low.c:10564
0xbcaae6 lower_omp_1
        ../../gcc/gcc/omp-low.c:12823
0xbcaae6 lower_omp
        ../../gcc/gcc/omp-low.c:13015
0xbc9bbd lower_omp_1
        ../../gcc/gcc/omp-low.c:12807
0xbc9bbd lower_omp
        ../../gcc/gcc/omp-low.c:13015
0xbc9bbd lower_omp_1
        ../../gcc/gcc/omp-low.c:12807
0xbc9bbd lower_omp
        ../../gcc/gcc/omp-low.c:13015
0xbc9bbd lower_omp_1
        ../../gcc/gcc/omp-low.c:12807
0xbc9bbd lower_omp
        ../../gcc/gcc/omp-low.c:13015
0xbc9bbd lower_omp_1
        ../../gcc/gcc/omp-low.c:12807
0xbc9bbd lower_omp
        ../../gcc/gcc/omp-low.c:13015
0xbc9bbd lower_omp_1
        ../../gcc/gcc/omp-low.c:12807
0xbc9bbd lower_omp
        ../../gcc/gcc/omp-low.c:13015

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

--- Comment #13 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Andreas Schwab from comment #12)
> This breaks gfortran.dg/gomp/target1.f90 on riscv.
See comment 10 – or PR95109, which points to comment 10.

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[cvs-commit at gcc dot gnu.org]

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

--- Comment #14 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:61c2d476a52bb108bd05d0226c5522bf0c4b24b5

commit r11-3066-g61c2d476a52bb108bd05d0226c5522bf0c4b24b5
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Wed Sep 9 09:33:51 2020 +0200

    Fortran: Fixes for OpenMP loop-iter privatization (PRs 95109 + 94690)

    This commit also fixes a gfortran.dg/gomp/target1.f90 regression;
    target1.f90 tests the resolve.c and openmp.c changes.

    gcc/fortran/ChangeLog:

            PR fortran/95109
            PR fortran/94690
            * resolve.c (gfc_resolve_code): Also call
            gfc_resolve_omp_parallel_blocks for 'distribute parallel do
(simd)'.
            * openmp.c (gfc_resolve_omp_parallel_blocks): Handle it.
            (gfc_resolve_do_iterator): Remove special code for SIMD, which is
            not needed.
            * trans-openmp.c (gfc_trans_omp_target): For
TARGET_PARALLEL_DO_SIMD,
            call simd not do processing function.

    gcc/testsuite/ChangeLog:

            PR fortran/95109
            PR fortran/94690
            * gfortran.dg/gomp/combined-if.f90: Update scan-tree-dump-times for
            'omp simd.*if'.
            * gfortran.dg/gomp/openmp-simd-5.f90: New test.

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:f93eba8c5fde83100bf0854607848b6f50c8bbb2

commit r10-8721-gf93eba8c5fde83100bf0854607848b6f50c8bbb2
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Wed Sep 9 11:54:43 2020 +0200

    Fortran: Fixes for OpenMP loop-iter privatization (PRs 95109 + 94690)

    This commit also fixes a gfortran.dg/gomp/target1.f90 regression;
    target1.f90 tests the resolve.c and openmp.c changes.

    gcc/fortran/ChangeLog:

            PR fortran/95109
            PR fortran/94690
            * resolve.c (gfc_resolve_code): Also call
            gfc_resolve_omp_parallel_blocks for 'distribute parallel do
(simd)'.
            * openmp.c (gfc_resolve_omp_parallel_blocks): Handle it.
            * trans-openmp.c (gfc_trans_omp_target): For
TARGET_PARALLEL_DO_SIMD,
            call simd not do processing function.

    gcc/testsuite/ChangeLog:

            PR fortran/95109
            PR fortran/94690
            * gfortran.dg/gomp/openmp-simd-5.f90: New test.

    (cherry picked from commit 61c2d476a52bb108bd05d0226c5522bf0c4b24b5)

[Bug fortran/94690] [OpenMP] omp ... distribute – lastprivate not permitted and more issues

[burnus at gcc dot gnu.org]

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

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

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

--- Comment #16 from Tobias Burnus <burnus at gcc dot gnu.org> ---
FIXED on mainline (GCC 11), backported to GCC 10 and to OG10 =
devel/omp/gcc-10.