[PATCH] Cilk Plus merging to trunk (2 of n)
Iyer, Balaji V
balaji.v.iyer@intel.com
Wed Sep 26 23:17:00 GMT 2012
Hello Joseph,
Please see my responses below and I have attached a fixed patch:
>-----Original Message-----
>From: Joseph Myers [mailto:joseph@codesourcery.com]
>Sent: Sunday, September 23, 2012 4:45 PM
>To: Iyer, Balaji V
>Cc: gcc-patches@gcc.gnu.org; aldyh@redhat.com; rth@redhat.com;
>law@redhat.com
>Subject: Re: [PATCH] Cilk Plus merging to trunk (2 of n)
>
>On Sun, 23 Sep 2012, Iyer, Balaji V wrote:
>
>> Hello Everyone,
>> Attached, please find a patch that will implement Cilk Plus Array
>> Notations for the C compiler. Array notations are indented to allow
>> programmers to directly express parallelism in their programs. Array
>> notations can be used to possibly see a more predictable performance
>> improvement, hardware resource-utilization and vectorization. To
>> enable the compiler recognize array notation syntax, we have added a
>> flag "-fcilkplus." If this flag is not used, none of these changes are
>> visible to the compiler user. For more information and examples about
>> array notations please see Chapter 4 in the Cilk Plus Specification
>>
>(http://software.intel.com/sites/default/files/m/6/3/1/cilk_plus_language_specif
>ication.pdf).
>
>There seem to be a lot of deficiencies in this specification. I list some here - I
>don't want answers in email explaining what is intended (unless it's already
>explained in the specification and I missed it), I want the specification revised to
>describe things in closer relation to C standard terms, and then you can answer in
>email pointing to the relevant wording in the improved specification - and to the
>testcases in your patch verifying that the semantics are properly implemented,
>but without needing to elaborate beyond pointing to specification text and
>testcases for each question.
>
>What syntax productions, in C11 terms, are <array base>, <lower bound>,
><length> and <stride> in the syntax given for section operators? Are there
>constraints that <lower bound>, <length> and <stride> must be of integer type?
>Are there other constraints on their types and values?
>(For example, must anything be constant? If there isn't such a requirement, what
>exactly is the requirement that "The right-hand side expression must have the
>same rank and size as the array context." - if a constraint, in what cases is it a
>constraint? Maybe a constraint that values must match if integer constant
>expressions, with non-matching runtime values, at least one not an integer
>constant expression, being undefined behavior at runtime, for example? What
>anyway is "size" in that quoted sentence? Unlike "rank" and "shape", it doesn't
>seem to be a defined term.)
>
>What do you mean, in standard terms, by "must have a declared size"? How is
>this defined in relation to standard C array-to-pointer decay? What about
>adjustment of function parameter types? What about array sizes declared as [*]?
>What if the <array base> is in parentheses?
>
>Should I take it, from the absence of any restrictions mentioned on this subject,
>that the array that is sectioned may have elements of arbitrary type - so it's valid
>to operate this way on arrays of pointers, complex numbers or structures, for
>example? But that the usual constraints on assignment apply, so that if you try
>A[:] = B[:] where A and B are two-dimensional arrays, this is a constraint
>violation, because the elements that would be assigned elementwise are
>themselves still arrays and C doesn't allow array assignment?
>
>Is it valid or invalid to have an expression of the form (A[1:2])[1:2] with
>parentheses around a partial array section expression, of which a further section
>is taken? It's not in the syntax given, but that syntax doesn't actually show the
>precise syntax productions added to C11.
>
>What is the type of an array section expression? What is the result of applying
>sizeof to such an expression? What about GNU C typeof? Use in
>C11 _Generic?
>
>Can such expressions be used in conditional expressions, scalar ? section
>: other-section? If you'd defined types, at least this could be deduced from the
>existing C rules on conditional expressions that say what the permitted types are.
>Can you use them with comma operators, A[:] = (B, C[:])? It's far from clear from
>the document as-is to what extent such expressions have an existence with types
>and values like normal expressions, in which case this would of course be
>permitted, as opposed to being special-case things for the RHS of assignments.
>
>You say (4.3.2.2) that certain operators are applied with the same precedence
>and rules on permitted operands as for scalars. What promotions apply? If
>operations are carried out on two signed char arrays, for example, are the
>elements considered to be promoted to int, resulting in an expression whose type
>is thought of as a section of an array of int, which may then be converted back to
>signed char if stored in a signed char array? (Thus, internal operations take place
>in int and what would have been overflow in signed char would not cause
>undefined
>behavior.)
>
>In general, do such conversions apply between different operands, and on
>assignment?
>
>I note you do not mention shift operators in the list in 4.3.2.2 of those permitted -
>obviously that requires testcases that they are duly rejected.
>
>Are functions such as __sec_reduce_add defined to apply the relevant operation
>to an accumulated value and each element of the section in turn, in some
>unspecified order? Or may it evaluate, for example, a sum of four elements as (a
>+ b) + (c + d)? What about if a user function is provided?
>
>I take it each function has constraints corresponding to those on the relevant
>arithmetic operation? (For example, __sec_reduce_add couldn't be called for
>pointers.) Can the *zero functions be called for pointers (testing whether they
>are NULL)? What about the *max* and *min* functions? What about those
>functions for floating-point - do they follow the semantics of fmax / fmin
>regarding NaNs? Are the results unspecified if the answer is a floating-point zero
>and both +0.0 and -0.0 are present?
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)
>
>(Regarding the patch itself, see my previous comments on previous patches in this
>series. For example:
>
>* All functions should have comments explaining the semantics of their
>arguments and return values.
Fixed. Please see attached patch.
>
>* Diagnostic function calls should have explicit locations passed.
Fixed.
>
>* Every diagnostic (that's not an ICE) should have a corresponding testcase in the
>testsuite for the associated cases of invalid code - I see no tests using dg-error at
>all in this patch. Every case where the specification says something is not
>permitted should have an associated check in the compiler, diagnostic and
>testcase.
Added. Please see testsuite/gcc.dg/cilk-plus/errors directory in the patch.
>
>* Diagnostic formatting also needs fixing to follow the GNU Coding Standards.
>
>* The usage
>
>+ error ("__sec_implicit_index parameter must be constant integer "
>+ "expression");
>+ error ("Bailing out due to previous error");
>+ exit (ICE_EXIT_CODE);
>
>should be avoided. Front-end code should never need to call exit directly. If you
>have an internal error - something that should not be possible for any user code,
>valid or invalid - use internal_error. If an error can occur for invalid user code,
>just call error_at and allow the compiler to continue execution to find further
>problems in the user's code, without bailing out.
Fixed.
>
>* There should be nothing x86-specific about the testcases so they shouldn't need
>to be conditioned on x86 targets.
OK, removed from the scripts.
>
>* Avoid using "int" as a type in the compiler to count the number of some entity
>on the host, use size_t instead, there's no need to add new cases that will cause
>trouble when someone wants to build a program with 2^31 of something,
>although there are plenty of such cases already.
OK.
>
>* Instead of casting the result of xmalloc, use existing macros such as XNEWVEC.
Fixed.
>
>* Use @option not @code for option named in the manual.
Fixed.
>
>* Instead of declaring non-static functions
>
>+int extract_sec_implicit_index_arg (tree); bool
>+is_sec_implicit_index_fn (tree); void array_notation_init_builtins
>+(void);
>
>in a source file (array-notation-common.c), either make them static or declare
>them in an appropriate header included everywhere needing those declarations.
Fixed.
>
Attached, please find a fixed patch for array notation for C. OK for trunk?
Here is the ChangeLog for the Changes:
gcc/ChangeLog
2012-09-26 Balaji V. Iyer <balaji.v.iyer@intel.com>
* tree.h (array_notation_reduce_type): Added new enumerator.
(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.
* tree.def: Added new tree ARRAY_NOTATION_REF.
* Makefile.in (OBJS): Added 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.
* tree-pretty-pretty.c (dump_generic_node): Added ARRAY_NOTATION_REF
case.
* array-notation-common.c: New file.
gcc/c-family/ChangeLog
2012-09-26 Balaji V. Iyer <balaji.v.iyer@intel.com>
* c-common.h (build_array_notation_expr): New function declaration.
* c-common.c (c_define_builtins): Added a call to initialize array
notation builtin functions.
* c.opt (-fcilkplus): Define new command line switch.
gcc/c/ChangeLog
2012-09-26 Balaji V. Iyer <balaji.v.iyer@intel.com>
* c-typeck.c (convert_arguments): Added a check if tree contains
array notation expressions before throwing errors or doing anything.
* 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_array_notation): New function.
* c-array-notation.c: New file.
gcc/testsuite/ChangeLog
2012-09-26 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/if_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.
Thanks,
Balaji V. Iyer.
>There may also be other issues.)
>
>--
>Joseph S. Myers
>joseph@codesourcery.com
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: array_notation_c_patch2.txt
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20120926/290a3473/attachment.txt>
More information about the Gcc-patches
mailing list