RE: [Ping]FW: [PATCH] Cilk Plus merging to trunk (2 of n)

["Iyer, Balaji V" <balaji.v.iyer@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 13443 bytes --]

Hello Joseph,
	Here is the fixed patch with all your changes and the ChangeLog entries below.

gcc/ChangeLog
2012-11-05  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * 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  <balaji.v.iyer@intel.com>

        * 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  <balaji.v.iyer@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 & 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  <balaji.v.iyer@intel.com>

        * 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

[-- Attachment #2: array_notation_c_patch5.txt.gz --]
[-- Type: application/x-gzip, Size: 31933 bytes --]