[C++ PATCH] Add CLEANUP_POINT_EXPRs around OMP_PARALLEL/TASK/FOR if needed (PR c++/51669)

Jakub Jelinek jakub@redhat.com
Mon Jan 2 22:10:00 GMT 2012


Hi!

The f1 function in the testcase fails newly starting with
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181332 because there
is no CLEANUP_POINT_EXPR around OMP_PARALLEL/OMP_TASK/OMP_FOR whose
IF/FINAL/NUM_THREADS/SCHEDULE clause expression might need some cleanups.
But as the testcase shows, it isn't hard to write a testcase where
even 4.6 or 4.2 ICEs too.  It isn't clear where to put the
CLEANUP_POINT_EXPR though, OpenMP 3.1 says just (e.g. for parallel):
"The if clause expression and the num_threads clause expression are
evaluated in the context outside of the parallel construct, and no ordering
of those evaluations is specified. It is also unspecified whether, in what order, or
how many times any side-effects of the evaluation of the num_threads or if clause
expressions occur."

Attached are two different patches, the first one puts the
CLEANUP_POINT_EXPR around the whole OMP_PARALLEL etc. stmt, the second
one wraps the individual clause expressions into CLEANUP_POINT_EXPR.

Both patches have been bootstrapped/regtested on x86_64-linux and
i686-linux, which one is preferrable?

	Jakub
-------------- next part --------------
2012-01-02  Jakub Jelinek  <jakub@redhat.com>

	PR c++/51669
	* tree.h (find_omp_clause): New prototype.
	* tree-flow.h (find_omp_clause): Remove prototype.
	* c-parser.c (c_parser_omp_for_loop): Call add_stmt
	if c_finish_omp_for returned non-NULL.
c-family/
	* c-omp.c (c_parser_omp_for_loop): Don't call add_stmt
	here.
cp/
	* semantics.c (finish_omp_parallel): If there is IF
	or NUM_THREADS clause, call add_stmt on maybe_cleanup_point_expr
	result.
	(finish_omp_task): If there is IF or FINAL clause, call add_stmt
	on maybe_cleanup_point_expr result.
	(finish_omp_for): If there is SCHEDULE clause, call
	maybe_cleanup_point_expr.  Call add_stmt if omp_for is non-NULL.
testsuite/
	* g++.dg/gomp/pr51669.C: New test.

--- gcc/tree.h.jj	2011-12-16 08:37:45.000000000 +0100
+++ gcc/tree.h	2012-01-02 16:59:18.037923291 +0100
@@ -5866,6 +5866,9 @@ extern unsigned HOST_WIDE_INT compute_bu
 extern unsigned HOST_WIDE_INT highest_pow2_factor (const_tree);
 extern tree build_personality_function (const char *);
 
+/* In omp-low.c.  */
+extern tree find_omp_clause (tree, enum omp_clause_code);
+
 /* In trans-mem.c.  */
 extern tree build_tm_abort_call (location_t, bool);
 extern bool is_tm_safe (const_tree);
--- gcc/tree-flow.h.jj	2011-11-29 08:58:52.000000000 +0100
+++ gcc/tree-flow.h	2012-01-02 16:57:46.538512564 +0100
@@ -392,7 +392,6 @@ extern struct omp_region *new_omp_region
 					  struct omp_region *);
 extern void free_omp_regions (void);
 void omp_expand_local (basic_block);
-extern tree find_omp_clause (tree, enum omp_clause_code);
 tree copy_var_decl (tree, tree, tree);
 
 /*---------------------------------------------------------------------------
--- gcc/c-parser.c.jj	2011-12-21 08:43:48.000000000 +0100
+++ gcc/c-parser.c	2012-01-02 17:13:17.187988709 +0100
@@ -10082,6 +10082,7 @@ c_parser_omp_for_loop (location_t loc,
       stmt = c_finish_omp_for (loc, declv, initv, condv, incrv, body, NULL);
       if (stmt)
 	{
+	  add_stmt (stmt);
 	  if (par_clauses != NULL)
 	    {
 	      tree *c;
--- gcc/c-family/c-omp.c.jj	2011-10-12 20:28:08.000000000 +0200
+++ gcc/c-family/c-omp.c	2012-01-02 17:12:54.706121214 +0100
@@ -576,7 +576,7 @@ c_finish_omp_for (location_t locus, tree
       OMP_FOR_PRE_BODY (t) = pre_body;
 
       SET_EXPR_LOCATION (t, locus);
-      return add_stmt (t);
+      return t;
     }
 }
 
--- gcc/cp/semantics.c.jj	2012-01-01 19:54:46.000000000 +0100
+++ gcc/cp/semantics.c	2012-01-02 17:16:08.640980769 +0100
@@ -4384,7 +4384,7 @@ begin_omp_parallel (void)
 tree
 finish_omp_parallel (tree clauses, tree body)
 {
-  tree stmt;
+  tree stmt, ret;
 
   body = finish_omp_structured_block (body);
 
@@ -4392,8 +4392,13 @@ finish_omp_parallel (tree clauses, tree
   TREE_TYPE (stmt) = void_type_node;
   OMP_PARALLEL_CLAUSES (stmt) = clauses;
   OMP_PARALLEL_BODY (stmt) = body;
+  ret = stmt;
+  if (find_omp_clause (clauses, OMP_CLAUSE_IF)
+      || find_omp_clause (clauses, OMP_CLAUSE_NUM_THREADS))
+    stmt = maybe_cleanup_point_expr (stmt);
+  add_stmt (stmt);
 
-  return add_stmt (stmt);
+  return ret;
 }
 
 tree
@@ -4406,7 +4411,7 @@ begin_omp_task (void)
 tree
 finish_omp_task (tree clauses, tree body)
 {
-  tree stmt;
+  tree stmt, ret;
 
   body = finish_omp_structured_block (body);
 
@@ -4414,8 +4419,13 @@ finish_omp_task (tree clauses, tree body
   TREE_TYPE (stmt) = void_type_node;
   OMP_TASK_CLAUSES (stmt) = clauses;
   OMP_TASK_BODY (stmt) = body;
+  ret = stmt;
+  if (find_omp_clause (clauses, OMP_CLAUSE_IF)
+      || find_omp_clause (clauses, OMP_CLAUSE_FINAL))
+    stmt = maybe_cleanup_point_expr (stmt);
+  add_stmt (stmt);
 
-  return add_stmt (stmt);
+  return ret;
 }
 
 /* Helper function for finish_omp_for.  Convert Ith random access iterator
@@ -4876,7 +4886,13 @@ finish_omp_for (location_t locus, tree d
 	TREE_VEC_ELT (OMP_FOR_INCR (omp_for), i) = TREE_VEC_ELT (orig_incr, i);
     }
   if (omp_for != NULL)
-    OMP_FOR_CLAUSES (omp_for) = clauses;
+    {
+      tree stmt = omp_for;
+      OMP_FOR_CLAUSES (omp_for) = clauses;
+      if (find_omp_clause (clauses, OMP_CLAUSE_SCHEDULE))
+	stmt = maybe_cleanup_point_expr (stmt);
+      add_stmt (stmt);
+    }
   return omp_for;
 }
 
--- gcc/testsuite/g++.dg/gomp/pr51669.C.jj	2012-01-02 17:18:47.268031834 +0100
+++ gcc/testsuite/g++.dg/gomp/pr51669.C	2012-01-02 17:17:05.000000000 +0100
@@ -0,0 +1,32 @@
+// PR c++/51669
+// { dg-do compile }
+// { dg-options "-fopenmp" }
+
+template <typename T> const T & min (const T &, const T &);
+
+void
+f1 ()
+{
+#pragma omp parallel num_threads (min (4, 5))
+  ;
+}
+
+struct A { A (); ~A (); };
+int foo (const A &);
+
+void
+f2 ()
+{
+  int i;
+#pragma omp parallel if (foo (A ())) num_threads (foo (A ()))
+  ;
+#pragma omp task if (foo (A ())) final (foo (A ()))
+  ;
+#pragma omp for schedule (static, foo (A ()))
+  for (i = 0; i < 10; i++)
+    ;
+#pragma omp parallel for schedule (static, foo (A ())) \
+  if (foo (A ())) num_threads (foo (A ()))
+  for (i = 0; i < 10; i++)
+    ;
+}
-------------- next part --------------
2012-01-02  Jakub Jelinek  <jakub@redhat.com>

	PR c++/51669
	* semantics.c (finish_omp_clauses): Call fold_build_cleanup_point_expr
	on OMP_CLAUSE_{IF,FINAL,NUM_THREADS,SCHEDULE_CHUNK}_EXPR.

	* g++.dg/gomp/pr51669.C: New test.

--- gcc/cp/semantics.c.jj	2012-01-02 20:39:59.000000000 +0100
+++ gcc/cp/semantics.c	2012-01-02 20:45:30.420357459 +0100
@@ -4067,6 +4067,8 @@ finish_omp_clauses (tree clauses)
 	  t = maybe_convert_cond (t);
 	  if (t == error_mark_node)
 	    remove = true;
+	  else if (!processing_template_decl)
+	    t = fold_build_cleanup_point_expr (TREE_TYPE (t), t);
 	  OMP_CLAUSE_IF_EXPR (c) = t;
 	  break;
 
@@ -4075,6 +4077,8 @@ finish_omp_clauses (tree clauses)
 	  t = maybe_convert_cond (t);
 	  if (t == error_mark_node)
 	    remove = true;
+	  else if (!processing_template_decl)
+	    t = fold_build_cleanup_point_expr (TREE_TYPE (t), t);
 	  OMP_CLAUSE_FINAL_EXPR (c) = t;
 	  break;
 
@@ -4089,7 +4093,12 @@ finish_omp_clauses (tree clauses)
 	      remove = true;
 	    }
 	  else
-	    OMP_CLAUSE_NUM_THREADS_EXPR (c) = mark_rvalue_use (t);
+	    {
+	      t = mark_rvalue_use (t);
+	      if (!processing_template_decl)
+		t = fold_build_cleanup_point_expr (TREE_TYPE (t), t);
+	      OMP_CLAUSE_NUM_THREADS_EXPR (c) = t;
+	    }
 	  break;
 
 	case OMP_CLAUSE_SCHEDULE:
@@ -4105,7 +4114,12 @@ finish_omp_clauses (tree clauses)
 	      remove = true;
 	    }
 	  else
-	    OMP_CLAUSE_SCHEDULE_CHUNK_EXPR (c) = mark_rvalue_use (t);
+	    {
+	      t = mark_rvalue_use (t);
+	      if (!processing_template_decl)
+		t = fold_build_cleanup_point_expr (TREE_TYPE (t), t);
+	      OMP_CLAUSE_SCHEDULE_CHUNK_EXPR (c) = t;
+	    }
 	  break;
 
 	case OMP_CLAUSE_NOWAIT:
--- gcc/testsuite/g++.dg/gomp/pr51669.C.jj	2012-01-02 20:41:57.320572664 +0100
+++ gcc/testsuite/g++.dg/gomp/pr51669.C	2012-01-02 20:41:57.320572664 +0100
@@ -0,0 +1,32 @@
+// PR c++/51669
+// { dg-do compile }
+// { dg-options "-fopenmp" }
+
+template <typename T> const T & min (const T &, const T &);
+
+void
+f1 ()
+{
+#pragma omp parallel num_threads (min (4, 5))
+  ;
+}
+
+struct A { A (); ~A (); };
+int foo (const A &);
+
+void
+f2 ()
+{
+  int i;
+#pragma omp parallel if (foo (A ())) num_threads (foo (A ()))
+  ;
+#pragma omp task if (foo (A ())) final (foo (A ()))
+  ;
+#pragma omp for schedule (static, foo (A ()))
+  for (i = 0; i < 10; i++)
+    ;
+#pragma omp parallel for schedule (static, foo (A ())) \
+  if (foo (A ())) num_threads (foo (A ()))
+  for (i = 0; i < 10; i++)
+    ;
+}


More information about the Gcc-patches mailing list