public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed] Fix fortran OpenMP schedule modifier parsing and resolving (PR fortran/87725)
@ 2018-10-25 10:41 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2018-10-25 10:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: fortran

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  <jakub@redhat.com>

	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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2018-10-25  8:06 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 10:41 [committed] Fix fortran OpenMP schedule modifier parsing and resolving (PR fortran/87725) Jakub Jelinek

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