[PATCH] Cilk Plus Array Notation for C++
Iyer, Balaji V
balaji.v.iyer@intel.com
Mon Jun 17 16:49:00 GMT 2013
Aldy,
Please see my responses below. Here is a fixed patch and the ChangeLog entries:
gcc/cp/ChangeLog
2013-06-17 Balaji V. Iyer <balaji.v.iyer@intel.com>
* call.c (convert_like_real): Added a check if array notation is present
in expression. If so, then no conversion of arguments is necessary.
(build_over_call): Likewise.
* typeck.c (cp_build_function_call_vec): Likewise.
(convert_for_assignment): Likewise.
(cp_build_array_ref): Reject array notations with a rank greater than 1
as an array's index.
(cp_build_binary_op): If array notations are preent in op, then call
find_correct_array_notation_type.
(cp_build_addr_expr_1): Handle ARRAY_NOTATION_REF similar to ARRAY_REF.
* cp-array-notation.c: New file.
* cp-objcp-common.c (cp_common_init_ts): Marked ARRAY_NOTATION_REF tree
as typed.
* cp-tree.h (fix_array_notation_exprs): New prototype.
* semantics.c (finish_return_stmt): Reject array notations as
return value.
(cxx_eval_constant_expression): Added ARRAY_NOTATION_REF case.
(potential_constant_expression_1): Likewise.
* tree.c (lvalue_kind): Likewise.
* error.c (dump_decl): Likewise.
(dump_expr): Likewise.
* pt.c (ARRAY_NOTATION_REF): Likewise.
(type_unification_real): Do not unify any arguments if array notations
are found in arg.
(instantiate_decl): Added a check for array notaitons inside the
function body. If so, then expand them.
* parser.c (cp_parser_array_notation): New function.
(cp_parser_postfix_open_square_expression): Added a check for colons
inside square braces. If found, then handle the array access as an
array notation access. Also, disable auto-correction from a single
colon to scope when Cilk Plus is enabled.
(cp_parser_compound_statement): Added a check for array notations
inside the statement. If found, then expand them.
(cp_parser_ctor_initializer_opt_and_function_body): Likewise.
(cp_parser_function_definition_after_declarator): Likewise.
(cp_parser_selection_statement): Searched for array notations inside
condition. If so, then emit an error.
(cp_parser_iteration_statement): Likewise.
(cp_parser_direct_declarator): Reject array notations inside a
variable or array declaration.
* Make-lang.in (CXX_AND_OBJCXX_OBJS): Added cp/cp-array-notation.o.
gcc/testsuite/ChangeLog
2013-06-17 Balaji V. Iyer <balaji.v.iyer@intel.com>
* c-c++-common/cilk-plus/AN/if_test_errors.c (main): Made certain
errors specific to C, if necessary. Also added new error hooks for C++.
* c-c++-common/cilk-plus/AN/misc.c (main): Likewise.
* c-c++-common/cilk-plus/AN/parser_errors.c (main): Likewise.
* c-c++-common/cilk-plus/AN/parser_errors2.c (main): Likewise.
* c-c++-common/cilk-plus/AN/parser_errors3.c (main): Likewise.
* c-c++-common/cilk-plus/AN/pr57541.c (main): Likewise.
* c-c++-common/cilk-plus/AN/parser_errors4.c (main): In addition to the
same changes as parser_errors3.c, spaces were added between colons to
not confuse C++ parser with 2 colons as scope.
* c-c++-common/cilk-plus/AN/vla.c: Make this test C specific.
* g++.dg/cilk-plus/AN/array_test1_tplt.cc: New test.
* g++.dg/cilk-plus/AN/array_test2_tplt.cc: Likewise.
* g++.dg/cilk-plus/AN/array_test_ND_tplt.cc: Likewise.
* g++.dg/cilk-plus/AN/braced_list.cc: Likewise.
* g++.dg/cilk-plus/AN/builtin_fn_custom_tplt.cc: Likewise.
* g++.dg/cilk-plus/AN/builtin_fn_mutating_tplt.cc: Likewise.
* g++.dg/cilk-plus/AN/fp_triplet_values_tplt.c: Likewise.
* g++.dg/cilk-plus/AN/preincr_test.cc: Likewise.
* g++.dg/cilk-plus/AN/postincr_test.cc: Likewise.
* g++.dg/cilk-plus/cilk-plus.exp: New script.
* gcc/testsuite/g++.dg/dg.exp: Included Cilk Plus C++ tests in the list.
Thanks,
Balaji V. Iyer.
> -----Original Message-----
> From: Aldy Hernandez [mailto:aldyh@redhat.com]
> Sent: Thursday, June 13, 2013 12:11 PM
> To: Iyer, Balaji V
> Cc: gcc-patches@gcc.gnu.org; Jason Merrill (jason@redhat.com);
> rth@redhat.com
> Subject: Re: [PATCH] Cilk Plus Array Notation for C++
>
>
> >> It looks like a NULL in INIT_INDEX is a specially handled case.
> >> Perhaps you should document that INIT_INDEX can be null and what it
> means.
> >> Also, you don't need to document what internal variable name you are
> >> using as a return value (VALUE_TREE). Perhaps instead of "The return
> >> value..." you could write "This function returns the
> >> ARRAY_NOTATION_REF node." or something like it.
> >
> > It is documented inside the function, right before checking for !init_index. Is
> that enough?
>
> Please put it in the function comment. As it stands, users would have to look
> through the body to find that init_index==NULL is a special case.
DONE!
>
>
> >> Changes to existing tests should be submitted as a separate patch, since this
> >> doesn't seem to be C++ specific. And BTW, this particular test change can be
> >> committed as obvious.
> >
> > OK will do. What about adding {target c} and {target c++} for test cases. Can
> that be submitted with the patch?
>
> Yes.
>
> > + else if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MAX)
> > + {
> > + /* If the TYPE_MIN_VALUE is available for the new_var_type, then
> > + set that as the initial value. */
> > + if (TYPE_MIN_VALUE (new_var_type))
> > + new_var_init = build_x_modify_expr (location, *new_var, NOP_EXPR,
> > + TYPE_MIN_VALUE (new_var_type),
> 1);
> > + else
> > + /* ... otherwise set initial value as the first element of array. */
> > + new_var_init = build_x_modify_expr (location, *new_var, NOP_EXPR,
> > + func_parm, 1);
> > + new_no_expr = build_x_modify_expr (location, *new_var, NOP_EXPR,
> > + *new_var, 1);
> > + new_yes_expr = build_x_modify_expr (location, *new_var, NOP_EXPR,
> > + func_parm, 1);
> > + new_cond_expr = build_x_binary_op (location, LT_EXPR, *new_var,
> > + TREE_CODE (*new_var), func_parm,
> > + TREE_CODE (func_parm), NULL,
> > + tf_warning_or_error);
> > + new_expr = build_x_conditional_expr (location, new_cond_expr,
> > + new_yes_expr, new_no_expr,
> > + tf_warning_or_error);
> > + }
> > + else if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN)
> > + {
> > + /* If the TYPE_MAX_VALUE is available for the new_var_type, then
> > + set that as the initial value. */
> > + if (TYPE_MAX_VALUE (new_var_type))
> > + new_var_init = build_x_modify_expr (location, *new_var, NOP_EXPR,
> > + TYPE_MAX_VALUE (new_var_type),
> 1);
> > + else
> > + /* ... otherwise set initial value as the first element of array. */
> > + new_var_init = build_x_modify_expr (location, *new_var, NOP_EXPR,
> > + func_parm, 1);
> > + new_no_expr = build_x_modify_expr (location, *new_var, NOP_EXPR,
> > + *new_var, 1);
> > + new_yes_expr = build_x_modify_expr (location, *new_var, NOP_EXPR,
> > + func_parm, 1);
> > + new_cond_expr = build_x_binary_op (location, GT_EXPR, *new_var,
> > + TREE_CODE (*new_var), func_parm,
> > + TREE_CODE (func_parm), NULL,
> > + tf_warning_or_error);
> > + new_expr = build_x_conditional_expr (location, new_cond_expr,
> > + new_yes_expr, new_no_expr,
> > + tf_warning_or_error);
> > + }
>
> The whole slew of these cases have a lot of duplicated code. For
> instance, BUILT_IN_CILKPLUS_SEC_REDUCE_MIN is the same as
> BUILT_IN_CILKPLUS_SEC_REDUCE_MAX, the only difference being GT_EXPR vs
> LT_EXPR. Surely you could do something like:
>
> if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN
> || an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MAX) {
> enum tree_code code;
> if (an_type == BUILT_IN_CILKPLUS_SEC_REDUCE_MIN)
> code = GT_EXPR;
> else
> code = LT_EXPR;
> // stuff
> }
>
> The compiler should be able to optimize the above, but even if it
> couldn't, I am willing to compare twice and save lines and lines of code.
DONE! The line size went from 261 to ~138
>
> Similarly for SEC_REDUCE_ANY_ZERO/SEC_REDUCE_ANY_NONZERO,
> SEC_REDUCE_ALL_ZERO/SEC_REDUCE_ALL_NONZERO,
> SEC_REDUCE_ADD/SEC_REDUCE_MUL, etc etc etc.
>
> > + if (location == UNKNOWN_LOCATION)
> > + {
> > + if (EXPR_LOCATION (lhs) != UNKNOWN_LOCATION)
> > + location = EXPR_LOCATION (lhs);
> > + else if (EXPR_LOCATION (rhs) != UNKNOWN_LOCATION)
> > + location = EXPR_LOCATION (rhs);
> > + }
> > +
> > +
> > + /* We need this when we have a scatter issue. */
>
> Extra whitespace.
>
> > + if (lhs_rank != 0 && rhs_rank != 0 && lhs_rank != rhs_rank)
> > + {
> > + tree lhs_base = lhs;
> > + tree rhs_base = rhs;
> > +
> > + for (ii = 0; ii < lhs_rank; ii++)
> > + lhs_base = ARRAY_NOTATION_ARRAY (lhs_base);
> > +
> > + while (rhs_base && TREE_CODE (rhs_base) != ARRAY_NOTATION_REF)
> > + rhs_base = TREE_OPERAND (rhs_base, 0);
> > + for (ii = 0; ii < rhs_rank; ii++)
> > + rhs_base = ARRAY_NOTATION_ARRAY (rhs_base);
> > +
> > + if (location == UNKNOWN_LOCATION && EXPR_HAS_LOCATION (lhs))
> > + location = EXPR_LOCATION (lhs);
> > + error_at (location, "rank mismatch between %qD and %qD", lhs_base,
> > + rhs_base);
> > + return error_mark_node;
> > + }
>
> This whole block seems to be indented incorrectly.
>
> > + /* Assign the array notation components to variable so that they can satisfy
> > + the exec-once rule. */
> > + for (ii = 0; ii < lhs_list_size; ii++)
> > + {
> > + tree array_node = (*lhs_list)[ii];
> > + tree array_begin = ARRAY_NOTATION_START (array_node);
> > + tree array_lngth = ARRAY_NOTATION_LENGTH (array_node);
> > + tree array_strde = ARRAY_NOTATION_STRIDE (array_node);
> > +
> > + if (TREE_CODE (array_begin) != INTEGER_CST)
> > + {
> > + lhs_begin_var = build_decl (location, VAR_DECL, NULL_TREE,
> > + integer_type_node);
> > + finish_expr_stmt (build_x_modify_expr (location, lhs_begin_var,
> > + NOP_EXPR, array_begin,
> > + complain));
> > + ARRAY_NOTATION_START (array_node) = lhs_begin_var;
> > + }
> > + if (TREE_CODE (array_lngth) != INTEGER_CST)
> > + {
> > + lhs_lngth_var = build_decl (location, VAR_DECL, NULL_TREE,
> > + integer_type_node);
> > + finish_expr_stmt (build_x_modify_expr (location, lhs_lngth_var,
> > + NOP_EXPR, array_lngth,
> > + complain));
> > + ARRAY_NOTATION_LENGTH (array_node) = lhs_lngth_var;
> > + }
>
> All these exec-one wrappers seem to be doing the same thing over and
> over. Can you abstract this sequence into a separate helper function?
> Perhaps something like:
>
> for (ii = 0; ii < lhs_list_size; ii++)
> {
> exec_once (&ARRAY_NOTATION_START (array_node));
> exec_once (&ARRAY_NOTATION_LENGTH (array_node));
> exec_once (&ARRAY_NOTATION_STRIDE (array_node));
> ...
> ...
> }
>
> or something to that effect.
DONE!
>
> > + if (lhs_rank)
> > + {
> > + for (ii = 0; ii < lhs_list_size; ii++)
> > + {
> > + jj = 0;
> > + ii_tree = (*lhs_list)[ii];
> > + while (ii_tree)
> > + {
> > + if (TREE_CODE (ii_tree) == ARRAY_NOTATION_REF)
> > + {
> > + lhs_array[ii][jj] = ii_tree;
> > + jj++;
> > + ii_tree = ARRAY_NOTATION_ARRAY (ii_tree);
> > + }
> > + else if (TREE_CODE (ii_tree) == ARRAY_REF)
> > + ii_tree = TREE_OPERAND (ii_tree, 0);
> > + else if (TREE_CODE (ii_tree) == VAR_DECL
> > + || TREE_CODE (ii_tree) == PARM_DECL)
> > + break;
> > + }
> > + }
> > + }
> > + else
> > + lhs_array[0][0] = NULL_TREE;
> > +
> > + if (rhs_rank)
> > + {
> > + for (ii = 0; ii < rhs_list_size; ii++)
> > + {
> > + jj = 0;
> > + ii_tree = (*rhs_list)[ii];
> > + while (ii_tree)
> > + {
> > + if (TREE_CODE (ii_tree) == ARRAY_NOTATION_REF)
> > + {
> > + rhs_array[ii][jj] = ii_tree;
> > + jj++;
> > + ii_tree = ARRAY_NOTATION_ARRAY (ii_tree);
> > + }
> > + else if (TREE_CODE (ii_tree) == ARRAY_REF)
> > + ii_tree = TREE_OPERAND (ii_tree, 0);
> > + else if (TREE_CODE (ii_tree) == VAR_DECL
> > + || TREE_CODE (ii_tree) == PARM_DECL
> > + || TREE_CODE (ii_tree) == CALL_EXPR)
> > + break;
> > + }
> > + }
> > + }
>
> Can't you abstract the common idiom into some inline function?
>
> > + for (ii = 0; ii < lhs_list_size; ii++)
> > + {
> > + if (TREE_CODE ((*lhs_list)[ii]) == ARRAY_NOTATION_REF)
> > + {
> > + for (jj = 0; jj < lhs_rank; jj++)
> > + {
> > + if (TREE_CODE (lhs_array[ii][jj]) == ARRAY_NOTATION_REF)
> > + {
> > + lhs_value[ii][jj] = ARRAY_NOTATION_ARRAY (lhs_array[ii][jj]);
> > + lhs_start[ii][jj] = ARRAY_NOTATION_START (lhs_array[ii][jj]);
> > + lhs_length[ii][jj] =
> > + fold_build1 (CONVERT_EXPR, integer_type_node,
> > + ARRAY_NOTATION_LENGTH (lhs_array[ii][jj]));
> > + lhs_stride[ii][jj] =
> > + fold_build1 (CONVERT_EXPR, integer_type_node,
> > + ARRAY_NOTATION_STRIDE (lhs_array[ii][jj]));
> > + lhs_vector[ii][jj] = true;
> > +
> > + /* If the stride value is variable (i.e. not constant) then
> > + assume that the length is positive. */
> > + if (!TREE_CONSTANT (lhs_length[ii][jj]))
> > + lhs_count_down[ii][jj] = false;
> > + else if (tree_int_cst_lt
> > + (lhs_length[ii][jj],
> > + build_zero_cst (TREE_TYPE (lhs_length[ii][jj]))))
> > + lhs_count_down[ii][jj] = true;
> > + else
> > + lhs_count_down[ii][jj] = false;
> > + }
> > + else
> > + lhs_vector[ii][jj] = false;
> > + }
> > + }
> > + }
> > + for (ii = 0; ii < rhs_list_size; ii++)
> > + {
> > + if (TREE_CODE ((*rhs_list)[ii]) == ARRAY_NOTATION_REF)
> > + {
> > + for (jj = 0; jj < rhs_rank; jj++)
> > + {
> > + if (TREE_CODE (rhs_array[ii][jj]) == ARRAY_NOTATION_REF)
> > + {
> > + rhs_value[ii][jj] = ARRAY_NOTATION_ARRAY (rhs_array[ii][jj]);
> > + rhs_start[ii][jj] = ARRAY_NOTATION_START (rhs_array[ii][jj]);
> > + rhs_length[ii][jj] =
> > + ARRAY_NOTATION_LENGTH (rhs_array[ii][jj]);
> > + rhs_stride[ii][jj] =
> > + ARRAY_NOTATION_STRIDE (rhs_array[ii][jj]);
> > + rhs_vector[ii][jj] = true;
> > + /* If the stride value is variable (i.e. not constant) then
> > + assume that the length is positive. */
> > + if (!TREE_CONSTANT (rhs_length[ii][jj]))
> > + rhs_count_down[ii][jj] = false;
> > + else if (tree_int_cst_lt
> > + (rhs_length[ii][jj],
> > + build_zero_cst (TREE_TYPE (rhs_length[ii][jj]))))
> > + rhs_count_down[ii][jj] = true;
> > + else
> > + rhs_count_down[ii][jj] = false;
> > + }
> > + else
> > + rhs_vector[ii][jj] = false;
> > + }
>
> Similarly here, though I don't know if the CONVERT_EXPR in the first
> case and not in the second case is an oversight.
>
> > + if (lhs_rank)
> > + {
> > + for (ii = 0; ii < lhs_list_size; ii++)
> > + {
> > + if (lhs_vector[ii][0])
> > + {
> > + /* The last ARRAY_NOTATION element's ARRAY component should
> be
> > + the array's base value. */
> > + tree lhs_array_opr = lhs_value[ii][lhs_rank - 1];
> > + for (s_jj = lhs_rank - 1; s_jj >= 0; s_jj--)
> > + {
> > + tree stride = NULL_TREE, var = NULL_TREE, start =
> NULL_TREE;
> > + if ((TREE_TYPE (lhs_start[ii][s_jj]) ==
> > + TREE_TYPE (lhs_stride[ii][s_jj]))
> > + && (TREE_TYPE (lhs_stride[ii][s_jj]) !=
> > + TREE_TYPE (lhs_var[s_jj])))
> > + {
> > + /* If stride and start are of same type and the induction
> > + var is not, convert induction variable to stride's
> > + type. */
> > + start = lhs_start[ii][s_jj];
> > + stride = lhs_stride[ii][s_jj];
> > + var = build_c_cast (location,
> > + TREE_TYPE (lhs_stride[ii][s_jj]),
> > + lhs_var[s_jj]);
> > + }
> > + else if (TREE_TYPE (lhs_start[ii][s_jj]) !=
> > + TREE_TYPE (lhs_stride[ii][s_jj]))
> > + {
> > + /* If we reach here, then the stride and start are of
> > + different types, and so it doesn't really matter what
> > + the induction variable type is, convert everything to
> > + integer. The reason why we pick an integer instead of
> > + something like size_t is because the stride and
> > + length can be + or -. */
> > + start = build_c_cast (location, integer_type_node,
> > + lhs_start[ii][s_jj]);
> > + stride = build_c_cast (location, integer_type_node,
> > + lhs_stride[ii][s_jj]);
> > + var = build_c_cast (location, integer_type_node,
> > + lhs_var[s_jj]);
> > + }
> > + else
> > + {
> > + start = lhs_start[ii][s_jj];
> > + stride = lhs_stride[ii][s_jj];
> > + var = lhs_var[s_jj];
> > + }
> > + if (lhs_count_down[ii][s_jj])
> > + /* Array[start_index - (induction_var * stride)]. */
> > + lhs_array_opr = grok_array_decl
> > + (location, lhs_array_opr,
> > + build2 (MINUS_EXPR, TREE_TYPE (var), start,
> > + build2 (MULT_EXPR, TREE_TYPE (var), var,
> > + stride)), false);
> > + else
> > + /* Array[start_index + (induction_var * stride)]. */
> > + lhs_array_opr = grok_array_decl
> > + (location, lhs_array_opr,
> > + build2 (PLUS_EXPR, TREE_TYPE (var), start,
> > + build2 (MULT_EXPR, TREE_TYPE (var), var,
> > + stride)), false);
> > + }
> > + vec_safe_push (lhs_array_operand, lhs_array_opr);
> > + }
> > + else
> > + vec_safe_push (lhs_array_operand, integer_one_node);
> > + }
>
> 99% of this looks exactly like the rhs_rank case, surely you can
> abstract most of this into a separate function.
>
> > + for (ii = 0; ii < rhs_list_size; ii++)
> > + {
> > + tree rhs_node = (*rhs_list)[ii];
> > + if (TREE_CODE (rhs_node) == CALL_EXPR)
> > + {
> > + int idx_value = 0;
> > + tree func_name = CALL_EXPR_FN (rhs_node);
> > + if (TREE_CODE (func_name) == ADDR_EXPR)
> > + if (is_sec_implicit_index_fn (func_name))
> > + {
> > + idx_value =
> > + extract_sec_implicit_index_arg (location, rhs_node);
> > + if (idx_value < (int) lhs_rank && idx_value >= 0)
> > + vec_safe_push (rhs_array_operand, rhs_var[idx_value]);
> > + else
> > + {
> > + size_t ee = 0;
> > + tree rhs_base = (*lhs_list)[ii];
> > + for (ee = 0; ee < rhs_rank; ee++)
> > + if (rhs_base
> > + && TREE_CODE (rhs_base) ==
> ARRAY_NOTATION_REF)
> > + rhs_base = ARRAY_NOTATION_ARRAY (rhs_base);
> > +
> > + error_at (location, "__sec_implicit_index argument %d
> "
> > + "must be less than rank of %qD", idx_value,
> > + rhs_base);
> > + return error_mark_node;
> > + }
> > + }
> > + }
> > + }
> > + replace_array_notations (&rhs, true, rhs_list, rhs_array_operand);
> > + array_expr_rhs = rhs;
> > + }
> > + else
> > + {
> > + for (ii = 0; ii < rhs_list_size; ii++)
> > + {
> > + tree rhs_node = (*rhs_list)[ii];
> > + if (TREE_CODE (rhs_node) == CALL_EXPR)
> > + {
> > + int idx_value = 0;
> > + tree func_name = CALL_EXPR_FN (rhs_node);
> > + if (is_sec_implicit_index_fn (func_name))
> > + {
> > + idx_value = extract_sec_implicit_index_arg (location,
> > + rhs_node);
> > + if (idx_value < (int) lhs_rank && idx_value >= 0)
> > + vec_safe_push (rhs_array_operand, lhs_var[idx_value]);
> > + else
> > + {
> > + size_t ee = 0;
> > + tree lhs_base = (*lhs_list)[ii];
> > + for (ee = 0; ee < lhs_rank; ee++)
> > + if (lhs_base
> > + && TREE_CODE (lhs_base) ==
> ARRAY_NOTATION_REF)
> > + lhs_base = ARRAY_NOTATION_ARRAY (lhs_base);
> > + error_at (location, "__sec_implicit_index argument %d "
> > + "must be less than the rank of %qD", idx_value,
> > + lhs_base);
> > + return error_mark_node;
> > + }
> > + }
> > + }
>
> Again, a lot of stuff that looks exactly the same.
DONE!
>
> Similarly in the rest of expand_an_in_modify_expr(). The function is
> HUGE, and there's a lot of code duplication going on. Try to abstract
> the things you do more than once into their own helper functions. This
> will cut down on the maintenance burden.
>
> > + if (TREE_CODE (orig_stmt) == COND_EXPR)
> > + {
> > + size_t cond_rank = 0, yes_rank = 0, no_rank = 0;
> > + tree yes_expr = COND_EXPR_THEN (orig_stmt);
> > + tree no_expr = COND_EXPR_ELSE (orig_stmt);
> > + tree cond = COND_EXPR_COND (orig_stmt);
> > + if (!find_rank (EXPR_LOCATION (cond), cond, cond, true, &cond_rank)
> > + || !find_rank (EXPR_LOCATION (yes_expr), yes_expr, yes_expr, true,
> > + &yes_rank)
> > + || find_rank (EXPR_LOCATION (no_expr), no_expr, no_expr, true,
> > + &no_rank))
> > + return error_mark_node;
> > + if (cond_rank != 0 && cond_rank != yes_rank && yes_rank != 0)
> > + {
> > + error_at (EXPR_LOCATION (yes_expr), "rank mismatch with
> controlling"
> > + " expression of parent if-statement");
> > + return error_mark_node;
> > + }
> > + else if (cond_rank != 0 && cond_rank != no_rank && no_rank != 0)
> > + {
> > + error_at (EXPR_LOCATION (no_expr), "rank mismatch with controlling
> "
> > + "expression of parent if-statement");
> > + return error_mark_node;
> > + }
> > + }
> > + else if (TREE_CODE (orig_stmt) == IF_STMT)
> > + {
> > + size_t cond_rank = 0, yes_rank = 0, no_rank = 0;
> > + tree yes_expr = THEN_CLAUSE (orig_stmt);
> > + tree no_expr = ELSE_CLAUSE (orig_stmt);
> > + tree cond = IF_COND (orig_stmt);
> > + if (!find_rank (EXPR_LOCATION (cond), cond, cond, true, &cond_rank)
> > + || (yes_expr
> > + && !find_rank (EXPR_LOCATION (yes_expr), yes_expr, yes_expr,
> true,
> > + &yes_rank))
> > + || (no_expr
> > + && !find_rank (EXPR_LOCATION (no_expr), no_expr, no_expr, true,
> > + &no_rank)))
> > + return error_mark_node;
> > + if (cond_rank != 0 && cond_rank != yes_rank && yes_rank != 0)
> > + {
> > + error_at (EXPR_LOCATION (yes_expr), "rank mismatch with
> controlling"
> > + " expression of parent if-statement");
> > + return error_mark_node;
> > + }
> > + else if (cond_rank != 0 && cond_rank != no_rank && no_rank != 0)
> > + {
> > + error_at (EXPR_LOCATION (no_expr), "rank mismatch with controlling
> "
> > + "expression of parent if-statement");
> > + return error_mark_node;
> > + }
> > + }
>
> And here in cp_expand_cond_array_notations().
>
> > + for (ii = 0; ii < list_size; ii++)
> > + {
> > + jj = 0;
> > + jj_tree = (*array_list)[ii];
> > + while (jj_tree)
> > + {
> > + if (TREE_CODE (jj_tree) == ARRAY_NOTATION_REF)
> > + {
> > + array_ops[ii][jj] = jj_tree;
> > + jj++;
> > + jj_tree = ARRAY_NOTATION_ARRAY (jj_tree);
> > + }
> > + else if (TREE_CODE (jj_tree) == ARRAY_REF)
> > + jj_tree = TREE_OPERAND (jj_tree, 0);
> > + else if (TREE_CODE (jj_tree) == VAR_DECL
> > + || TREE_CODE (jj_tree) == PARM_DECL)
> > + break;
> > + }
> > + }
DONE!
>
> The above snippet appears in the exact same form in both
> expand_sec_reduce_builtin() and cp_expand_cond_array_notations(). I'm
> not going to mention any more of these duplicated code sequences, but
> you should go through the entire file and abstract what you can into
> separate inlineable functions, no sense maintaining 10 copies of the
> same thing.
I think I have fixed everything. Also, replaced the mallocs with vec_trees for ease of passing parameters.
So, is this OK for trunk?
>
> Aldy
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch_array_notation_cpp.txt
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20130617/b3c726ac/attachment.txt>
More information about the Gcc-patches
mailing list