On 11/18/2013 04:50 PM, Iyer, Balaji V wrote:
+  int flags = LOOKUP_PROTECT | LOOKUP_ONLYCONVERTING;

Why not LOOKUP_NORMAL? LOOKUP_ONLYCONVERTING isn't relevant in this context.

+  tree exp = build_new_op (EXPR_LOCATION (op1), code, flags, op0, op1,
+                          NULL_TREE, NULL, 0);

Use tf_none instead of 0.

+  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;

This doesn't make sense to me. build_x_modify_expr takes codes like PLUS_EXPR and then does an assignment afterward; we don't want to quietly do += just because there's some error with the evaluation of the + operation. What is this code trying to do?

+/* 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.  Returns an expression (or a
+   STATEMENT_LIST of expressions).  If it is unable to find the appropriate
+   iteration, then it returns an error mark node and its parent will set
+   the loop as invalid.  */

This doesn't explain what VAR2 is. And it seems like you're also using LOW as the increment?

+      tree new_stmt = build_x_modify_expr (loc, new_var, INIT_EXPR,
+                                          build_zero_cst (TREE_TYPE (new_var)),
+                                          tf_warning_or_error);
+      if (new_stmt == error_mark_node)
+       return error_mark_node;
+      append_to_statement_list (new_stmt, &exp);
+      new_stmt = build_x_modify_expr (loc, new_var, NOP_EXPR, low,
+                                     tf_warning_or_error);

Why assign 0 if you're going to immediately assign low afterwards?

+  /* We have to manually create this loop for two reasons:
+     a. We need to have access to continue and start label since we need
+        to resolve continue and breaks by hand.

Why do you need to resolve them by hand? It looks like break isn't even allowed.

+     b. C++ doesn't provide a c_finish_loop function like C does.  */

Why is that important?

   sk_for,           /* The scope of the variable declared in a
                        for-init-statement.  */
+  sk_cilk_for,       /* The scope of the variable declared in _Cilk_for init
+                       statement.  */

How is this different from a normal for-init-statement? Nothing seems to use it.

Jason

Reply via email to