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_language_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. 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.
