[Cilkplus] Patch to fix Template type inside cilk_for

Ok, fixed all the changes you mentioned. Here is the patch.

Thanks,

Balaji V. Iyer.

-----Original Message-----
From: H.J. Lu [mailto:hjl.tools@gmail.com] 
Sent: Friday, September 09, 2011 11:54 AM
To: Iyer, Balaji V
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH][Cilkplus] Patch to fix Template type inside cilk_for

On Fri, Sep 9, 2011 at 8:37 AM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:
> Here is a fixed patch with all the changes you have requested.


--
H.J.
diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk
index 8880b0a..aadf5da 100644
--- a/gcc/ChangeLog.cilk
+++ b/gcc/ChangeLog.cilk
@@ -1,7 +1,13 @@
+2011-09-09  Balaji V. Iyer  <balaji.v.iyer@intel.com>
+
+	* tree.c (walk_tree_1): Added "case CILK_FOR_STMT:".
+	* tree.h (CILK_FOR_VAR): Changed TREE_OPERAND(..., 4) to
+	TREE_OPERAND(..., 5).
+
 2011-09-08  Balaji V. Iyer  <balaji.v.iyer@intel.com>
 
-	* gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
-	statement.
+	* gimplify.c (gimplify_call_expr): Removed if 
+	(SPAWN_CALL_P (*expr)) statement.
 
 2011-09-06  Balaji V. Iyer  <balaji.v.iyer@intel.com>
 
diff --git a/gcc/cp/ChangeLog.cilk b/gcc/cp/ChangeLog.cilk
index b49f3bf..9e58fcd 100644
--- a/gcc/cp/ChangeLog.cilk
+++ b/gcc/cp/ChangeLog.cilk
@@ -1,3 +1,12 @@
+2011-09-08  Balaji V. Iyer  <balaji.v.iyer@intel.com>
+
+	* cp-tree.h (FOR_SCOPE): Changed FOR_STMT_CHECK to 
+	FOR_STMT_CHECK2
+	* cilk.c (check_incr): Added a check for variable entity name 
+	match, not just var. Removed the assert to check if operand 0 
+	is the variable.
+	(cp_extract_for_fields): Likewise.
+
 2011-09-07  Balaji V. Iyer  <balaji.v.iyer@intel.com>
 
 	* parser.c (cp_parser_jump_statement): Removed "IN_CILK_FOR | " from
diff --git a/gcc/cp/cilk.c b/gcc/cp/cilk.c
index 139ec27..49af1d7 100644
--- a/gcc/cp/cilk.c
+++ b/gcc/cp/cilk.c
@@ -1024,17 +1024,18 @@ check_incr(tree var, tree arith_type, tree incr)
   if (TREE_CODE (incr) == MODIFY_EXPR)
     {
       modify = true;
-      if (TREE_OPERAND (incr, 0) != var)
+      if (TREE_OPERAND (incr, 0) != var
+	  && DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var))
 	{
 	  error("Cilk for increment does not modify the loop variable.\n");
 	  return false;
 	}
       incr = TREE_OPERAND (incr, 1);
       incr_code = TREE_CODE (incr);
-      gcc_assert (TREE_OPERAND (incr, 0) == var);
     
     }
-  else if (TREE_OPERAND (incr, 0) != var)
+  else if (TREE_OPERAND (incr, 0) != var
+	   && DECL_NAME (TREE_OPERAND (incr, 0)) != DECL_NAME (var))
     {
       error ("Cilk for increment does not modify the loop variable.");
       return false;
@@ -2589,7 +2590,6 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree for_stmt)
     case MODIFY_EXPR:
       /* We don't get here unless the expression has the form
 	 (modify var (op var incr)) */
-      gcc_assert (TREE_OPERAND (incr, 0) == var);
       incr = TREE_OPERAND (incr, 1);
       /* again, should have checked form of increment earlier */
       if (TREE_CODE (incr) == PLUS_EXPR)
@@ -2597,9 +2597,13 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree for_stmt)
 	  tree op0 = TREE_OPERAND (incr, 0);
 	  tree op1 = TREE_OPERAND (incr, 1);
 
-	  if (op0 == var)
+	  /* if op0 is a pointer, then we should make sure the original 
+	     variable also works (e.g. if we declared as *i, then i++ is 
+	     acceptable) 
+	   */
+	  if (op0 == var || DECL_NAME (op0) == DECL_NAME (var))
 	    incr = op1;
-	  else if (op1 == var)
+	  else if (op1 == var || DECL_NAME (op1) == DECL_NAME (var))
 	    incr = op0;
 	  else
 	    gcc_unreachable ();
@@ -2637,8 +2641,17 @@ cp_extract_for_fields (struct cilk_for_desc *cfd, tree for_stmt)
 	  tree op0 = TREE_OPERAND (incr, 0);
 	  tree op1 = TREE_OPERAND (incr, 1);
 
-	  gcc_assert (op0 == var);
-	  incr = op1;
+	  /* if op0 is a pointer, then we should make sure the original 
+	     variable also works (e.g. if we declared as *i, then i++ is 
+	     acceptable) 
+	   */
+	  if (op0 == var || DECL_NAME (op0) == DECL_NAME (var))
+	    incr = op1;
+	  else if (op1 == var || DECL_NAME (op1) == DECL_NAME (var))
+	    incr = op0;
+	  else
+	    gcc_unreachable ();
+
 	  /* Store the amount to be subtracted.
 	     Negating it could overflow. */
 	  negate_incr = true;
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f33b7f4..a924b73 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3874,7 +3874,7 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
    condition, update expression, and body of the for statement,
    respectively.  */
 /* bviyer: we need it in C, so I have defined them in tree.h */
-#define FOR_SCOPE(NODE)		TREE_OPERAND (FOR_STMT_CHECK (NODE), 4)
+#define FOR_SCOPE(NODE)		TREE_OPERAND (FOR_STMT_CHECK2 (NODE), 4)
 #define FOR_STMT_PRAGMA_SIMD_INDEX(NODE)               \
  (FOR_STMT_CHECK(NODE)->base.pragma_simd_index)
 
diff --git a/gcc/testsuite/ChangeLog.cilk b/gcc/testsuite/ChangeLog.cilk
index 50da2a5..5301fff 100644
--- a/gcc/testsuite/ChangeLog.cilk
+++ b/gcc/testsuite/ChangeLog.cilk
@@ -1,3 +1,7 @@
+2011-09-09  Balaji V. Iyer  <balaji.v.iyer@intel.com>
+
+	* g++.dg/cilk-plus/template_cilk_for_plus_equal.cpp: New.
+
 2011-09-08  Balaji V. Iyer  <balaji.v.iyer@intel.com>
 
 	* gcc.dg/cilk-plus/label_test.c: New.
diff --git a/gcc/testsuite/g++.dg/cilk-plus/template_cilk_for_plus_equal.cpp b/gcc/testsuite/g++.dg/cilk-plus/template_cilk_for_plus_equal.cpp
new file mode 100644
index 0000000..8f22f5b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cilk-plus/template_cilk_for_plus_equal.cpp
@@ -0,0 +1,36 @@
+#include <iostream>
+#include <cilk/cilk.h>
+#include <cstdlib>
+
+template <typename T>
+void some_func(char *number)
+{
+  /* this shouldn't output an error */
+  cilk_for (T i = 0; i < atoi (number); i += 1)
+    std::cout << "Test += " << std::endl;
+
+  cilk_for (T j = atoi(number); j > 0 ; j -= 1)
+    std::cout << "Test -=" << std::endl;
+
+  cilk_for (T k = 0; k < atoi (number); k++)
+    std::cout << "Test ++" << std::endl;
+
+  cilk_for (T kk = atoi (number); kk > 0; kk--) 
+    std::cout << "Test --" << std::endl;
+
+  std::cout << std::endl;
+  return;
+}
+
+int main(int argc, char **argv)
+{
+  if (argc == 1)
+    return -1;
+
+  some_func<int>(argv[1]);
+  some_func<char>(argv[1]);
+  some_func<long>(argv[1]);
+  some_func<unsigned char>(argv[1]);
+  return 0;
+}
+
diff --git a/gcc/tree.c b/gcc/tree.c
index ac903e2..5cd21d2 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -10606,6 +10606,17 @@ walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
 	WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len));
       }
 
+    case CILK_FOR_STMT:
+      {
+	WALK_SUBTREE (CILK_FOR_INIT (*tp));
+	WALK_SUBTREE (FOR_COND (*tp));
+	WALK_SUBTREE (FOR_EXPR (*tp));
+	WALK_SUBTREE (FOR_BODY (*tp));
+	WALK_SUBTREE (CILK_FOR_GRAIN (*tp));
+	WALK_SUBTREE (CILK_FOR_VAR (*tp));
+      }
+      break;
+
     case DECL_EXPR:
       /* If this is a TYPE_DECL, walk into the fields of the type that it's
 	 defining.  We only want to walk into these fields of a type in this
diff --git a/gcc/tree.h b/gcc/tree.h
index 3a889ea..fec4164 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -6076,7 +6076,7 @@ extern bool block_may_fallthru (const_tree);
 #define FOR_BODY(NODE)		TREE_OPERAND (FOR_STMT_CHECK2 (NODE), 3)
 
 /* Some cilk #defines */
-#define CILK_FOR_VAR(NODE)      TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 4)
+#define CILK_FOR_VAR(NODE)      TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 5)
 #define CILK_FOR_INIT(NODE)     TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 0)
 #define CILK_FOR_GRAIN(NODE)    TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 6)

On Fri, Sep 09, 2011 at 11:56:29AM -0700, Iyer, Balaji V wrote:
> diff --git a/gcc/ChangeLog.cilk b/gcc/ChangeLog.cilk index 8880b0a..299febb 100644
> --- a/gcc/ChangeLog.cilk
> +++ b/gcc/ChangeLog.cilk
> @@ -2,6 +2,9 @@
> 
>  	* gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
>  	statement.
> +	* tree.c (walk_tree_1): Added "case CILK_FOR_STMT:".
> +	* tree.h (CILK_FOR_VAR): Changed TREE_OPERAND(..., 4) to
> +	TREE_OPERAND(..., 5).

The above is still not correct ChangeLog, you are quoting the actual code
way too much.   E.g. the second could be:
	* tree.c (walk_tree_1): Handle CILK_FOR_STMT.
third maybe:
	* tree.c (CILK_FOR_VAR): Use 5 instead of 4 as last TREE_OPERAND
	argument.
The first one should describe what kind of code you've actually removed,
Don't handle this or that.
etc.

> +2011-09-08  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +	* cp-tree.h (FOR_SCOPE): Changed FOR_STMT_CHECK to FOR_STMT_CHECK2

All ChangeLog entries end with a dot.

> +	* cilk.c (check_incr): Added a check for variable entity name match, not just
> +	var. Removed the assert to check if operand 0 is the variable.

Two spaces after . in between sentences.

	Jakub

On Fri, Sep 9, 2011 at 11:56 AM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:
> Ok, fixed all the changes you mentioned. Here is the patch.
>
> Thanks,
>

Please provide a patch against the current branch since your patch
won't apply.

-	* gimplify.c (gimplify_call_expr): Removed if (SPAWN_CALL_P (*expr))
-	statement.
+	* gimplify.c (gimplify_call_expr): Removed if
+	(SPAWN_CALL_P (*expr)) statement.

Please use a separate patch to change existing ChangeLog entries.

+    case CILK_FOR_STMT:
+      {
+	WALK_SUBTREE (CILK_FOR_INIT (*tp));
+	WALK_SUBTREE (FOR_COND (*tp));
+	WALK_SUBTREE (FOR_EXPR (*tp));
+	WALK_SUBTREE (FOR_BODY (*tp));
+	WALK_SUBTREE (CILK_FOR_GRAIN (*tp));
+	WALK_SUBTREE (CILK_FOR_VAR (*tp));
+      }
+      break;
+

Please remove extra {}.