>-----Original Message----- >From: Richard Guenther [mailto:richard.guent...@gmail.com] >Sent: Wednesday, September 26, 2012 8:16 AM >To: Iyer, Balaji V >Cc: gcc-patches@gcc.gnu.org; al...@redhat.com; r...@redhat.com; >l...@redhat.com >Subject: Re: [PATCH] Cilk Plus merging to trunk (2 of n) > >On Mon, Sep 24, 2012 at 5:05 PM, Iyer, Balaji V <balaji.v.i...@intel.com> >wrote: >> Hi Richard, >> Please see my comments embedded below. >> >>>-----Original Message----- >>>From: Richard Guenther [mailto:richard.guent...@gmail.com] >>>Sent: Monday, September 24, 2012 5:53 AM >>>To: Iyer, Balaji V >>>Cc: gcc-patches@gcc.gnu.org; al...@redhat.com; r...@redhat.com; >>>l...@redhat.com >>>Subject: Re: [PATCH] Cilk Plus merging to trunk (2 of n) >>> >>>On Sun, Sep 23, 2012 at 2:01 AM, Iyer, Balaji V <balaji.v.i...@intel.com> >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_langu >>>age_specif >>>ication.pdf). >>>> >>>> Here are the ChangeLog entries for the changes I have made in this patch. >>> >>>Just a few comments on the middle-end side: >>> >>>+/* Array Notation expression. >>>+ Operand 0 is the array; operand 1 is the starting array index >>>+ Operand 2 contains the number of elements you need to access. >>>+ Operand 3 is the stride. >>>+ Operand 4 is the element size measured in units of alignments of >>>+ element type. */ >>>+DEFTREECODE (ARRAY_NOTATION_REF, "array_notation_ref", >tcc_reference, >>>+5) >>> >>>I suppose that similar to ARRAY_RANGE_REF the type of the expression >>>specifies the size of the result which is an array itself? >>>ARRAY_NOTATION_REF is then ARRAY_RANGE_REF with stride possibly != 1. >>>Thus please instead change ARRAY_RANGE_REF to contain an explicit stride. >> >> IIRC ARRAY_RANGE_REF holds the start index and the end-index with no stride. >Whereas ARRAY_NOTATION_REF holds start_index, the number of elements you >need to access in the array and the stride. So, they both hold different >information. Converting one to another can cause some implementation and >performance complexities when we have assignments that have different stride >while accessing the same number of elements. Also, I did not see much (if any) >usage of Array_range_ref in C or C++. The only place that I saw were using it >were in Fortran and Ada. Adding the extra field for one thing can require >modifications those front-ends. This I felt was a bit unnecessary and >excessive. I >am replacing the ARRAY_NOTATION_REF tree with the appropriate array_ref and >a loop. Thus, having a tree that holds array notation information made things >look straightforward. >> >>> >>>+ case ARRAY_NOTATION_REF: >>>+ /* Nothing should happen here. We just return ALL_DONE. */ >>>+ ret = GS_ALL_DONE; >>>+ break; >>>+ >>> >>>I don't believe that. You should not restrict GENERIC to put >>>gimplfied operands into the operand slots. See how ARRAY_RANGE_REF is >handled. >>>The exception may be if ARRAY_NOTATION_REF never survives until >>>gimplfication (and thus is not part of GENERIC but a frontend specific tree). >> >> ARRAY_NOTATION_REF does not survive gimplifcation. It gets broken up >toward the end of parsing. In hindsight, I should have done something like >this: >> >> case ARRAY_NOTATION_REF: >> gcc_unreachable (); > >You should instead add ARRAY_NOTATION_REF to c-family/c-common.def as C- >family specific tree code then.
I actually took out the ARRAY_NOTATION_REF case from gimplify_expr (will send out the fixed patch soon). And, I can do what you suggested. But the reason why I put it as a common tree is because if someone in future wants to implement array notations for other languages, they can do so without much difficulty. If I did put the tree in c-common.def, where do I document the tree node (Or do I even need to document it)? I tried grepping the existing tree names (in c-common.def) in the doc directory and I don't see any hits. Thanks, Balaji V. Iyer. > >Richard. > >> Thanks, >> >> Balaji V. Iyer. >> >>> >>>Richard. >>> >>>> gcc/ChangeLog: >>>> 2012-09-22 Balaji V. Iyer <balaji.v.i...@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. >>>> * gimplify.c (gimplify_expr): Added a ARRAY_NOTATION_REF case. >>>> * 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 >>>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. >>>> * array-notation-common.c: New file. >>>> >>>> gcc/c/ChangeLog >>>> 2012-09-22 Balaji V. Iyer <balaji.v.i...@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/c-family/ChangeLog >>>> 2012-09-22 Balaji V. Iyer <balaji.v.i...@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/testsuite/ChangeLog >>>> 2012-09-21 Balaji V. Iyer <balaji.v.i...@intel.com> >>>> * gcc.dg/cilk-plus/array_notation/execute/execute.exp: New script. >>>> * gcc.dg/cilk-plus/array_notation/compile/compile.exp: New script. >>>> * 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/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. >>>> >>>> >>>> I have tested this on x86 and x86_64 and passes all the applicable >>>> regression >>>test. Is this OK for trunk? >>>> >>>> Thanking You, >>>> >>>> Yours Sincerely, >>>> >>>> Balaji V. Iyer.
