This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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
Attachment:
patch_array_notation_cpp.txt
Description: patch_array_notation_cpp.txt
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |