On 11/25/2013 11:03 PM, Iyer, Balaji V wrote:
On a broad note, I think there's a lot of OpenMP code you could be
reusing here rather than writing it all again. And that way Cilk code
will benefit from improvements to OpenMP handling, and vice versa. It
probably makes sense to turn Cilk_for into an OMP_FOR loop, and then
gimplify into GIMPLE_OMP_FOR, rather than create a new tree code and
handle everything at the tree level. But I don't know the OMP code well
enough to suggest exactly how that would work.
Finer-grained comments:
+ tree exp = build_new_op (EXPR_LOCATION (op1), code, flags, op0, op1,
+ NULL_TREE, NULL, tf_none);
+ if (exp == error_mark_node)
+ exp = build_x_modify_expr (EXPR_LOCATION (op1), op0, code, op1, tf_none);
+ if (exp && exp != error_mark_node)
+ return exp;
I thought you were changing this?
+/* Handler for iterator to compute the loop variable. ADD_OP indicates
+ whether we need a '+' or '-' operation. LOW indicates the starting point
+ and LOOP_VAR is the induction variable. This functin returns an
+ INIT_EXPR. */
This comment still doesn't document VAR2.
"function"
+ tree exp = build_new_op (loc, add_op, 0, low, loop_var, NULL_TREE, 0,
+ tf_none);
+ gcc_assert (exp != error_mark_node);
+ exp = cp_build_modify_expr (var2, INIT_EXPR, exp, tf_warning_or_error);
Looking at online Cilk documentation I see:
The increment expression must add to or subtract from the control variable
using one of the following supported operations:
+=
-=
++ (prefix or postfix)
-- (prefix or postfix)
From this, I think people would expect the increment to use a
user-defined operator+=/-=/++/--, but your code above uses operator+/-
instead.
+ "-fcilkplus must be enabled t use %<_Cilk_for%>");
"to"
+cp_parser_cilk_for (cp_parser *parser, tree grain)
Please reuse cp_parser_omp_for, like Aldy did for #pragma simd
(cp_parser_cilk_simd) rather than write yet another for-statement
parser. This should reduce the patch size quite a bit.
+ case PRAGMA_CILK_GRAINSIZE:
+ if (context == pragma_external)
+ {
+ error_at (pragma_tok->location,
+ "%<#pragma cilk grainsize%> may only be be used inside a "
+ "function");
+ break;
+ }
+
+ /* Ignore the pragma if Cilk Plus is not enabled. */
+ if (flag_enable_cilkplus)
+ {
+ cp_parser_cilk_grainsize (parser, pragma_tok);
+ return true;
+ }
default:
Do you mean to fall through to the default case if Cilk+ is not enabled?
+ tmp = CILK_FOR_COND (t);
+ if (COMPARISON_CLASS_P (tmp))
+ {
+ tree op0 = RECUR (TREE_OPERAND (tmp, 0));
+ tree op1 = RECUR (TREE_OPERAND (tmp, 1));
+ tmp = build2 (TREE_CODE (tmp), boolean_type_node, op0, op1);
+ }
+ CILK_FOR_COND (stmt) = tmp;
Why not just recur into CILK_FOR_COND?
+ tmp = CILK_FOR_EXPR (t);
+ if (TREE_CODE (tmp) == MODIFY_EXPR)
+ {
+ tree lhs = TREE_OPERAND (tmp, 0);
+ tree rhs = TREE_OPERAND (tmp, 1);
+ lhs = RECUR (lhs);
+ rhs = build2 (TREE_CODE (rhs), TREE_TYPE (lhs),
+ RECUR (TREE_OPERAND (rhs, 0)),
+ RECUR (TREE_OPERAND (rhs, 1)));
+ tmp = build2 (MODIFY_EXPR, void_type_node, lhs, rhs);
+ }
+ else
+ tmp = build2 (TREE_CODE (tmp), void_type_node,
+ RECUR (TREE_OPERAND (tmp, 0)),
+ RECUR (TREE_OPERAND (tmp, 1)));
+ finish_for_expr (tmp, stmt);
And CILK_FOR_EXPR?
Jason
