diff mbox

[gomp4.1] C++ iterators with #omp ordered depend(sink:)

Message ID 55A67270.6050103@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez July 15, 2015, 2:47 p.m. UTC 
This fixes the problem with C++ iterators not working as sink() iterator 
variables.

OK for branch?

Aldy
commit feb44bd0b32a941092441206af9157cb45995d81
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Jul 14 19:23:09 2015 -0700

    	* gimplify.c (gimplify_omp_for): Use OMP_FOR_ORIG_DECLS.
    	* tree.def (omp_for): Add new operand.
    	* tree.h (OMP_FOR_ORIG_DECLS): New macro.
    c-family/
    	* c-common.h (c_finish_omp_for): Add argument.
    	* c-omp.c (c_finish_omp_for): Set OMP_FOR_ORIG_DECLS.
    cp/
    	* semantics.c (finish_omp_for): Pass original DECLs to
    	c_finish_omp_for.
    c/
    	* c-parser.c (c_parser_omp_for_loop): Pass new argument to
    	c_finish_omp_for.

Comments

Jakub Jelinek July 15, 2015, 3:21 p.m. UTC | #1 
On Wed, Jul 15, 2015 at 07:47:12AM -0700, Aldy Hernandez wrote:
> This fixes the problem with C++ iterators not working as sink() iterator
> variables.
> 
> OK for branch?

I wonder (though not 100% sure about) if, because we really need the
OMP_FOR_ORIG_DECLS on OMP_FOR only, no other construct can be ordered,
it wouldn't make more sense to make
#define OMP_FOR_ORIG_DECLS(NODE)   TREE_OPERAND (OMP_FOR_CHECK (NODE), 6)
(note, FOR rather than LOOP) and only use 7 arguments on OMP_FOR and
not all the other loop constructs.  That would of course mean
guarding all setters and users of OMP_FOR_ORIG_DECLS (x) with
if (TREE_CODE (x) == OMP_FOR), but that is just 2 locations.

> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -7311,7 +7311,11 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
>        gcc_assert (DECL_P (decl));
>        gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (decl))
>  		  || POINTER_TYPE_P (TREE_TYPE (decl)));
> -      gimplify_omp_ctxp->iter_vars.quick_push (decl);
> +      if (OMP_FOR_ORIG_DECLS (for_stmt))
> +	gimplify_omp_ctxp->iter_vars.quick_push
> +	  (TREE_VEC_ELT (OMP_FOR_ORIG_DECLS (for_stmt), i));
> +      else
> +	gimplify_omp_ctxp->iter_vars.quick_push (decl);

No matter the decision about above, I think you want to quick_push
here twice for each decl (or have two vectors, but a single one is probably
better), and rewrite all uses of the orig decl to the new decl.
Note, gimplify_omp_for can also do similar replacement of an orig decl
with a new decl (even for OMP_FOR_ORIG_DECLS == NULL case aka C),
when the iterator var is addressable.

> +#pragma omp parallel for ordered(1)
> +  for (it = v.begin(); it < v.end(); ++it)
> +    {
> +#pragma omp ordered depend(sink:it)
> +    std::cout << *it << '\n';
> +    }
> +}

I'd try to avoid adding such blatanly wrong testcases, even just
for parsing, perhaps one day we'll want to diagnose it.
Depending on the current iteration is just wrong (like on a future one).
And with no depend(source) it will just hang.
So, can you make it depend(sink:it-1) and add
#pragma omp ordered depend(source) after it?

	Jakub
diff mbox

Patch

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 202c8f9..7e857c3 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1245,7 +1245,7 @@  extern void c_finish_omp_flush (location_t);
 extern void c_finish_omp_taskwait (location_t);
 extern void c_finish_omp_taskyield (location_t);
 extern tree c_finish_omp_for (location_t, enum tree_code, tree, tree, tree,
-			      tree, tree, tree);
+			      tree, tree, tree, tree);
 extern tree c_finish_oacc_wait (location_t, tree, tree);
 extern void c_omp_split_clauses (location_t, enum tree_code, omp_clause_mask,
 				 tree, tree *);
diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index f020a80..81aef7a 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -432,6 +432,10 @@  c_omp_for_incr_canonicalize_ptr (location_t loc, tree decl, tree incr)
 
 /* Validate and generate OMP_FOR.
    DECLV is a vector of iteration variables, for each collapsed loop.
+
+   ORIG_DECLV, if non-NULL, is a vector with the original iteration
+   variables (prior to any transformations, by say, C++ iterators).
+
    INITV, CONDV and INCRV are vectors containing initialization
    expressions, controlling predicates and increment expressions.
    BODY is the body of the loop and PRE_BODY statements that go before
@@ -439,7 +443,8 @@  c_omp_for_incr_canonicalize_ptr (location_t loc, tree decl, tree incr)
 
 tree
 c_finish_omp_for (location_t locus, enum tree_code code, tree declv,
-		  tree initv, tree condv, tree incrv, tree body, tree pre_body)
+		  tree orig_declv, tree initv, tree condv, tree incrv,
+		  tree body, tree pre_body)
 {
   location_t elocus;
   bool fail = false;
@@ -678,6 +683,7 @@  c_finish_omp_for (location_t locus, enum tree_code code, tree declv,
       OMP_FOR_INCR (t) = incrv;
       OMP_FOR_BODY (t) = body;
       OMP_FOR_PRE_BODY (t) = pre_body;
+      OMP_FOR_ORIG_DECLS (t) = orig_declv;
 
       SET_EXPR_LOCATION (t, locus);
       return add_stmt (t);
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 90486d2..0a42072 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -13696,7 +13696,7 @@  c_parser_omp_for_loop (location_t loc, c_parser *parser, enum tree_code code,
      an error from the initialization parsing.  */
   if (!fail)
     {
-      stmt = c_finish_omp_for (loc, code, declv, initv, condv,
+      stmt = c_finish_omp_for (loc, code, declv, NULL, initv, condv,
 			       incrv, body, NULL);
       if (stmt)
 	{
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 5e7a94d..5a4638d 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7333,6 +7333,7 @@  finish_omp_for (location_t locus, enum tree_code code, tree declv, tree initv,
   if (processing_template_decl)
     orig_incr = make_tree_vec (TREE_VEC_LENGTH (incrv));
 
+  tree orig_declv = copy_node (declv);
   for (i = 0; i < TREE_VEC_LENGTH (declv); )
     {
       decl = TREE_VEC_ELT (declv, i);
@@ -7430,8 +7431,8 @@  finish_omp_for (location_t locus, enum tree_code code, tree declv, tree initv,
   if (code == CILK_FOR && !processing_template_decl)
     block = push_stmt_list ();
 
-  omp_for = c_finish_omp_for (locus, code, declv, initv, condv, incrv,
-			      body, pre_body);
+  omp_for = c_finish_omp_for (locus, code, declv, orig_declv, initv, condv,
+			      incrv, body, pre_body);
 
   if (omp_for == NULL)
     {
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 9ba3f37..90df326 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -7311,7 +7311,11 @@  gimplify_omp_for (tree *expr_p, gimple_seq *pre_p)
       gcc_assert (DECL_P (decl));
       gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (decl))
 		  || POINTER_TYPE_P (TREE_TYPE (decl)));
-      gimplify_omp_ctxp->iter_vars.quick_push (decl);
+      if (OMP_FOR_ORIG_DECLS (for_stmt))
+	gimplify_omp_ctxp->iter_vars.quick_push
+	  (TREE_VEC_ELT (OMP_FOR_ORIG_DECLS (for_stmt), i));
+      else
+	gimplify_omp_ctxp->iter_vars.quick_push (decl);
 
       /* Make sure the iteration variable is private.  */
       tree c = NULL_TREE;
diff --git a/gcc/tree.def b/gcc/tree.def
index 6703b57..5b6624a 100644
--- a/gcc/tree.def
+++ b/gcc/tree.def
@@ -1087,37 +1087,42 @@  DEFTREECODE (OMP_TASK, "omp_task", tcc_statement, 2)
 	from INIT, COND, and INCR that are technically part of the
 	OMP_FOR structured block, but are evaluated before the loop
 	body begins.
+   Operand 6: OMP_FOR_ORIG_DECLS: If non-NULL, list of DECLs initialized
+	in OMP_FOR_INIT.  In some cases, like C++ iterators, the original
+	DECL init has been lost in gimplification and now contains a
+	temporary (D.nnnn).  This list contains the original DECLs in
+	the source.
 
    VAR must be an integer or pointer variable, which is implicitly thread
    private.  N1, N2 and INCR are required to be loop invariant integer
    expressions that are evaluated without any synchronization.
    The evaluation order, frequency of evaluation and side-effects are
    unspecified by the standards.  */
-DEFTREECODE (OMP_FOR, "omp_for", tcc_statement, 6)
+DEFTREECODE (OMP_FOR, "omp_for", tcc_statement, 7)
 
 /* OpenMP - #pragma omp simd [clause1 ... clauseN]
    Operands like for OMP_FOR.  */
-DEFTREECODE (OMP_SIMD, "omp_simd", tcc_statement, 6)
+DEFTREECODE (OMP_SIMD, "omp_simd", tcc_statement, 7)
 
 /* Cilk Plus - #pragma simd [clause1 ... clauseN]
    Operands like for OMP_FOR.  */
-DEFTREECODE (CILK_SIMD, "cilk_simd", tcc_statement, 6)
+DEFTREECODE (CILK_SIMD, "cilk_simd", tcc_statement, 7)
 
 /* Cilk Plus - _Cilk_for (..)
    Operands like for OMP_FOR.  */
-DEFTREECODE (CILK_FOR, "cilk_for", tcc_statement, 6)
+DEFTREECODE (CILK_FOR, "cilk_for", tcc_statement, 7)
 
 /* OpenMP - #pragma omp distribute [clause1 ... clauseN]
    Operands like for OMP_FOR.  */
-DEFTREECODE (OMP_DISTRIBUTE, "omp_distribute", tcc_statement, 6)
+DEFTREECODE (OMP_DISTRIBUTE, "omp_distribute", tcc_statement, 7)
 
 /* OpenMP - #pragma omp taskloop [clause1 ... clauseN]
    Operands like for OMP_FOR.  */
-DEFTREECODE (OMP_TASKLOOP, "omp_taskloop", tcc_statement, 6)
+DEFTREECODE (OMP_TASKLOOP, "omp_taskloop", tcc_statement, 7)
 
 /* OpenMP - #pragma acc loop [clause1 ... clauseN]
    Operands like for OMP_FOR.  */
-DEFTREECODE (OACC_LOOP, "oacc_loop", tcc_statement, 6)
+DEFTREECODE (OACC_LOOP, "oacc_loop", tcc_statement, 7)
 
 /* OpenMP - #pragma omp teams [clause1 ... clauseN]
    Operand 0: OMP_TEAMS_BODY: Teams body.
diff --git a/gcc/tree.h b/gcc/tree.h
index 7ea7a93..282bfac 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1260,6 +1260,7 @@  extern void protected_set_expr_location (tree, location_t);
 #define OMP_FOR_COND(NODE)	   TREE_OPERAND (OMP_LOOP_CHECK (NODE), 3)
 #define OMP_FOR_INCR(NODE)	   TREE_OPERAND (OMP_LOOP_CHECK (NODE), 4)
 #define OMP_FOR_PRE_BODY(NODE)	   TREE_OPERAND (OMP_LOOP_CHECK (NODE), 5)
+#define OMP_FOR_ORIG_DECLS(NODE)   TREE_OPERAND (OMP_LOOP_CHECK (NODE), 6)
 
 #define OMP_SECTIONS_BODY(NODE)    TREE_OPERAND (OMP_SECTIONS_CHECK (NODE), 0)
 #define OMP_SECTIONS_CLAUSES(NODE) TREE_OPERAND (OMP_SECTIONS_CHECK (NODE), 1)
diff --git a/libgomp/testsuite/libgomp.c++/sink-1.C b/libgomp/testsuite/libgomp.c++/sink-1.C
new file mode 100644
index 0000000..c7b3c16
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c++/sink-1.C
@@ -0,0 +1,21 @@ 
+// { dg-do compile }
+
+// Test that iterators are allowed in ordered loops.
+
+#include <iostream>
+#include <vector>
+
+int main ()
+{
+  std::vector<int> v;
+  for (int i=1; i<=5; i++) v.push_back(i);
+
+  std::vector<int>::const_iterator it;
+
+#pragma omp parallel for ordered(1)
+  for (it = v.begin(); it < v.end(); ++it)
+    {
+#pragma omp ordered depend(sink:it)
+    std::cout << *it << '\n';
+    }
+}