[committed] Fix fortran OpenMP schedule modifier parsing and resolving (PR fortran/87725)

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[committed] Fix fortran OpenMP schedule modifier parsing and resolving (PR fortran/87725)

Jakub Jelinek
Hi!

In the C/C++ FE, we are using:
      if (nmodifiers++ == 0
          && cp_lexer_next_token_is (parser->lexer, CPP_COMMA))
but it is in a loop guarded on that the next token is a CPP_NAME.
The point of the check is to parse the comma at most once, because
the syntax allows 0, 1 or 2 schedule modifiers and if there is 1 or 2 of
them, it must be followed by colon.
In the Fortran FE, nmodifiers is bumped when gfc_match succeeds, and
so if (nmodifiers == 0
after it is incorrect, that will allow incorrect schedule (, simd: dynamic, 2)
but not correct schedule (simd, nonmonotonic: dynamic, 2).
The following patch fixes that, allows (meaningless, to be fixed in OpenMP 5.1)
duplication of the same modifiers twice and improves or adds diagnostics
for some restrictions.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk,
queued for backports.

2018-10-25  Jakub Jelinek  <[hidden email]>

        PR fortran/87725
        * openmp.c (gfc_match_omp_clauses): Parse simd, monotonic and
        nonmonotonic modifiers regardless of if they have been parsed
        already or if the opposite one has.  Fix up check whether
        comma after modifier should be parsed.
        (resolve_omp_clauses): Diagnose schedule modifier restrictions.

        * c-c++-common/gomp/schedule-modifiers-1.c (bar): Separate modifier
        from kind with a colon rather than comma.
        * gfortran.dg/gomp/schedule-modifiers-1.f90: New test.
        * gfortran.dg/gomp/schedule-modifiers-2.f90: New test.

--- gcc/fortran/openmp.c.jj 2018-09-25 15:14:47.455199379 +0200
+++ gcc/fortran/openmp.c 2018-10-24 18:56:46.473997792 +0200
@@ -1710,22 +1710,17 @@ gfc_match_omp_clauses (gfc_omp_clauses *
       locus old_loc2 = gfc_current_locus;
       do
  {
-  if (!c->sched_simd
-      && gfc_match ("simd") == MATCH_YES)
+  if (gfc_match ("simd") == MATCH_YES)
     {
       c->sched_simd = true;
       nmodifiers++;
     }
-  else if (!c->sched_monotonic
-   && !c->sched_nonmonotonic
-   && gfc_match ("monotonic") == MATCH_YES)
+  else if (gfc_match ("monotonic") == MATCH_YES)
     {
       c->sched_monotonic = true;
       nmodifiers++;
     }
-  else if (!c->sched_monotonic
-   && !c->sched_nonmonotonic
-   && gfc_match ("nonmonotonic") == MATCH_YES)
+  else if (gfc_match ("nonmonotonic") == MATCH_YES)
     {
       c->sched_nonmonotonic = true;
       nmodifiers++;
@@ -1736,7 +1731,7 @@ gfc_match_omp_clauses (gfc_omp_clauses *
  gfc_current_locus = old_loc2;
       break;
     }
-  if (nmodifiers == 0
+  if (nmodifiers == 1
       && gfc_match (" , ") == MATCH_YES)
     continue;
   else if (gfc_match (" : ") == MATCH_YES)
@@ -4075,6 +4070,30 @@ resolve_omp_clauses (gfc_code *code, gfc
  gfc_warning (0, "INTEGER expression of SCHEDULE clause's chunk_size "
      "at %L must be positive", &expr->where);
     }
+  if (omp_clauses->sched_kind != OMP_SCHED_NONE
+      && omp_clauses->sched_nonmonotonic)
+    {
+      if (omp_clauses->sched_kind != OMP_SCHED_DYNAMIC
+  && omp_clauses->sched_kind != OMP_SCHED_GUIDED)
+ {
+  const char *p;
+  switch (omp_clauses->sched_kind)
+    {
+    case OMP_SCHED_STATIC: p = "STATIC"; break;
+    case OMP_SCHED_RUNTIME: p = "RUNTIME"; break;
+    case OMP_SCHED_AUTO: p = "AUTO"; break;
+    default: gcc_unreachable ();
+    }
+  gfc_error ("NONMONOTONIC modifier specified for %s schedule kind "
+     "at %L", p, &code->loc);
+ }
+      else if (omp_clauses->sched_monotonic)
+ gfc_error ("Both MONOTONIC and NONMONOTONIC schedule modifiers "
+   "specified at %L", &code->loc);
+      else if (omp_clauses->ordered)
+ gfc_error ("NONMONOTONIC schedule modifier specified with ORDERED "
+   "clause at %L", &code->loc);
+    }
 
   /* Check that no symbol appears on multiple clauses, except that
      a symbol can appear on both firstprivate and lastprivate.  */
--- gcc/testsuite/c-c++-common/gomp/schedule-modifiers-1.c.jj 2015-11-05 16:03:53.007122210 +0100
+++ gcc/testsuite/c-c++-common/gomp/schedule-modifiers-1.c 2018-10-24 18:58:34.975212574 +0200
@@ -80,21 +80,21 @@ bar (void)
   #pragma omp for schedule (nonmonotonic : auto) /* { dg-error ".nonmonotonic. modifier specified for .auto. schedule kind" } */
   for (i = 0; i < 64; i++)
     ;
-  #pragma omp for schedule (nonmonotonic, dynamic) ordered /* { dg-error ".nonmonotonic. schedule modifier specified together with .ordered. clause" } */
+  #pragma omp for schedule (nonmonotonic : dynamic) ordered /* { dg-error ".nonmonotonic. schedule modifier specified together with .ordered. clause" } */
   for (i = 0; i < 64; i++)
     #pragma omp ordered
       ;
-  #pragma omp for ordered schedule(nonmonotonic, dynamic, 5) /* { dg-error ".nonmonotonic. schedule modifier specified together with .ordered. clause" } */
+  #pragma omp for ordered schedule(nonmonotonic : dynamic, 5) /* { dg-error ".nonmonotonic. schedule modifier specified together with .ordered. clause" } */
   for (i = 0; i < 64; i++)
     #pragma omp ordered
       ;
-  #pragma omp for schedule (nonmonotonic, guided) ordered(1) /* { dg-error ".nonmonotonic. schedule modifier specified together with .ordered. clause" } */
+  #pragma omp for schedule (nonmonotonic : guided) ordered(1) /* { dg-error ".nonmonotonic. schedule modifier specified together with .ordered. clause" } */
   for (i = 0; i < 64; i++)
     {
       #pragma omp ordered depend(sink: i - 1)
       #pragma omp ordered depend(source)
     }
-  #pragma omp for ordered(1) schedule(nonmonotonic, guided, 2) /* { dg-error ".nonmonotonic. schedule modifier specified together with .ordered. clause" } */
+  #pragma omp for ordered(1) schedule(nonmonotonic : guided, 2) /* { dg-error ".nonmonotonic. schedule modifier specified together with .ordered. clause" } */
   for (i = 0; i < 64; i++)
     {
       #pragma omp ordered depend(source)
--- gcc/testsuite/gfortran.dg/gomp/schedule-modifiers-1.f90.jj 2018-10-24 17:35:21.903117157 +0200
+++ gcc/testsuite/gfortran.dg/gomp/schedule-modifiers-1.f90 2018-10-24 17:55:41.058985314 +0200
@@ -0,0 +1,63 @@
+! { dg-do compile }
+! { dg-options "-fopenmp" }
+
+subroutine foo
+  integer :: i
+  !$omp do simd schedule (simd, simd: static, 5)
+  do i = 0, 64
+  end do
+  !$omp do simd schedule (monotonic, simd: static)
+  do i = 0, 64
+  end do
+  !$omp do simd schedule (simd , monotonic : static, 6)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic, monotonic : static, 7)
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic, nonmonotonic : dynamic)
+  do i = 0, 64
+  end do
+  !$omp do simd schedule (nonmonotonic , simd : dynamic, 3)
+  do i = 0, 64
+  end do
+  !$omp do simd schedule (nonmonotonic,simd:guided,4)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic: static, 2)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic : static)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic : dynamic)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic : dynamic, 3)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic : guided)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic : guided, 7)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic : runtime)
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic : auto)
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic : dynamic)
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic : dynamic, 3)
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic : guided)
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic : guided, 7)
+  do i = 0, 64
+  end do
+end subroutine foo
--- gcc/testsuite/gfortran.dg/gomp/schedule-modifiers-2.f90.jj 2018-10-24 17:55:30.122165488 +0200
+++ gcc/testsuite/gfortran.dg/gomp/schedule-modifiers-2.f90 2018-10-24 18:57:44.813037899 +0200
@@ -0,0 +1,44 @@
+! { dg-do compile }
+! { dg-options "-fopenmp" }
+
+subroutine foo
+  integer :: i
+  !$omp do schedule (nonmonotonic: static, 2) ! { dg-error "NONMONOTONIC modifier specified for STATIC schedule kind" }
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic : static) ! { dg-error "NONMONOTONIC modifier specified for STATIC schedule kind" }
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic : runtime) ! { dg-error "NONMONOTONIC modifier specified for RUNTIME schedule kind" }
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic : auto) ! { dg-error "NONMONOTONIC modifier specified for AUTO schedule kind" }
+  do i = 0, 64
+  end do
+  !$omp do schedule (nonmonotonic : dynamic) ordered ! { dg-error "NONMONOTONIC schedule modifier specified with ORDERED clause" }
+  do i = 0, 64
+    !$omp ordered
+    !$omp end ordered
+  end do
+  !$omp do ordered schedule(nonmonotonic : dynamic, 5) ! { dg-error "NONMONOTONIC schedule modifier specified with ORDERED clause" }
+  do i = 0, 64
+    !$omp ordered
+    !$omp end ordered
+  end do
+  !$omp do schedule (nonmonotonic : guided) ordered(1) ! { dg-error "NONMONOTONIC schedule modifier specified with ORDERED clause" }
+  do i = 0, 64
+    !$omp ordered depend(sink: i - 1)
+    !$omp ordered depend(source)
+  end do
+  !$omp do ordered(1) schedule(nonmonotonic : guided, 2) ! { dg-error "NONMONOTONIC schedule modifier specified with ORDERED clause" }
+  do i = 0, 64
+    !$omp ordered depend(source)
+    !$ordered depend(sink: i - 1)
+  end do
+  !$omp do schedule (nonmonotonic , monotonic : dynamic) ! { dg-error "Both MONOTONIC and NONMONOTONIC schedule modifiers specified" }
+  do i = 0, 64
+  end do
+  !$omp do schedule (monotonic,nonmonotonic:dynamic) ! { dg-error "Both MONOTONIC and NONMONOTONIC schedule modifiers specified" }
+  do i = 0, 64
+  end do
+end subroutine foo

        Jakub