[gomp] Error on firstprivate or reduction omp for iteration var (PR middle-end/27415)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

We fail to diagnose firstprivate or reduction omp for iteration var, as
shown on the attached testcases.  The iterator is predetermined private and
as exception it can be listed explicitly in private or lastprivate clauses.
The patch is so big because at least as far as I understand the standard,
there is a difference between
#pragma omp parallel firstprivate(i)
#pragma omp for
  for (i = 0; i < 10; ++i)
    ;
(where i is predetermined private in omp for and explicitly determined
firstprivate in the outer parallel) and
#pragma omp parallel for firstprivate(i)
  for (i = 0; i < 10; ++i)
    ;
which ought to be invalid (i is predetermined private in the combined
parallel work-sharing construct, but at the same time explicitly
determined), so the patch adds a flag which says whether OMP_PARALLEL comes
from an explicit combined parallel work-sharing pragma or not.

Ok for trunk?

2006-05-16  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/27415
	* tree.h (OMP_PARALLEL_COMBINED): Define.
	* gimplify.c (struct gimplify_omp_ctx): Add is_combined_parallel field.
	(new_omp_context): Add is_combined_parallel argument.
	(gimplify_scan_omp_clauses): Add in_combined_parallel argument, adjust
	new_omp_context caller.
	(gimplify_omp_parallel, gimplify_omp_for, gimplify_omp_workshare):
	Adjust gimplify_scan_omp_clauses callers.
	(omp_is_private): Issue errors if iteration variable is firstprivate
	or reduction in the current context.
	* c-parser.c (c_parser_omp_parallel): Set OMP_PARALLEL_COMBINED
	on combined parallel workshare constructs.
cp/
	* parser.c (cp_parser_omp_parallel): Set OMP_PARALLEL_COMBINED
	on combined parallel workshare constructs.
	* pt.c (tsubst_expr): Copy OMP_PARALLEL_COMBINED flag.
fortran/
	* trans-openmp.c (gfc_trans_omp_parallel_do,
	gfc_trans_omp_parallel_sections, gfc_trans_omp_parallel_workshare): Set
	OMP_PARALLEL_COMBINED flag.
testsuite/
	* gcc.dg/gomp/pr27415.c: New test.
	* g++.dg/gomp/pr27415.C: New test.

--- gcc/tree.h.jj	2006-05-10 14:07:01.000000000 +0200
+++ gcc/tree.h	2006-05-16 17:57:06.000000000 +0200
@@ -443,6 +443,8 @@ struct tree_common GTY(())
 	   OMP_RETURN
        OMP_SECTION_LAST in
 	   OMP_SECTION
+       OMP_PARALLEL_COMBINED in
+	   OMP_PARALLEL
 
    protected_flag:
 
@@ -1583,6 +1585,11 @@ struct tree_constructor GTY(())
 #define OMP_RETURN_NOWAIT(NODE) \
   TREE_PRIVATE (OMP_RETURN_CHECK (NODE))
 
+/* True on an OMP_PARALLEL statement if it represents an explicit
+   combined parallel work-sharing constructs.  */
+#define OMP_PARALLEL_COMBINED(NODE) \
+  TREE_PRIVATE (OMP_PARALLEL_CHECK (NODE))
+
 /* True on a PRIVATE clause if its decl is kept around for debugging
    information only and its DECL_VALUE_EXPR is supposed to point
    to what it has been remapped to.  */
--- gcc/gimplify.c.jj	2006-05-12 22:13:05.000000000 +0200
+++ gcc/gimplify.c	2006-05-16 17:34:15.000000000 +0200
@@ -73,6 +73,7 @@ struct gimplify_omp_ctx
   location_t location;
   enum omp_clause_default_kind default_kind;
   bool is_parallel;
+  bool is_combined_parallel;
 };
 
 struct gimplify_ctx
@@ -259,7 +260,7 @@ splay_tree_compare_decl_uid (splay_tree_
 /* Create a new omp construct that deals with variable remapping.  */
 
 static struct gimplify_omp_ctx *
-new_omp_context (bool is_parallel)
+new_omp_context (bool is_parallel, bool is_combined_parallel)
 {
   struct gimplify_omp_ctx *c;
 
@@ -269,6 +270,7 @@ new_omp_context (bool is_parallel)
   c->privatized_types = pointer_set_create ();
   c->location = input_location;
   c->is_parallel = is_parallel;
+  c->is_combined_parallel = is_combined_parallel;
   c->default_kind = OMP_CLAUSE_DEFAULT_SHARED;
 
   return c;
@@ -4452,6 +4454,18 @@ omp_is_private (struct gimplify_omp_ctx 
 	  else
 	    return false;
 	}
+      else if ((n->value & GOVD_EXPLICIT) != 0
+	       && (ctx == gimplify_omp_ctxp
+		   || (ctx->is_combined_parallel
+		       && gimplify_omp_ctxp->outer_context == ctx)))
+	{
+	  if ((n->value & GOVD_FIRSTPRIVATE) != 0)
+	    error ("iteration variable %qs should not be firstprivate",
+		   IDENTIFIER_POINTER (DECL_NAME (decl)));
+	  else if ((n->value & GOVD_REDUCTION) != 0)
+	    error ("iteration variable %qs should not be reduction",
+		   IDENTIFIER_POINTER (DECL_NAME (decl)));
+	}
       return true;
     }
 
@@ -4467,12 +4481,13 @@ omp_is_private (struct gimplify_omp_ctx 
    and previous omp contexts.  */
 
 static void
-gimplify_scan_omp_clauses (tree *list_p, tree *pre_p, bool in_parallel)
+gimplify_scan_omp_clauses (tree *list_p, tree *pre_p, bool in_parallel,
+			   bool in_combined_parallel)
 {
   struct gimplify_omp_ctx *ctx, *outer_ctx;
   tree c;
 
-  ctx = new_omp_context (in_parallel);
+  ctx = new_omp_context (in_parallel, in_combined_parallel);
   outer_ctx = ctx->outer_context;
 
   while ((c = *list_p) != NULL)
@@ -4717,7 +4732,8 @@ gimplify_omp_parallel (tree *expr_p, tre
 {
   tree expr = *expr_p;
 
-  gimplify_scan_omp_clauses (&OMP_PARALLEL_CLAUSES (expr), pre_p, true);
+  gimplify_scan_omp_clauses (&OMP_PARALLEL_CLAUSES (expr), pre_p, true,
+			     OMP_PARALLEL_COMBINED (expr));
 
   push_gimplify_context ();
 
@@ -4743,7 +4759,7 @@ gimplify_omp_for (tree *expr_p, tree *pr
 
   for_stmt = *expr_p;
 
-  gimplify_scan_omp_clauses (&OMP_FOR_CLAUSES (for_stmt), pre_p, false);
+  gimplify_scan_omp_clauses (&OMP_FOR_CLAUSES (for_stmt), pre_p, false, false);
 
   t = OMP_FOR_INIT (for_stmt);
   gcc_assert (TREE_CODE (t) == MODIFY_EXPR);
@@ -4825,7 +4841,7 @@ gimplify_omp_workshare (tree *expr_p, tr
 {
   tree stmt = *expr_p;
 
-  gimplify_scan_omp_clauses (&OMP_CLAUSES (stmt), pre_p, false);
+  gimplify_scan_omp_clauses (&OMP_CLAUSES (stmt), pre_p, false, false);
   gimplify_to_stmt_list (&OMP_BODY (stmt));
   gimplify_adjust_omp_clauses (&OMP_CLAUSES (stmt));
 
--- gcc/c-parser.c.jj	2006-05-10 14:07:01.000000000 +0200
+++ gcc/c-parser.c	2006-05-16 17:09:29.000000000 +0200
@@ -7651,6 +7651,7 @@ c_parser_omp_parallel (c_parser *parser)
       if (stmt)
 	OMP_FOR_CLAUSES (stmt) = ws_clause;
       stmt = c_finish_omp_parallel (par_clause, block);
+      OMP_PARALLEL_COMBINED (stmt) = 1;
       break;
 
     case PRAGMA_OMP_PARALLEL_SECTIONS:
@@ -7660,6 +7661,7 @@ c_parser_omp_parallel (c_parser *parser)
       if (stmt)
 	OMP_SECTIONS_CLAUSES (stmt) = ws_clause;
       stmt = c_finish_omp_parallel (par_clause, block);
+      OMP_PARALLEL_COMBINED (stmt) = 1;
       break;
 
     default:
--- gcc/cp/parser.c.jj	2006-05-16 15:03:38.000000000 +0200
+++ gcc/cp/parser.c	2006-05-16 17:11:53.000000000 +0200
@@ -18809,7 +18809,10 @@ cp_parser_omp_parallel (cp_parser *parse
     }
 
   cp_parser_end_omp_structured_block (parser, save);
-  return finish_omp_parallel (par_clause, block);
+  stmt = finish_omp_parallel (par_clause, block);
+  if (p_kind != PRAGMA_OMP_PARALLEL)
+    OMP_PARALLEL_COMBINED (stmt) = 1;
+  return stmt;
 }
 
 /* OpenMP 2.5:
--- gcc/cp/pt.c.jj	2006-05-10 14:06:56.000000000 +0200
+++ gcc/cp/pt.c	2006-05-16 17:14:07.000000000 +0200
@@ -8454,7 +8454,8 @@ tsubst_expr (tree t, tree args, tsubst_f
 				args, complain, in_decl);
       stmt = begin_omp_parallel ();
       tsubst_expr (OMP_PARALLEL_BODY (t), args, complain, in_decl);
-      finish_omp_parallel (tmp, stmt);
+      OMP_PARALLEL_COMBINED (finish_omp_parallel (tmp, stmt))
+	= OMP_PARALLEL_COMBINED (t);
       break;
 
     case OMP_FOR:
--- gcc/fortran/trans-openmp.c.jj	2006-05-12 13:53:25.000000000 +0200
+++ gcc/fortran/trans-openmp.c	2006-05-16 17:18:52.000000000 +0200
@@ -1095,6 +1095,7 @@ gfc_trans_omp_parallel_do (gfc_code *cod
   else
     poplevel (0, 0, 0);
   stmt = build4_v (OMP_PARALLEL, stmt, omp_clauses, NULL, NULL);
+  OMP_PARALLEL_COMBINED (stmt) = 1;
   gfc_add_expr_to_block (&block, stmt);
   return gfc_finish_block (&block);
 }
@@ -1119,6 +1120,7 @@ gfc_trans_omp_parallel_sections (gfc_cod
   else
     poplevel (0, 0, 0);
   stmt = build4_v (OMP_PARALLEL, stmt, omp_clauses, NULL, NULL);
+  OMP_PARALLEL_COMBINED (stmt) = 1;
   gfc_add_expr_to_block (&block, stmt);
   return gfc_finish_block (&block);
 }
@@ -1143,6 +1145,7 @@ gfc_trans_omp_parallel_workshare (gfc_co
   else
     poplevel (0, 0, 0);
   stmt = build4_v (OMP_PARALLEL, stmt, omp_clauses, NULL, NULL);
+  OMP_PARALLEL_COMBINED (stmt) = 1;
   gfc_add_expr_to_block (&block, stmt);
   return gfc_finish_block (&block);
 }
--- gcc/testsuite/gcc.dg/gomp/pr27415.c.jj	2006-05-16 18:06:44.000000000 +0200
+++ gcc/testsuite/gcc.dg/gomp/pr27415.c	2006-05-16 18:12:41.000000000 +0200
@@ -0,0 +1,50 @@
+/* PR middle-end/27415 */
+/* { dg-do compile } */
+
+void
+test1 (void)
+{
+  int i = 0;
+#pragma omp parallel
+#pragma omp for firstprivate (i)		/* { dg-error "should not be firstprivate" } */
+  for (i = 0; i < 10; i++)
+    ;
+}
+
+void
+test2 (void)
+{
+  int i = 0;
+#pragma omp parallel for firstprivate (i)
+  for (i = 0; i < 10; i++)			/* { dg-error "should not be firstprivate" } */
+    ;
+}
+
+void
+test3 (void)
+{
+  int i = 0;
+#pragma omp parallel
+#pragma omp for reduction (+:i)			/* { dg-error "should not be reduction" } */
+  for (i = 0; i < 10; i++)
+    ;
+}
+
+void
+test4 (void)
+{
+  int i = 0;
+#pragma omp parallel for reduction (*:i)
+  for (i = 0; i < 10; i++)			/* { dg-error "should not be reduction" } */
+    ;
+}
+
+void
+test5 (void)
+{
+  int i = 0;
+#pragma omp parallel firstprivate (i)
+#pragma omp for
+  for (i = 0; i < 10; i++)
+    ;
+}
--- gcc/testsuite/g++.dg/gomp/pr27415.C.jj	2006-05-16 18:06:44.000000000 +0200
+++ gcc/testsuite/g++.dg/gomp/pr27415.C	2006-05-16 18:13:05.000000000 +0200
@@ -0,0 +1,50 @@
+// PR middle-end/27415
+// { dg-do compile }
+
+void
+test1 (void)
+{
+  int i = 0;
+#pragma omp parallel
+#pragma omp for firstprivate (i)		// { dg-error "should not be firstprivate" }
+  for (i = 0; i < 10; i++)
+    ;
+}
+
+void
+test2 (void)
+{
+  int i = 0;
+#pragma omp parallel for firstprivate (i)
+  for (i = 0; i < 10; i++)			// { dg-error "should not be firstprivate" }
+    ;
+}
+
+void
+test3 (void)
+{
+  int i = 0;
+#pragma omp parallel
+#pragma omp for reduction (+:i)			// { dg-error "should not be reduction" }
+  for (i = 0; i < 10; i++)
+    ;
+}
+
+void
+test4 (void)
+{
+  int i = 0;
+#pragma omp parallel for reduction (*:i)
+  for (i = 0; i < 10; i++)			// { dg-error "should not be reduction" }
+    ;
+}
+
+void
+test5 (void)
+{
+  int i = 0;
+#pragma omp parallel firstprivate (i)
+#pragma omp for
+  for (i = 0; i < 10; i++)
+    ;
+}

	Jakub