Hello Joseph, Here is the fixed patch with all your changes and the ChangeLog entries below. gcc/ChangeLog 2012-11-05 Balaji V. Iyer * Makefile.in (C_COMMON_OBJS): Added c-family/array-notation-common.o. * doc/passes.texi (Cilk Plus Transformation): Documented array notation and overall transformations for Cilk Plus. * doc/invoke.texi (C Dialect Options): Documented -fcilkplus flag. * doc/generic.texi (Storage References): Documented ARRAY_NOTATION_REF tree addition. gcc/c-family/ChangeLog 2012-11-05 Balaji V. Iyer * c-common.h (build_array_notation_expr): New function declaration. (ARRAY_NOTATION_ARRAY): Added new #define. (ARRAY_NOTATION_CHECK): Likewise. (ARRAY_NOTATION_START): Likewise. (ARRAY_NOTATION_LENGTH): Likewise. (ARRAY_NOTATION_STRIDE): Likewise. (ARRAY_NOTATION_TYPE): Likewise. (enum array_notation_reduce_type): Added new enumerator. * c-common.def: Added new tree ARRAY_NOTATION_REF. * c-common.c (c_define_builtins): Added a call to initialize array notation builtin functions. (c_common_init_ts): Set ARRAY_NOTATION_REF as typed. * c-pretty-print.c (pp_c_postfix_expression): Added ARRAY_NOTATION_REF case. * c.opt (-fcilkplus): Define new command line switch. * array-notation-common.c: New file. gcc/c/ChangeLog 2012-11-05 Balaji V. Iyer * 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 & 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. * 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): Same as switch statement, but for a while. (c_parser_do_statement): Same as switch statement, but for a do-while. (c_parser_for_statement): Same as switch statement, 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. gcc/testsuite/ChangeLog 2012-11-05 Balaji V. Iyer * gcc.dg/cilk-plus/array_notation/execute/execute.exp: New script. * gcc.dg/cilk-plus/array_notation/compile/compile.exp: Likewise. * gcc.dg/cilk-plus/array_notation/errors/errors.exp: Likewise. * gcc.dg/cilk-plus/array_notation/execute/sec_implicit_ex.c: New test. * gcc.dg/cilk-plus/array_notation/execute/comma_exp.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/conditional.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/exec-once.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/if_test.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/n-ptr_test.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/gather_scatter.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/builtin_func_double2.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/builtin_func_double.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/builtin_fn_custom.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/builtin_fn_mutating.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/array_test_ND.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/array_test2.c: Likewise. * gcc.dg/cilk-plus/array_notation/execute/array_test1.c: Likewise. * gcc.dg/cilk-plus/array_notation/compile/sec_implicit_ex.c: Likewise. * gcc.dg/cilk-plus/array_notation/compile/gather_scatter.c: Likewise. * gcc.dg/cilk-plus/array_notation/compile/builtin_func_double2.c: Likewise. * gcc.dg/cilk-plus/array_notation/compile/array_test_ND.c: Likewise. * gcc.dg/cilk-plus/array_notation/compile/if_test.c: Likewise. * gcc.dg/cilk-plus/array_notation/compile/builtin_func_double.c: Likewise. * gcc.dg/cilk-plus/array_notation/compile/array_test1.c: Likewise * gcc.dg/cilk-plus/array_notation/compile/array_test2.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/sec_implicit.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/sec_implicit2.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/parse_error.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/parse_error2.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/parse_error3.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/parse_error4.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/sec_reduce_max_min_ind.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/fn_triplet_values.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/gather-scatter.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/misc.c: Likewise. * gcc.dg/cilk-plus/array_notation/errors/vla.c: Likewise. Thanks, Balaji V. Iyer. > -----Original Message----- > From: Joseph Myers [mailto:joseph@codesourcery.com] > Sent: Friday, October 19, 2012 5:38 PM > To: Iyer, Balaji V > Cc: Richard Guenther; gcc-patches@gcc.gnu.org > Subject: RE: [Ping]FW: [PATCH] Cilk Plus merging to trunk (2 of n) > > On Thu, 4 Oct 2012, Iyer, Balaji V wrote: > > > >>>>Here is a link to the latest spec. This should clear several of > > >>>>the questions you are seeking. > > >>>>(http://software.intel.com/sites/default/files/m/4/e/7/3/1/40297- > > >>>>Intel_Cilk_plus_lang_spec_2.htm#array) > > This specification is much improved, especially as regards specifying the types of > section expressions. I'm not convinced that "the type of the array being > subscripted shall have a declared size" is properly defined in standard terms, but > I can guess reasonable semantics - that if the array-to-pointer decay were > considered not to occur in such a context, then the expressions for the array > being subscripted shall have array type, not pointer type, and the array type shall > not be one with unspecified size (array[]), although it may be a VLA. For > example, given "int a[10];", it would be valid to say a[:] or (a)[:] but not (+a)[:]. I > don't, however, see any testcases at all in this patch for that particular > requirements - not even for the completely clear-cut cases, such as giving an > error for "extern int a[]; a[:];" or "int *a; a[:];". > > Say expr1 through expr9 are expressions with side effects, and you have: > > expr1[expr2:expr3:expr4] = expr5[expr6:expr7:expr8] + expr9; > > The spec says "However, in such a statement, a sub-expression with rank zero is > evaluated only once." - that is, each of the nine expressions is evaluated once. I > don't see any calls to save_expr to ensure these semantics, or any testcases that > verify that they are adhered to. > > (Are multidimensional section expressions valid when what you have is pointers > to pointers, e.g. "int ***p; p[0:10][0:10][0:10];"? I don't see anything to rule > them out, so I assume they are valid, but don't see testcases for them either.) > > Looking at the patch itself: > > In find_rank you have error ("Rank Mismatch!"); - this is not a properly > formatted error message according to the GNU Coding standards (which > typically would not have any uppercase). I'd also suggest that when you find a > rank, you store (through a location_t * pointer) the location of the first > expression found with that rank, so if you then find a mismatching rank you can > use error_at to point to that rank and then inform to point to the previous rank > it didn't match. > > I'm not convinced that your logic, falling back to examining each operand for a > generic expression, is correct to find the ranks of all kinds of expressions. For > example, there are rules: > > * "The rank of a simple subscript expression (postfix-expression [ expression ]) is > the sum of the ranks of its operand expressions. The rank of the subscript > operand shall not be greater than one." - how do you ensure this rule? Where > do you test for errors if the subscript has too high a rank (both in the front-end > code, and in the testsuite), and test (in the testsuite) for cases where the > subscript has rank 1? > > * "The rank of a comma expression is the rank of its second operand." - I don't > see anything special to handle that. Are there testcases for rank of comma > expressions? Apart from testing rank, you may need to test how they are > evaluated (that each part, with independent rank, gets fully evaluted in turn) - I > don't see anything obvious in the code to handle them appropriately. > > In general, I'd say you should have tests in the testsuite for each syntactic type > of expression supported by GCC, both standard and GNU extensions, testing > how it interacts with section expressions - both valid cases, and cases that are > invalid because of rank mismatches. As another example, you don't have tests > of conditional expressions. > > Where do you test (both in code, and testcases to verify errors) that "The rank > of each expression in a section triplet shall be zero."? What about "The rank of > the postfix expression identifying the function to call shall be zero."? "A full > expression shall have rank zero, unless it appears in an expression statement or > as the controlling expression of an if statement."? (This means, I suppose, that > uses such as initializers or sizes in array declarators must be rejected.) I'd advise > going through each sentence in the relevant part of the spec that says > something is invalid and making sure you diagnose it and have a test of this. > > Where in the patch you use int for the size of something (e.g. a list) on the host, > please use size_t. > > In extract_array_notation_exprs you appear to be reallocating every time > something is added to a list (with XRESIZEVEC). It would probably be more > efficient to use the vec.h infrastructure for an automatically resizing vector on > which you push things. > > In c_parser_array_notation you appear to be converting indices to > integer_type_node in some cases, not converting at all in other cases. > But the spec says "The expressions in a triplet are converted to ptrdiff_t.", so > you need to convert to target ptrdiff_t, not target int. > And there's a requirement that "Each of the expressions in a section triplet shall > have integer type.". So you need to check that, and give an error if it doesn't > have integer type, before converting - and of course add testcases for each of > the possible positions for an expression having one that doesn't have integer > type. > > In c-typeck.c you disable some errors and warnings for expressions containing > array notations. I don't know where the later point is at which you check for > such errors - but in any case, you need testcases for these diagnostics on those > cases to show that they aren't being lost. > > In invoke.texi you have: > > +@opindex flag_enable_cilkplus > > But @opindex is for the user-visible options, not for internal variables. > That is, > > @opindex fcilkplus > > would be appropriate. > > In passes.texi you refer to "the Cilk runtime library (located in libcilkrts > directory)". But no such directory is added by this patch. > Only add references to it in documentation with the patch that adds the > directory. > > -- > Joseph S. Myers > joseph@codesourcery.com