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

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
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.
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++)
+    ;
+}

On 01/03/2012 09:10 AM, Jakub Jelinek wrote:
> 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?

I don't have a real preference.  I merely note that the second patch
is smaller and less complex, which would tend to lean me that direction.

Jason?


r~

On 01/02/2012 05:49 PM, Richard Henderson wrote:
> On 01/03/2012 09:10 AM, Jakub Jelinek wrote:
>> 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?
>
> I don't have a real preference.  I merely note that the second patch
> is smaller and less complex, which would tend to lean me that direction.

The second patch is also more correct, let's go with that one.

Jason