This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH] Cilk Plus merging to trunk (2 of n)
>-----Original Message-----
>From: Richard Guenther [mailto:richard.guenther@gmail.com]
>Sent: Wednesday, September 26, 2012 8:16 AM
>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 Mon, Sep 24, 2012 at 5:05 PM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:
>> Hi Richard,
>> Please see my comments embedded below.
>>
>>>-----Original Message-----
>>>From: Richard Guenther [mailto:richard.guenther@gmail.com]
>>>Sent: Monday, September 24, 2012 5:53 AM
>>>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, Sep 23, 2012 at 2:01 AM, Iyer, Balaji V <balaji.v.iyer@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.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.
>>>> * 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.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/c-family/ChangeLog
>>>> 2012-09-22 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/testsuite/ChangeLog
>>>> 2012-09-21 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: 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.