Cilk Plus Array Notation for C++

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jason Merrill
> Sent: Friday, June 28, 2013 9:43 AM
> To: Iyer, Balaji V; Richard Henderson
> Cc: Aldy Hernandez; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Cilk Plus Array Notation for C++
> 

> > +         /* Sometimes, when comma_expr has a function call in it, it will
> > +            typecast it to void.  Find_inv_trees finds those nodes and so
> > +            if it void type, then don't bother creating a new var to hold
> > +            the return value.   */
> > +         if (VOID_TYPE_P (TREE_TYPE (t)))
> > +           new_var = t;
> > +         else
> > +           new_var = get_temp_regvar (TREE_TYPE (t), t);
> 
> I was suggesting
> 
> if (VOID_TYPE_P (TREE_TYPE (t)))
>    {
>      finish_expr_stmt (t);
>      new_var = void_zero_node;
>    }
>

Fixed! 

> On 06/28/2013 12:54 AM, Iyer, Balaji V wrote:
> >             /* If stride and start are of same type and the induction var
> >                is not, convert induction variable to stride's type.  */
> >             if (TREE_TYPE (start) == TREE_TYPE (stride)
> >                 && TREE_TYPE (stride) != TREE_TYPE (var))
> >               {
> >                 st = start;
> >                 str = stride;
> >                 v = build_c_cast (loc, TREE_TYPE (str), var);
> >               }
> >             else if (TREE_TYPE (start) != TREE_TYPE (stride))
> >               {
> >                 /* If we reach here, then the stridje and start are of
> >                    different types, and so it doesn't really matter what
> >                    the induction variable type is, convert everything to
> >                    integer.  The reason why we pick an integer
> >                    instead of something like size_t is because the stride
> >                    and length can be + or -.  */
> >                 st = build_c_cast (loc, ptrdiff_type_node, start);
> >                 str = build_c_cast (loc, ptrdiff_type_node, stride);
> >                 v = build_c_cast (loc, ptrdiff_type_node, var);
> >               }
> >             else
> >               {
> >                 st = start;
> >                 str = stride;
> >                 v = var;
> >               }
> 
> I still think that what you want is to unconditionally convert start and stride to
> ptrdiff_t, either here or in build_array_notation_ref.
> 

Ok. I made the cast unconditional here. 

> > +           tree ii_tree = array_exprs[ii][jj];
> > +           (*node)[ii][jj].is_vector = true;
> > +           (*node)[ii][jj].value = ARRAY_NOTATION_ARRAY (ii_tree);
> > +           (*node)[ii][jj].start = ARRAY_NOTATION_START (ii_tree);
> > +           (*node)[ii][jj].length =
> > +             fold_build1 (CONVERT_EXPR, integer_type_node,
> > +                          ARRAY_NOTATION_LENGTH (ii_tree));
> > +           (*node)[ii][jj].stride =
> > +             fold_build1 (CONVERT_EXPR, integer_type_node,
> > +                          ARRAY_NOTATION_STRIDE (ii_tree));
> 
> I still don't understand what the purpose of cilkplus_an_parts is; it seems to have
> exactly the same information as the ARRAY_NOTATION_REF, so you might as
> well keep the array of ARRAY_NOTATION_REF instead of copying it into an array
> of cilkplus_an_parts.
> 

Well, it is mainly for ease of book-keeping. For example, if I have A[0:10][5:12] It is stored in ARRAY_NOTATION_REF like this:

<ARRAY_NOTATION_REF  <ARRAY_NOTATION_REF < 0, 10, 1>, 5, 12, 1>>

Now, if we take a simple polynomial example: A[0:10][5:12] + B[7:10][6:12:2]), we have something like this:

<PLUS_EXPR <ARRAY_NOTATION_REF < ARRAY_NOTATION_REF < 0, 10, 1>, 5, 12, 1>>, <ARRAY_NOTATION_REF<ARRAY_NOTATION_REF < 7, 10, 1>, 6, 12, 2>>>

Now, if we store this  PLUS_EXPR in the cilkplus_an_parts, we get something like this:

Node[0][0].start = 0
Node[0][0].length = 10
Node[0][0].stride = 1

Node[0][1].start = 5
Node[0][1].length = 12
Node[0][1].stride = 1

Node[1][0].start = 7
Node[1][0].length = 10
Node[1][0].stride = 1

Node[1][1].start = 6
Node[1][1].length = 12
Node[1][1].stride = 2

Now, if I have to do checking (e.g. make sure all the lengths are same across the rank), it is much easier for me to go through this set of size X rank arrays, than to recursively walk through the PLUS_EXPR and extract all the length values and check them. Also, if I have to add more features to array notations at a later time, this I feel is a lot more easier and straight-forward. In future, it something needs to be debugged, this table-like approach is a lot more simpler for me. Now, if we add things like a function call into the mix, then things get a bit more complicated for me.

Honestly, I don't think it adds that much memory overhead or dynamic code-footprint, and I free all the memory I allocated at the end of the function call.

> 
> > +       stride = RECUR (ARRAY_NOTATION_STRIDE (t));
> > +       if (!cilkplus_an_triplet_types_ok_p (loc, start_index, length, stride,
> > +                                            TREE_TYPE (op1)))
> > +         RETURN (error_mark_node);
> 
> You don't need to check the triplet types in tsubst, since you're checking them in
> build_array_notation_ref.
> 

Fixed!

So, is this OK for trunk?

> Jason

gcc/ChangeLog
2013-06-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * builtins.def: Fixed the function type of CILKPLUS_BUILTIN.

gcc/c/ChangeLog
2013-06-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-parser.c (c_parser_array_notation): Removed rejection of array
        notations in an array of function pointers.

gcc/c-family/ChangeLog
2013-06-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * array-notation-common.c (length_mismatch_in_expr_p): Collapsed two
        if-statements and compared the trees directly using tree_int_cst_equal.
        (find_rank): Checked for array notations in function name to handle
        array notations in function pointers.
        (extract_array_notation_exprs): Likewise.
        (replace_array_notations): Likewise.
        (cilkplus_extract_an_triplets): Replaced safe_push with
        safe_grow_cleared.  Also removed an unnecessary check to see if the
        node is of type ARRAY_NOTATION_REF.
        (fix_sec_implicit_args): Removed an unnecessary check for ADDR_EXPR.
        Also switched two if-statements to remove an unnecessary comparison.

gcc/cp/ChangeLog
2013-06-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * parser.c (cp_parser_array_notation): Removed rejection array notation
        of type function pointers. Added handling of array expressions when
        Cilk Plus is enabled.  Took out type-checking.
        (cp_parser_postfix_open_square_expression): Moved normal array expr.
        parsing into cp_parser_array_notation when cilkplus is enabled.
        (cp_parser_compound_statement): Removed expansion of array notations.
        (cp_parser_ctor_initializer_opt_and_function_body): Likewise.
        (cp_parser_function_definition_after_declarator): Likewise.
        (cp_parser_selection_statement): Removed error reporting.
        (cp_parser_iteration_statement): Likewise.
        (cp_parser_direct_declarator): Removed error checking/reporting if
        array notations are used in the declarator.
        * pt.c (instantiate_decl): Likewise.
        (type_unification_real): Removed a check for ARRAY_NOTATION_REF.
        (cxx_eval_constant_expression): Removed ARRAY_NOTATION_REF case.
        (potential_constant_expression_1): Returned false for ARRAY_NOTATION_REF
        case.
	(tsubst_copy_and_build): Removed type checking of array notation triplets.
        * cp-gimplify.c (cp_genericize): Added expansion of array notation
        expressions here.
        * cp-array-notation.c (make_triplet_val_inv): Removed loc and cry 
        parameters.  Replaced build_decls with get_temp_regvar with type as
        ptrdiff.
        (create_array_refs): Made the type-casting to ptrdiff_type unconditional.
        (replace_invariant_var): Added a check for void return type before 
	creating new var.  Replaced build_decl and build_min_nt_loc with
        get_temp_regvar.
        (expand_an_in_modify_expr): Ditto.  Replaced body of redundant else with
        gcc_unreachable.  Removed few unwanted checks.  Made induction variable
        type as ptrdiff_type.  Removed loc and complain arguments passed into
        make_triplet_val_inv.  Replaced all modify expression's code from NOP
        EXPR to INIT EXPR.  Replaced all forceful appending into stmt. list
        with the non-forceful one.  Replaced some integer conversion and
        equality-checking to using tree_int_cst_equal.
        (expand_sec_reduce_builtin): All changes mentioned in above function
        expand_an_in_modify_expr.  Made the new variable type of
        SEC_REDUCE_ANY/ALL_{NON}ZERO intrinsic functions as bool.
        (expand_array_notation_exprs): Removed SWITCH_EXPR case.  Moved all
        the error reporting from parser to this function.  Removed unwanted
        statements and checks from SWITCH_STMT, WHILE_STMT, and DO_STMT cases.
        (cilkplus_an_triplet_types_ok_p): Removed rejection of array notation
        in function pointers.
        (cp_expand_cond_array_notations): Added a new if statements to check
        if condition has a zero rank.  If so, then just return.
        (expand_return_expr): Added a check for return expressions with a rank.
        Replaced get_tmp_regvar with a create_temporary_var.
        (build_array_notation_ref): Simplified and removed unwanted if-stmts.
        Moved common code outside if-statements.  Moved type-checking from
        parser to here.
        * semantics.c (finish_return_stmt): Removed a check for return exprs.
        with a rank.
        * call.c (convert_like_real): Removed a check for array notation
        expression in a function.
        (build_over_call): Likewise.
        (magic_varargs_p): Added a check for builtin array notation function.
        Made this function non-static and removed its prototype.
        * cp-tree.h (magic_varargs_p): New prototype.
        * typeck.c (cp_build_function_call_vec): Removed automatic setting of
        nargs to the param->length when builtin reduction function is used.
        (convert_arguments): Replaced check for a constant_p function with
        margic_varargs_p function call.
        (cp_build_binary_op): Removed calling of the function
        find_correct_array_notation_type.
        (cp_build_addr_expr_1): Removed an unwanted if-statement.
        (convert_for_assignment): Removed automatic return of rhs when array
        notation builtin function is used.

gcc/testsuite/ChangeLog
2013-06-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-c++-common/cilk-plus/AN/decl-ptr-colon.c (main): Made this testcase
        c specific.
        * c-c++-common/cilk-plus/AN/decl-ptr-colon.c (main): Changed dg-error
        strings to match the fixed error messages.
        * c-c++-common/cilk-plus/AN/misc.c (main): Likewise.
        * c-c++-common/cilk-plus/AN/rank_mismatch.c (main): Added a new error
        message check.

Cilk Plus Array Notation for C++

Commit Message

Comments

Patch