Hello Richard et al.,
Attached, please find a fixed patch. I have done the following changes:
1. Used the c_finish_loop (...) function instead of building the loop myself
2. Got rid of ARRAY_NOTATION_TYPE and just used TREE_TYPE ().
It is passing all the regression tests and not breaking/passing any other tests
that were not already breaking/passing in the trunk.
Here are the ChangeLog Entries:
gcc/ChangeLog
013-05-23 Balaji V. Iyer <balaji.v.i...@intel.com>
* doc/extend.texi (C Extensions): Added documentation about Cilk Plus
array notation built-in reduction functions.
* doc/passes.texi (Passes): Added documentation about changes done
for Cilk Plus.
* doc/invoke.texi (C Dialect Options): Added documentation about
the -fcilkplus flag.
* doc/generic.texi (Storage References): Added documentation for
ARRAY_NOTATION_REF storage.
* Makefile.in (C_COMMON_OBJS): Added c-family/array-notation-common.o.
(BUILTINS_DEF): Depend on cilkplus.def.
* builtins.def: Include cilkplus.def. Define DEF_CILKPLUS_BUILTIN.
* builtin-types.def: Define BT_FN_INT_PTR_PTR_PTR.
* cilkplus.def: New file.
gcc/c-family/ChangeLog
2013-05-23 Balaji V. Iyer <balaji.v.i...@intel.com>
* c-common.c (c_define_builtins): When cilkplus is enabled, the
function array_notation_init_builtins is called.
(c_common_init_ts): Added ARRAY_NOTATION_REF as typed.
* c-common.def (ARRAY_NOTATION_REF): New tree.
* c-common.h (build_array_notation_expr): New function declaration.
(build_array_notation_ref): Likewise.
(extract_sec_implicit_index_arg): New extern declaration.
(is_sec_implicit_index_fn): Likewise.
(ARRAY_NOTATION_CHECK): New define.
(ARRAY_NOTATION_ARRAY): Likewise.
(ARRAY_NOTATION_START): Likewise.
(ARRAY_NOTATION_LENGTH): Likewise.
(ARRAY_NOTATION_STRIDE): Likewise.
* c-pretty-print.c (pp_c_postifix_expression): Added a new case for
ARRAY_NOTATION_REF.
(pp_c_expression): Likewise.
* c.opt (flag_enable_cilkplus): New flag.
* array-notation-common.c: New file.
gcc/c/ChangeLog
2013-05-23 Balaji V. Iyer <balaji.v.i...@intel.com>
* c-typeck.c (build_array_ref): Added a check to see if array's
index is greater than one. If true, then emit an error.
(build_function_call_vec): Exclude error reporting and checking
for builtin array-notation functions.
(convert_arguments): Likewise.
(c_finish_return): Added a check for array notations as a return
expression. If true, then emit an error.
(c_finish_loop): Added a check for array notations in a loop
condition. If true then emit an error.
(lvalue_p): Added a ARRAY_NOTATION_REF case.
(build_binary_op): Added a check for array notation expr inside
op1 and op0. If present, we call another function to find correct
type.
* Make-lang.in (C_AND_OBJC_OBJS): Added c-array-notation.o.
* c-parser.c (c_parser_compound_statement): Check if array
notation code is used in tree, if so, then transform them into
appropriate C code.
(c_parser_expr_no_commas): Check if array notation is used in LHS
or RHS, if so, then build array notation expression instead of
regular modify.
(c_parser_postfix_expression_after_primary): Added a check for
colon(s) after square braces, if so then handle it like an array
notation. Also, break up array notations in unary op if found.
(c_parser_direct_declarator_inner): Added a check for array
notation.
(c_parser_compound_statement): Added a check for array notation in
a stmt. If one is present, then expand array notation expr.
(c_parser_if_statement): Likewise.
(c_parser_switch_statement): Added a check for array notations in
a switch statement's condition. If true, then output an error.
(c_parser_while_statement): Similarly, but for a while.
(c_parser_do_statement): Similarly, but for a do-while.
(c_parser_for_statement): Similarly, but for a for-loop.
(c_parser_unary_expression): Check if array notation is used in a
pre-increment or pre-decrement expression. If true, then expand
them.
(c_parser_array_notation): New function.
* c-array-notation.c: New file.
* c-tree.h (is_cilkplus_reduce_builtin): Protoize.
gcc/testsuite/ChangeLog
2013-05-23 Balaji V. Iyer <balaji.v.i...@intel.com>
* gcc.dg/cilk-plus/array_notation/compile/array_test2.c: New test.
* gcc.dg/cilk-plus/array_notation/compile/array_test1.c: Ditto.
* gcc.dg/cilk-plus/array_notation/compile/array_test_ND.c: Ditto.
* gcc.dg/cilk-plus/array_notation/compile/builtin_func_double.c: Ditto.
* gcc.dg/cilk-plus/array_notation/compile/builtin_func_double2.c: Ditto.
* gcc.dg/cilk-plus/array_notation/compile/gather_scatter.c: Ditto.
* gcc.dg/cilk-plus/array_notation/compile/if_test.c: Ditto.
* gcc.dg/cilk-plus/array_notation/compile/sec_implicit_ex.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/dimensionless-arrays.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/fp_triplet_values.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/gather-scatter.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/misc.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/parser_errors.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/parser_errors2.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/parser_errors3.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/parser_errors4.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/rank_mismatch.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/rank_mismatch2.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/rank_mismatch3.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/sec_implicit.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/sec_implicit2.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/sec_reduce_max_min_ind.c:
Ditto.
* gcc.dg/cilk-plus/array_notation/errors/tst_lngth.c: Ditto.
* gcc.dg/cilk-plus/array_notation/errors/vla.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/an-if.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/array_test1.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/array_test2.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/array_test_ND.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/builtin_fn_custom.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/builtin_fn_mutating.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/builtin_func_double.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/builtin_func_double2.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/comma_exp.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/conditional.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/exec-once.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/exec-once2.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/gather_scatter.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/if_test.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/n-ptr-test.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/sec_implicit_ex.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/side-effects-1.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/test_builtin_return.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/test_sec_limits.c: Ditto.
* gcc.dg/cilk-plus/array_notation/execute/cilkplus_AN_c_execute.exp:
New script.
* gcc.dg/cilk-plus/array_notation/compile/cilkplus_AN_c_compile.exp:
Ditto.
* gcc.dg/cilk-plus/array_notation/errors/cilkplus_AN_c_errors.exp:
Ditto.
Thanks,
Balaji V. Iyer.
PS. I used "Ditto" instead of "Likewise" for the testsuite ChangeLog entries so
that I can fix it all in 1 line. I hope that's OK.
> -----Original Message-----
> From: Richard Henderson [mailto:r...@redhat.com]
> Sent: Wednesday, May 22, 2013 1:34 PM
> To: Iyer, Balaji V
> Cc: 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
>
> On 2013-05-22 08:18, Iyer, Balaji V wrote:
> > The overall function names are same, but the components inside it
> > function differs greatly from C and C++. For example, in C++ I can't
> > use build_modify_expr, but build_x_modify_expr. Also, I need to handle
> > overloaded function types, and that requires further checking.
> > Similarly, the trees are different from C and C++, and so I have to do
> > more checks and handle them differently in C++. In my mind, this seem
> > to be the most straight-forward and legible way to do it.
>
> Really, that's surprising:
>
> /* For use from the C common bits. */
> tree
> build_modify_expr (location_t /*location*/,
> tree lhs, tree /*lhs_origtype*/,
> enum tree_code modifycode,
> location_t /*rhs_location*/, tree rhs,
> tree /*rhs_origtype*/) {
> return cp_build_modify_expr (lhs, modifycode, rhs, tf_warning_or_error); }
>
> I can definitely see how function overloading could potentially get in the
> way.
>
> Although by delaying expansion of ARRAY_NOTATION_REF, given that the
> TREE_TYPE of the ARRAY_NOTATION_REF is not some complex artificial slice
> type but the element type of the array, I wonder how much of the overloaded
> function selection would just happen automatically?
>
> Which reminds me: What is ARRAY_NOTATION_TYPE for? Isn't it always the
> same as the TREE_TYPE of the ARRAY_NOTATION_REF?
>
> > Before I started my implementation, I did briefly consider using
> > c_finish_loop, but adding multiple expressions got a bit complicated.
> > For example, if I have 2 increment/condition expressions (my initial
> > implementation had it but later on I eliminated that need), I had to
> > create a statement list and then add them using
> > append_to_statement_list() and then pass that into c_finish_loop.
> > Also, if I am not mistaken, I still had to create some of the
> > individual labels when using c_finish_loop(). Overall, I didn't find
> > much code-size decrease, and I found this a bit more straight forward for
> > me. I
> apologize if I am mistaken.
>
> Well, append_to_statement_list etc was certainly not needed. Mirroring
> exactly
> how you'd write it in C, with a comma operator also works. That's a
> COMPOUND_EXPR, fyi.
>
> There are continue/break label arguments to c_finish_loop, but they may be
> (and often are) null. They will only be non-null if the relevant statements
> actually exist inside the loop. C.f. c_finish_bc_stmt and c_break_label.
>
> You're right about it not saving *that* much code, but the point is more
> about if
> we make improvements to normal C loop representation -- such as to support
> #pragma simd -- then we want to be able to take advantage of that here.
>
> > Are these changes a blocking issue for acceptance into trunk? Or,
> > would it be ok to accept it as-is and then I make these changes at a later
> > date?
>
> I'm leaning to honoring the advice you were given last year and accepting the
> patch more or less in its current form.
>
>
> r~
patch.txt.bz2
Description: patch.txt.bz2
