[PING]RE: [patch] cilkplus: Array notation for C patch

[PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

Can someone please review this patch for us?

Thanks,

Balaji V. Iyer.


> -----Original Message-----
> From: Iyer, Balaji V
> Sent: Monday, May 06, 2013 11:32 AM
> To: Joseph S. Myers
> Cc: 'Aldy Hernandez'; 'gcc-patches'
> Subject: RE: [patch] cilkplus: Array notation for C patch
> 
> Attached, please find a fixed patch. My responses to your comments are inline
> below:
> 
> Thanks,
> 
> Balaji V. Iyer.
> 
> Here are the ChangeLog entries:
> 
> gcc/ChangeLog
> +2013-05-06  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +	* doc/extend.texi (C Extensions): Added documentation about Cilk Plus
> +	array notation built-in reduction functions.
> +	* doc/passes.texi (Passes): Added documentation about changes done
> +	for Cilk Plus.
> +	* doc/invoke.texi (C Dialect Options): Added documentation about
> +	the -fcilkplus flag.
> +	* doc/generic.texi (Storage References): Added documentation for
> +	ARRAY_NOTATION_REF storage.
> +	* Makefile.in (C_COMMON_OBJS): Added c-family/array-notation-
> common.o.
> +	(BUILTINS_DEF): Depend on cilkplus.def.
> +	* builtins.def: Include cilkplus.def.  Define DEF_CILKPLUS_BUILTIN.
> +	* builtin-types.def: Define BT_FN_INT_PTR_PTR_PTR.
> +	* cilkplus.def: New file.
> 
> gcc/c-family/ChangeLog
> +2013-05-06  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +	* c-common.c (c_define_builtins): When cilkplus is enabled, the
> +	function array_notation_init_builtins is called.
> +	(c_common_init_ts): Added ARRAY_NOTATION_REF as typed.
> +	* c-common.def (ARRAY_NOTATION_REF): New tree.
> +	* c-common.h (build_array_notation_expr): New function declaration.
> +	(build_array_notation_ref): Likewise.
> +	(extract_sec_implicit_index_arg): New extern declaration.
> +	(is_sec_implicit_index_fn): Likewise.
> +	(ARRAY_NOTATION_CHECK): New define.
> +	(ARRAY_NOTATION_ARRAY): Likewise.
> +	(ARRAY_NOTATION_START): Likewise.
> +	(ARRAY_NOTATION_LENGTH): Likewise.
> +	(ARRAY_NOTATION_STRIDE): Likewise.
> +	(ARRAY_NOTATION_TYPE): Likewise.
> +	* c-pretty-print.c (pp_c_postifix_expression): Added a new case for
> +	ARRAY_NOTATION_REF.
> +	(pp_c_expression): Likewise.
> +	* c.opt (flag_enable_cilkplus): New flag.
> +	* array-notation-common.c: New file.
> 
> gcc/c/ChangeLog
> +2013-05-06  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +	* c-typeck.c (build_array_ref): Added a check to see if array's
> +	index is greater than one.  If true, then emit an error.
> +	(build_function_call_vec): Exclude error reporting and checking
> +	for builtin array-notation functions.
> +	(convert_arguments): Likewise.
> +	(c_finish_return): Added a check for array notations as a return
> +	expression.  If true, then emit an error.
> +	(c_finish_loop): Added a check for array notations in a loop
> +	condition.  If true then emit an error.
> +	(lvalue_p): Added a ARRAY_NOTATION_REF case.
> +	(build_binary_op): Added a check for array notation expr inside
> +	op1 and op0.  If present, we call another function to find correct
> +	type.
> +	* 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_direct_declarator_inner): Added a check for array
> +	notation.
> +	(c_parser_compound_statement): Added a check for array notation in
> +	a stmt.  If one is present, then expand array notation expr.
> +	(c_parser_if_statement): Likewise.
> +	(c_parser_switch_statement): Added a check for array notations in
> +	a switch statement's condition.  If true, then output an error.
> +	(c_parser_while_statement): Similarly, but for a while.
> +	(c_parser_do_statement): Similarly, but for a do-while.
> +	(c_parser_for_statement): Similarly, but for a for-loop.
> +	(c_parser_unary_expression): Check if array notation is used in a
> +	pre-increment or pre-decrement expression.  If true, then expand
> +	them.
> +	(c_parser_array_notation): New function.
> +	* c-array-notation.c: New file.
> +	* c-tree.h (is_cilkplus_reduce_builtin): Protoize.
> 
> 
> > -----Original Message-----
> > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> > owner@gcc.gnu.org] On Behalf Of Joseph S. Myers
> > Sent: Monday, April 29, 2013 6:55 PM
> > To: Iyer, Balaji V
> > Cc: 'Aldy Hernandez'; 'gcc-patches'
> > Subject: RE: [patch] cilkplus: Array notation for C patch
> >
> > Here's a review of the changes to the compiler proper in this patch.
> > I don't think much more will come up from reviews of the compiler
> > changes - but I still need to review the testsuite changes against the
> > language specification to make sure that everything is properly
> > covered in the testsuite (which might in turn show up further things needing to
> be addressed in the compiler).
> >
> > > +	  error_at (location, "__sec_implicit_index parameter must be a "
> > > +		    "integer constant expression");
> >
> > "an", not "a".
> 
> FIXED!
> 
> >
> > > diff --git a/gcc/c/ChangeLog.cilkplus b/gcc/c/ChangeLog.cilkplus
> >
> > I believe the actual trunk commit, when this is ready to go in, should
> > simply add the ChangeLog entries for the committed changes to the top
> > of the existing ChangeLog files, rather than creating such a new ChangeLog
> file.
> >
> > > diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
> >
> > > +#include "gcc.h"
> >
> > That header is for the compiler driver.  Including it in anything
> > built into cc1 is suspicious.
> 
> Removed. This was an unnecessary #include.
> 
> >
> > > +/* Given an FNDECL or an ADDR_EXPR, return the corresponding
> >
> > I think you mean something like "Given FNDECL, a FUNCTION_DECL or an
> > ADDR_EXPR", rather than "Given an FNDECL or an ADDR_EXPR".
> 
> Yep. This is fixed.
> 
> >
> > > +/* Set *RANK of expression ARRAY, ignoring array notation specific built-in
> > > +   functions if IGNORE_BUILTIN_FN is true.  The ORIG_EXPR is
> > > +printed out if
> > an
> > > +   error occured in the rank calculation.  The functions returns false if it
> > > +   encounters an error in rank calculation.
> > > +
> > > +   For example, an array notation of A[:][:] or B[0:10][0:5:2] or C[5][:][1:0]
> > > +   all have a rank of 2.  */
> >
> > This still doesn't seem to say anything about the semantics of the
> > value *RANK on entry to the function.  (I think it's something like
> > *RANK being either 0, or the rank of another subexpression that must
> > have the same rank as this one, but you need to say that.)
> 
> I have reworded this, please let me know if it is OK.
> 
> >
> > > +/* Extracts all array notations in NODE and stores them in ARRAY_LIST.  If
> > > +   IGNORE_BUILTIN_FN is set, then array notations inside array notation
> > > +   specific built-in functions are ignored.  The NODE can be anything from a
> > > +   full function to a single variable.   */
> >
> > "can be anything"?  That seems rather ad hoc.  I'd think there should
> > be defined classes of trees - probably expressions and things that can
> > appear in them, but not tcc_exceptional or tcc_type - that can appear
> > here, and that you should check (in an assertion) for EXPR_P or one of the
> other cases allowed.
> 
> I have made this more specific
> 
> >
> > In particular, you allow TREE_LIST in this function.  How can
> > TREE_LISTs get here and can they readily be avoided?  It's generally a
> > bad idea (and rare) to have places where something with the static
> > type "tree" can be either a TREE_LIST or some other kind of tree.  I
> > note that in the function replace_array_notations, which is presumably
> > intended to match this one, you
> > *don't* handle TREE_LIST.
> 
> I removed TREE_LIST checks.
> 
> >
> > These functions recurse down into operands of trees.  But what about
> > into types?  If a type contains an expression that needs to be
> > evaluated as part of evaluating VLA sizes, that gets stored specially
> > by grokdeclarator, and in the end that expression get put in a
> > statement somewhere to ensure that it does get evaluated.  But that's
> > for expressions with side effects involved in types.  Array notation
> > expressions may not necessarily have side effects.  And as I
> > understand it, even if an expression is extracted OK by
> > extract_array_notation_exprs because it appears somewhere that
> > function looks at, replace_array_notations will need to substitute it
> everywhere - substituting a copy appearing directly in a statement / expression,
> while missing a copy embedded in a type, won't suffice.
> > So maybe you need to recurse down into types in some way?  (Then I'm
> > not entirely sure when it's safe to modify an existing type and when
> > you'd need to build up a new, similar type with the expression
> > modified appropriately.)
> >
> > Maybe an example would help.  I see nothing in the Cilk Plus
> > specification to rule out expressions of the form
> >
> > a[:] = ((int (*)[b[:]][c[:]]) d[:])[1][2];
> >
> > meaning that each element of the array d should be cast to a
> > pointer-to-VLA type, with the dimensions of the VLA coming from
> > corresponding elements of arrays b and c, and then element[1][2] of
> > that VLA extracted.  But the rules for determining rank don't really
> > seem to consider subexpressions that appear within types, so maybe
> > adjustments are needed there as well.  (Of course such type names can
> > appear within expressions in sizeof, or compound literals, or several
> > other cases in the syntax, not just in casts.)
> >
> > It's possible that the above case does work despite types not being
> > adjusted, because the logic to multiply by array sizes when doing
> > pointer addition / array dereference may already have taken effect
> > while the expressions were constructed.  But leaving types unadjusted
> > still seems rather risky, and would seem likely to cause problems with
> > debug info (consider the case where a variable is actually being
> > declared with the type involving array notation, in a GNU C statement
> expression, for example) if nothing else.
> >
> > (I suppose changing the specification to disallow array notation
> > within types would be one way to avoid a lot of such issues.)
> 
> Yes, you can't have array notation in types. We will fix this in the next revision of
> the manual.
> 
> >
> > > +/* Top-level function to replace ARRAY_NOTATION_REF in a
> > > +conditional
> > statement
> > > +   in STMT.  */
> > > +
> > > +tree
> > > +fix_conditional_array_notations (tree stmt)
> >
> > This comment doesn't seem to say anything abou the return value semantics.
> 
> FIXED!
> 
> >
> > > +/* Returns true of EXPR (and its subtrees) contain
> > > +ARRAY_NOTATION_EXPR node.  */
> >
> > "true if", not "true of".
> 
> FIXED!
> 
> >
> > > +/* Walks through tree node T and find all the call-statments that
> > > +do not return
> >
> > Statements, not statments.
> 
> FIXED!
> 
> >
> > > diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index
> > > 2ae4622..f051ab5 100644
> > > --- a/gcc/c/c-parser.c
> > > +++ b/gcc/c/c-parser.c
> > > @@ -55,6 +55,7 @@ along with GCC; see the file COPYING3.  If not see
> > > #include "cgraph.h"
> > >  #include "plugin.h"
> > >
> > > +
> > >
> 
> > >  /* Initialization routine for this file.  */
> > >
> >
> > Spurious diff hunk just adding whitespace.
> 
> FIXED!
> 
> >
> > > +  if (flag_enable_cilkplus && contains_array_notation_expr (cond))
> > > +    {
> > > +      error_at (start_locus, "array notation expression cannot be used in a "
> > > +		"loop%'s condition");
> > > +      return;
> > > +    }
> > > +  if (flag_enable_cilkplus && contains_array_notation_expr (incr) && 0)
> > > +    {
> > > +      error_at (start_locus, "array notation expression cannot be used in a "
> > > +		"loop's increment expression");
> > > +      return;
> > > +    }
> >
> > Use %' in the second error here, as you did in the first.
> >
> 
> Removed the 2nd if statement, thus this issue is solved.
> 
> > > diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
> >
> > > +    case ARRAY_NOTATION_REF:
> >
> > Since this is a C-family tree node, I'd think the handling in
> > c-pretty-print.c should suffice, without needing anything in tree-pretty-print.c
> as well.
> 
> Removed!
> 
> >
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Richard Henderson]

Let me start by saying that I think the patch is generally ok, especially 
considering the advice that's already been given.  That said...

> +++ b/gcc/c/c-array-notation.c
> @@ -0,0 +1,3121 @@

So, like, are we going to need to replicate 3000 lines to add array notation to 
c++ too?  How much of this are we going to be able to share?

While I don't think we should make the array notation global, we might be able 
to share the code via c-family/.  The Ideal way I think would be via 
c-gimplify, in which we don't expand the array notation until later.  But even 
if delaying the expansion causes other problems that I havn't thought about, 
just placing the common expansion logic in the shared directory would be better.

> +  for (ii = 0; ii < rank ; ii++)
> +    {
> +      /* This will create the if statement label.  */
> +      if_stmt_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> +                                     void_type_node);
> +      DECL_CONTEXT (if_stmt_label[ii]) = current_function_decl;
> +      DECL_ARTIFICIAL (if_stmt_label[ii]) = 0;
> +      DECL_IGNORED_P (if_stmt_label[ii]) = 1;
> +
> +      /* This label statement will point to the loop body.  */
> +      body_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> +                                  void_type_node);
> +      DECL_CONTEXT (body_label[ii]) = current_function_decl;
> +      DECL_ARTIFICIAL (body_label[ii]) = 0;
> +      DECL_IGNORED_P (body_label[ii]) = 1;
> +      body_label_expr[ii] = build1 (LABEL_EXPR, void_type_node, body_label[ii])
> ;
> +
> +      /* This will create the exit label, i.e. where the while loop will branch
> +        out of.  */
> +      exit_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> +                                  void_type_node);
> +      DECL_CONTEXT (exit_label[ii]) = current_function_decl;
> +      DECL_ARTIFICIAL (exit_label[ii]) = 0;
> +      DECL_IGNORED_P (exit_label[ii]) = 1;
> +      exit_label_expr[ii] = build1 (LABEL_EXPR, void_type_node, exit_label[ii]);
> +    }

Is there any particular reason why you're open-coding the loop expansion 
instead of using existing infrastructure like c_finish_loop?

Yes, the specific example of c_finish_loop goes against the c++ sharing, but 
it's fairly easy to add new interfaces to c-common.h that are implemented in 
the two front ends.



r~

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jakub Jelinek]

On Tue, May 21, 2013 at 07:57:10PM -0700, Richard Henderson wrote:
> >+      /* This will create the if statement label.  */
> >+      if_stmt_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> >+                                     void_type_node);
> >+      DECL_CONTEXT (if_stmt_label[ii]) = current_function_decl;
> >+      DECL_ARTIFICIAL (if_stmt_label[ii]) = 0;
> >+      DECL_IGNORED_P (if_stmt_label[ii]) = 1;
> >+
> >+      /* This label statement will point to the loop body.  */
> >+      body_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> >+                                  void_type_node);
> >+      DECL_CONTEXT (body_label[ii]) = current_function_decl;
> >+      DECL_ARTIFICIAL (body_label[ii]) = 0;
> >+      DECL_IGNORED_P (body_label[ii]) = 1;
> >+      body_label_expr[ii] = build1 (LABEL_EXPR, void_type_node, body_label[ii])
> >;
> >+
> >+      /* This will create the exit label, i.e. where the while loop will branch
> >+        out of.  */
> >+      exit_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> >+                                  void_type_node);
> >+      DECL_CONTEXT (exit_label[ii]) = current_function_decl;
> >+      DECL_ARTIFICIAL (exit_label[ii]) = 0;
> >+      DECL_IGNORED_P (exit_label[ii]) = 1;
> >+      exit_label_expr[ii] = build1 (LABEL_EXPR, void_type_node, exit_label[ii]);
> >+    }
> 
> Is there any particular reason why you're open-coding the loop
> expansion instead of using existing infrastructure like
> c_finish_loop?
> 
> Yes, the specific example of c_finish_loop goes against the c++
> sharing, but it's fairly easy to add new interfaces to c-common.h
> that are implemented in the two front ends.

Furthermore, do you want to generate just normal loop out of it, or
shouldn't we generate a #pragma omp simd loop out of it instead?
Haven't read the spec if array notation guarantees lack of forward/backward
loop dependencies and what are the restrictions on the calls you can do
inside of array notation.  Perhaps the lowering to normal vs. simd loop
could depend on whether all called functions in the expression are
elemental.

	Jakub

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

Hello Richard,
	Thank you for reviewing my code. Please see my responses below.

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Tuesday, May 21, 2013 10:57 PM
> To: Iyer, Balaji V
> Cc: 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> Let me start by saying that I think the patch is generally ok, especially
> considering the advice that's already been given.  That said...
> 
> > +++ b/gcc/c/c-array-notation.c
> > @@ -0,0 +1,3121 @@
> 
> So, like, are we going to need to replicate 3000 lines to add array notation to
> c++ too?  How much of this are we going to be able to share?

The overall function names are same, but the components inside it function differs greatly from C and C++. For example, in C++ I can't use build_modify_expr, but build_x_modify_expr. Also, I need to handle overloaded function types, and that requires further checking. Similarly, the trees are different from C and C++, and so I have to do more checks and handle them differently in C++. In my mind, this seem to be the most straight-forward and legible way to do it.

> 
> While I don't think we should make the array notation global, we might be able
> to share the code via c-family/.  The Ideal way I think would be via c-gimplify, in
> which we don't expand the array notation until later.  But even if delaying the
> expansion causes other problems that I havn't thought about, just placing the
> common expansion logic in the shared directory would be better.
> 
> > +  for (ii = 0; ii < rank ; ii++)
> > +    {
> > +      /* This will create the if statement label.  */
> > +      if_stmt_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> > +                                     void_type_node);
> > +      DECL_CONTEXT (if_stmt_label[ii]) = current_function_decl;
> > +      DECL_ARTIFICIAL (if_stmt_label[ii]) = 0;
> > +      DECL_IGNORED_P (if_stmt_label[ii]) = 1;
> > +
> > +      /* This label statement will point to the loop body.  */
> > +      body_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> > +                                  void_type_node);
> > +      DECL_CONTEXT (body_label[ii]) = current_function_decl;
> > +      DECL_ARTIFICIAL (body_label[ii]) = 0;
> > +      DECL_IGNORED_P (body_label[ii]) = 1;
> > +      body_label_expr[ii] = build1 (LABEL_EXPR, void_type_node,
> > + body_label[ii])
> > ;
> > +
> > +      /* This will create the exit label, i.e. where the while loop will branch
> > +        out of.  */
> > +      exit_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> > +                                  void_type_node);
> > +      DECL_CONTEXT (exit_label[ii]) = current_function_decl;
> > +      DECL_ARTIFICIAL (exit_label[ii]) = 0;
> > +      DECL_IGNORED_P (exit_label[ii]) = 1;
> > +      exit_label_expr[ii] = build1 (LABEL_EXPR, void_type_node, exit_label[ii]);
> > +    }
> 
> Is there any particular reason why you're open-coding the loop expansion
> instead of using existing infrastructure like c_finish_loop?

Before I started my implementation, I did briefly consider using c_finish_loop, but adding multiple expressions got a bit complicated. For example, if I have 2 increment/condition expressions (my initial implementation had it but later on I eliminated that need), I had to create a statement list and then add them using append_to_statement_list() and then pass that into c_finish_loop. Also, if I am not mistaken, I still had to create some of the individual labels when using c_finish_loop(). Overall, I didn't find much code-size decrease, and I found this a bit more straight forward for me. I apologize if I am mistaken.

> 
> Yes, the specific example of c_finish_loop goes against the c++ sharing, but it's
> fairly easy to add new interfaces to c-common.h that are implemented in the
> two front ends.

For C++, I looked into creating FOR_STMT instead. I found this slightly more convenient than c_finish_loop, but decided to stick with this approach for convenience, and I didn't find much code-size decrease.

> 

Are these changes a blocking issue for acceptance into trunk? Or, would it be ok to accept it as-is and then I make these changes at a later date?

> 
> 
> r~

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Richard Henderson]

On 2013-05-21 23:15, Jakub Jelinek wrote:
> Furthermore, do you want to generate just normal loop out of it, or
> shouldn't we generate a #pragma omp simd loop out of it instead?
> Haven't read the spec if array notation guarantees lack of forward/backward
> loop dependencies and what are the restrictions on the calls you can do
> inside of array notation.  Perhaps the lowering to normal vs. simd loop
> could depend on whether all called functions in the expression are
> elemental.

The only overlap allowed is exact, i.e. a[0:5] = a[0:5] + b[1:5],
but not a[0:5] = a[1:5] + b[1:5].  So, yes, I believe that safelen
is essentially unlimited.


r~

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

Hi Jakub,
	Please see my response below.
Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Wednesday, May 22, 2013 2:15 AM
> To: Richard Henderson
> Cc: Iyer, Balaji V; 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On Tue, May 21, 2013 at 07:57:10PM -0700, Richard Henderson wrote:
> > >+      /* This will create the if statement label.  */
> > >+      if_stmt_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> > >+                                     void_type_node);
> > >+      DECL_CONTEXT (if_stmt_label[ii]) = current_function_decl;
> > >+      DECL_ARTIFICIAL (if_stmt_label[ii]) = 0;
> > >+      DECL_IGNORED_P (if_stmt_label[ii]) = 1;
> > >+
> > >+      /* This label statement will point to the loop body.  */
> > >+      body_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> > >+                                  void_type_node);
> > >+      DECL_CONTEXT (body_label[ii]) = current_function_decl;
> > >+      DECL_ARTIFICIAL (body_label[ii]) = 0;
> > >+      DECL_IGNORED_P (body_label[ii]) = 1;
> > >+      body_label_expr[ii] = build1 (LABEL_EXPR, void_type_node,
> > >+ body_label[ii])
> > >;
> > >+
> > >+      /* This will create the exit label, i.e. where the while loop will branch
> > >+        out of.  */
> > >+      exit_label[ii] = build_decl (location, LABEL_DECL, NULL_TREE,
> > >+                                  void_type_node);
> > >+      DECL_CONTEXT (exit_label[ii]) = current_function_decl;
> > >+      DECL_ARTIFICIAL (exit_label[ii]) = 0;
> > >+      DECL_IGNORED_P (exit_label[ii]) = 1;
> > >+      exit_label_expr[ii] = build1 (LABEL_EXPR, void_type_node, exit_label[ii]);
> > >+    }
> >
> > Is there any particular reason why you're open-coding the loop
> > expansion instead of using existing infrastructure like c_finish_loop?
> >
> > Yes, the specific example of c_finish_loop goes against the c++
> > sharing, but it's fairly easy to add new interfaces to c-common.h that
> > are implemented in the two front ends.
> 
> Furthermore, do you want to generate just normal loop out of it, or shouldn't
> we generate a #pragma omp simd loop out of it instead?
> Haven't read the spec if array notation guarantees lack of forward/backward
> loop dependencies and what are the restrictions on the calls you can do inside of
> array notation.  Perhaps the lowering to normal vs. simd loop could depend on
> whether all called functions in the expression are elemental.

What you mentioned is a good performance optimization. It is something I have  planned to do in future. But, the spec does not require the loop (or array notation expansion) to be vectorized.

> 
> 	Jakub

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Wednesday, May 22, 2013 11:30 AM
> To: Jakub Jelinek
> Cc: Iyer, Balaji V; 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 2013-05-21 23:15, Jakub Jelinek wrote:
> > Furthermore, do you want to generate just normal loop out of it, or
> > shouldn't we generate a #pragma omp simd loop out of it instead?
> > Haven't read the spec if array notation guarantees lack of
> > forward/backward loop dependencies and what are the restrictions on
> > the calls you can do inside of array notation.  Perhaps the lowering
> > to normal vs. simd loop could depend on whether all called functions
> > in the expression are elemental.
> 
> The only overlap allowed is exact, i.e. a[0:5] = a[0:5] + b[1:5], but not a[0:5] =
> a[1:5] + b[1:5].  So, yes, I believe that safelen is essentially unlimited.

Yes, the vectorlength/safelen can be the entire loop-size.

> 
> 
> r~

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jeff Law]

On 05/22/2013 09:37 AM, Iyer, Balaji V wrote:
>>>
>> Furthermore, do you want to generate just normal loop out of it, or
>> shouldn't we generate a #pragma omp simd loop out of it instead?
>> Haven't read the spec if array notation guarantees lack of
>> forward/backward loop dependencies and what are the restrictions on
>> the calls you can do inside of array notation.  Perhaps the
>> lowering to normal vs. simd loop could depend on whether all called
>> functions in the expression are elemental.
>
> What you mentioned is a good performance optimization. It is
> something I have  planned to do in future. But, the spec does not
> require the loop (or array notation expansion) to be vectorized.
So if we can represent array notation as an OpenMP SIMD loop and re-use 
the OpenMP code generation, that's a significant win.  I realize the 
OpenMP SIMD stuff is still in-progress, but from a design standpoint 
we'd like to separate out the front-end issues from the code generation 
issues -- with the goal being that we can re-use the OpenMP SIMD path to 
also handle Cilk SIMD & Cilk array notation and possible bits of OpenACC.

Balaji, how feasible is it to rewire the code generation aspects of the 
array notation patch to instead feed into the OpenMP SIMD bits Jakub is 
working on?


Jeff

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Richard Henderson]

On 2013-05-22 08:18, Iyer, Balaji V wrote:
> The overall function names are same, but the components inside it function
> differs greatly from C and C++. For example, in C++ I can't use
> build_modify_expr, but build_x_modify_expr. Also, I need to handle
> overloaded function types, and that requires further checking. Similarly,
> the trees are different from C and C++, and so I have to do more checks and
> handle them differently in C++. In my mind, this seem to be the most
> straight-forward and legible way to do it.

Really, that's surprising:

/* For use from the C common bits.  */
tree
build_modify_expr (location_t /*location*/,
                    tree lhs, tree /*lhs_origtype*/,
                    enum tree_code modifycode,
                    location_t /*rhs_location*/, tree rhs,
                    tree /*rhs_origtype*/)
{
   return cp_build_modify_expr (lhs, modifycode, rhs, tf_warning_or_error);
}

I can definitely see how function overloading could potentially get in the way.

Although by delaying expansion of ARRAY_NOTATION_REF, given that the TREE_TYPE 
of the ARRAY_NOTATION_REF is not some complex artificial slice type but the 
element type of the array, I wonder how much of the overloaded function 
selection would just happen automatically?

Which reminds me: What is ARRAY_NOTATION_TYPE for?  Isn't it always the same as 
the TREE_TYPE of the ARRAY_NOTATION_REF?

> Before I started my implementation, I did briefly consider using
> c_finish_loop, but adding multiple expressions got a bit complicated. For
> example, if I have 2 increment/condition expressions (my initial
> implementation had it but later on I eliminated that need), I had to create
> a statement list and then add them using append_to_statement_list() and then
> pass that into c_finish_loop. Also, if I am not mistaken, I still had to
> create some of the individual labels when using c_finish_loop(). Overall, I
> didn't find much code-size decrease, and I found this a bit more straight
> forward for me. I apologize if I am mistaken.

Well, append_to_statement_list etc was certainly not needed.  Mirroring exactly 
how you'd write it in C, with a comma operator also works.  That's a 
COMPOUND_EXPR, fyi.

There are continue/break label arguments to c_finish_loop, but they may be (and 
often are) null.  They will only be non-null if the relevant statements 
actually exist inside the loop.  C.f. c_finish_bc_stmt and c_break_label.

You're right about it not saving *that* much code, but the point is more about 
if we make improvements to normal C loop representation -- such as to support 
#pragma simd -- then we want to be able to take advantage of that here.

> Are these changes a blocking issue for acceptance into trunk? Or, would it
> be ok to accept it as-is and then I make these changes at a later date?

I'm leaning to honoring the advice you were given last year and accepting the 
patch more or less in its current form.


r~

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jeff Law
> Sent: Wednesday, May 22, 2013 12:20 PM
> To: Iyer, Balaji V
> Cc: Jakub Jelinek; Richard Henderson; 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-
> patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/22/2013 09:37 AM, Iyer, Balaji V wrote:
> >>>
> >> Furthermore, do you want to generate just normal loop out of it, or
> >> shouldn't we generate a #pragma omp simd loop out of it instead?
> >> Haven't read the spec if array notation guarantees lack of
> >> forward/backward loop dependencies and what are the restrictions on
> >> the calls you can do inside of array notation.  Perhaps the lowering
> >> to normal vs. simd loop could depend on whether all called functions
> >> in the expression are elemental.
> >
> > What you mentioned is a good performance optimization. It is something
> > I have  planned to do in future. But, the spec does not require the
> > loop (or array notation expansion) to be vectorized.
> So if we can represent array notation as an OpenMP SIMD loop and re-use the
> OpenMP code generation, that's a significant win.  I realize the OpenMP SIMD
> stuff is still in-progress, but from a design standpoint we'd like to separate out
> the front-end issues from the code generation issues -- with the goal being that
> we can re-use the OpenMP SIMD path to also handle Cilk SIMD & Cilk array
> notation and possible bits of OpenACC.
> 
> Balaji, how feasible is it to rewire the code generation aspects of the array
> notation patch to instead feed into the OpenMP SIMD bits Jakub is working on?

Hi Jeff,
	Yes, converting the array notation expansion to #pragma simd (or #pragma omp simd) trees will be beneficial performance wise. But, it will require semi-significant re-write and this can destabilize a currently stable implementation. IMHO, for now I would like to keep array notation as is and let us see how it interacts with other flavors of gcc backends. At the moment, we do not see any reason for instability in this, but we have not done any significant testing on non x86 backends. After OMP 4 hits the main-line and everything is stable, we could proceed as you indicated.

Thanks,

Balaji V. Iyer.


> 
> 
> Jeff

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jeff Law]

On 05/22/2013 01:13 PM, Iyer, Balaji V wrote:
>
> Hi Jeff, Yes, converting the array notation expansion to #pragma simd
> (or #pragma omp simd) trees will be beneficial performance wise. But,
> it will require semi-significant re-write and this can destabilize a
> currently stable implementation. IMHO, for now I would like to keep
> array notation as is and let us see how it interacts with other
> flavors of gcc backends. At the moment, we do not see any reason for
> instability in this, but we have not done any significant testing on
> non x86 backends. After OMP 4 hits the main-line and everything is
> stable, we could proceed as you indicated.
I was thinking more from maintenance standpoint.  It seems kindof silly 
to have multiple blobs of code generating loops.  The fact that it gets 
us vectorized code more often is just icing on the cake.

This is certainly something that I want to see happen, though I'll defer 
to Richard, Joseph, Aldy, etc on the timing of that change.

jeff

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Wednesday, May 22, 2013 1:34 PM
> To: Iyer, Balaji V
> Cc: 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 2013-05-22 08:18, Iyer, Balaji V wrote:
> > The overall function names are same, but the components inside it
> > function differs greatly from C and C++. For example, in C++ I can't
> > use build_modify_expr, but build_x_modify_expr. Also, I need to handle
> > overloaded function types, and that requires further checking.
> > Similarly, the trees are different from C and C++, and so I have to do
> > more checks and handle them differently in C++. In my mind, this seem
> > to be the most straight-forward and legible way to do it.
> 
> Really, that's surprising:
> 
> /* For use from the C common bits.  */
> tree
> build_modify_expr (location_t /*location*/,
>                     tree lhs, tree /*lhs_origtype*/,
>                     enum tree_code modifycode,
>                     location_t /*rhs_location*/, tree rhs,
>                     tree /*rhs_origtype*/) {
>    return cp_build_modify_expr (lhs, modifycode, rhs, tf_warning_or_error); }
> 
> I can definitely see how function overloading could potentially get in the way.

There are couple other ones such as build_x_unary_op, build_x_conditional_expr which does not have such a mapping that I know of.

> 
> Although by delaying expansion of ARRAY_NOTATION_REF, given that the
> TREE_TYPE of the ARRAY_NOTATION_REF is not some complex artificial slice
> type but the element type of the array, I wonder how much of the overloaded
> function selection would just happen automatically?
> 
> Which reminds me: What is ARRAY_NOTATION_TYPE for?  Isn't it always the
> same as the TREE_TYPE of the ARRAY_NOTATION_REF?

Yes, they are both the same. A while back, I found a couple corner cases where the TREE_TYPE of the array notations inside __sec_reduce functions that was getting changed. This is a storage location that will be untouched so that I can get the original type. 

> 
> > Before I started my implementation, I did briefly consider using
> > c_finish_loop, but adding multiple expressions got a bit complicated.
> > For example, if I have 2 increment/condition expressions (my initial
> > implementation had it but later on I eliminated that need), I had to
> > create a statement list and then add them using
> > append_to_statement_list() and then pass that into c_finish_loop.
> > Also, if I am not mistaken, I still had to create some of the
> > individual labels when using c_finish_loop(). Overall, I didn't find
> > much code-size decrease, and I found this a bit more straight forward for me. I
> apologize if I am mistaken.
> 
> Well, append_to_statement_list etc was certainly not needed.  Mirroring exactly
> how you'd write it in C, with a comma operator also works.  That's a
> COMPOUND_EXPR, fyi.
> 
> There are continue/break label arguments to c_finish_loop, but they may be
> (and often are) null.  They will only be non-null if the relevant statements
> actually exist inside the loop.  C.f. c_finish_bc_stmt and c_break_label.
> 
> You're right about it not saving *that* much code, but the point is more about if
> we make improvements to normal C loop representation -- such as to support
> #pragma simd -- then we want to be able to take advantage of that here.

OK. I see your point. I will look into changing them.

> 
> > Are these changes a blocking issue for acceptance into trunk? Or,
> > would it be ok to accept it as-is and then I make these changes at a later date?
> 
> I'm leaning to honoring the advice you were given last year and accepting the
> patch more or less in its current form.

Thank you!

> 
> 
> r~

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Joseph S. Myers]

On Wed, 22 May 2013, Jeff Law wrote:

> So if we can represent array notation as an OpenMP SIMD loop and re-use the
> OpenMP code generation, that's a significant win.  I realize the OpenMP SIMD
> stuff is still in-progress, but from a design standpoint we'd like to separate
> out the front-end issues from the code generation issues -- with the goal
> being that we can re-use the OpenMP SIMD path to also handle Cilk SIMD & Cilk
> array notation and possible bits of OpenACC.

Regarding commonality between OpenMP and Cilk, note also the new C 
Parallel Language Extensions WG14 study group chaired by Clark Nelson and 
aiming to propose a standard set of C extensions for parallel programming, 
announced yesterday on the WG14 reflector.  I don't know if anyone here is 
intending to be involved in that.

http://www.open-std.org/mailman/listinfo/cplex

I'm OK with the array notation patch as it stands now, although certainly 
as discussed today there is still some scope for further improvements.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Richard Henderson]

On 05/22/2013 02:25 PM, Iyer, Balaji V wrote:
> Yes, they are both the same. A while back, I found a couple corner cases
> where the TREE_TYPE of the array notations inside __sec_reduce functions
> that was getting changed. This is a storage location that will be untouched
> so that I can get the original type.

Do you have test cases for these tree type vs sec_reduce changes?

I'd like to understand how these types could get changed.  This sounds like a
bug elsewhere, possibly wrt incorrectly shared trees, and that this field is a
hack and should be eliminated.


r~

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Richard Henderson
> Sent: Wednesday, May 22, 2013 6:18 PM
> To: Iyer, Balaji V
> Cc: 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/22/2013 02:25 PM, Iyer, Balaji V wrote:
> > Yes, they are both the same. A while back, I found a couple corner
> > cases where the TREE_TYPE of the array notations inside __sec_reduce
> > functions that was getting changed. This is a storage location that
> > will be untouched so that I can get the original type.
> 
> Do you have test cases for these tree type vs sec_reduce changes?
> 
> I'd like to understand how these types could get changed.  This sounds like a bug
> elsewhere, possibly wrt incorrectly shared trees, and that this field is a hack and
> should be eliminated.

I am currently looking to eliminate this also. We had a different representation for built-in functions (they weren't truly builtin functions and Aldy helped me fix that issue) and the new representation might have gotten rid of this bug.  I will let you know as soon as I find something.

> 
> 
> r~

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jeff Law]

On 05/22/2013 03:58 PM, Joseph S. Myers wrote:
>
> Regarding commonality between OpenMP and Cilk, note also the new C
> Parallel Language Extensions WG14 study group chaired by Clark Nelson and
> aiming to propose a standard set of C extensions for parallel programming,
> announced yesterday on the WG14 reflector.  I don't know if anyone here is
> intending to be involved in that.
>
> http://www.open-std.org/mailman/listinfo/cplex
>
> I'm OK with the array notation patch as it stands now, although certainly
> as discussed today there is still some scope for further improvements.
I'm aware of efforts to standardize the Cilk+ style extensions for ISO 
C.  Is the new study group effectively that effort or something different?


jeff

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Wednesday, May 22, 2013 10:11 PM
> To: Joseph S. Myers
> Cc: Iyer, Balaji V; Jakub Jelinek; Richard Henderson; 'Aldy Hernandez'; 'gcc-
> patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/22/2013 03:58 PM, Joseph S. Myers wrote:
> >
> > Regarding commonality between OpenMP and Cilk, note also the new C
> > Parallel Language Extensions WG14 study group chaired by Clark Nelson
> > and aiming to propose a standard set of C extensions for parallel
> > programming, announced yesterday on the WG14 reflector.  I don't know
> > if anyone here is intending to be involved in that.
> >
> > http://www.open-std.org/mailman/listinfo/cplex
> >
> > I'm OK with the array notation patch as it stands now, although
> > certainly as discussed today there is still some scope for further improvements.
> I'm aware of efforts to standardize the Cilk+ style extensions for ISO C.  Is the
> new study group effectively that effort or something different?

I will inquire about this and get back as soon as I get some response.

Thanks,

Balaji V. Iyer.

> 
> 
> jeff

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Wednesday, May 22, 2013 10:11 PM
> To: Joseph S. Myers
> Cc: Iyer, Balaji V; Jakub Jelinek; Richard Henderson; 'Aldy Hernandez'; 'gcc-
> patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/22/2013 03:58 PM, Joseph S. Myers wrote:
> >
> > Regarding commonality between OpenMP and Cilk, note also the new C
> > Parallel Language Extensions WG14 study group chaired by Clark Nelson
> > and aiming to propose a standard set of C extensions for parallel
> > programming, announced yesterday on the WG14 reflector.  I don't know
> > if anyone here is intending to be involved in that.
> >
> > http://www.open-std.org/mailman/listinfo/cplex
> >
> > I'm OK with the array notation patch as it stands now, although
> > certainly as discussed today there is still some scope for further improvements.
> I'm aware of efforts to standardize the Cilk+ style extensions for ISO C.  Is the
> new study group effectively that effort or something different?

Yes, it is part of the Cilk Plus standardization effort for C.

> 
> 
> jeff

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

[-- Attachment #1: Type: text/plain, Size: 11983 bytes --]

Hello Richard et al.,
	Attached, please find a fixed patch. I have done the following changes:

1. Used the c_finish_loop (...) function instead of building the loop myself
2. Got rid of ARRAY_NOTATION_TYPE and just used TREE_TYPE ().

It is passing all the regression tests and not breaking/passing any other tests that were not already breaking/passing in the trunk.

Here are the ChangeLog Entries:

gcc/ChangeLog
013-05-23  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * doc/extend.texi (C Extensions): Added documentation about Cilk Plus
        array notation built-in reduction functions.
        * doc/passes.texi (Passes): Added documentation about changes done
        for Cilk Plus.
        * doc/invoke.texi (C Dialect Options): Added documentation about
        the -fcilkplus flag.
        * doc/generic.texi (Storage References): Added documentation for
        ARRAY_NOTATION_REF storage.
        * Makefile.in (C_COMMON_OBJS): Added c-family/array-notation-common.o.
        (BUILTINS_DEF): Depend on cilkplus.def.
        * builtins.def: Include cilkplus.def.  Define DEF_CILKPLUS_BUILTIN.
        * builtin-types.def: Define BT_FN_INT_PTR_PTR_PTR.
        * cilkplus.def: New file.

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

        * c-common.c (c_define_builtins): When cilkplus is enabled, the
        function array_notation_init_builtins is called.
        (c_common_init_ts): Added ARRAY_NOTATION_REF as typed.
        * c-common.def (ARRAY_NOTATION_REF): New tree.
        * c-common.h (build_array_notation_expr): New function declaration.
        (build_array_notation_ref): Likewise.
        (extract_sec_implicit_index_arg): New extern declaration.
        (is_sec_implicit_index_fn): Likewise.
        (ARRAY_NOTATION_CHECK): New define.
        (ARRAY_NOTATION_ARRAY): Likewise.
        (ARRAY_NOTATION_START): Likewise.
        (ARRAY_NOTATION_LENGTH): Likewise.
        (ARRAY_NOTATION_STRIDE): Likewise.
        * c-pretty-print.c (pp_c_postifix_expression): Added a new case for
        ARRAY_NOTATION_REF.
        (pp_c_expression): Likewise.
        * c.opt (flag_enable_cilkplus): New flag.
        * array-notation-common.c: New file.

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

        * c-typeck.c (build_array_ref): Added a check to see if array's
        index is greater than one.  If true, then emit an error.
        (build_function_call_vec): Exclude error reporting and checking
        for builtin array-notation functions.
        (convert_arguments): Likewise.
        (c_finish_return): Added a check for array notations as a return
        expression.  If true, then emit an error.
        (c_finish_loop): Added a check for array notations in a loop
        condition.  If true then emit an error.
        (lvalue_p): Added a ARRAY_NOTATION_REF case.
        (build_binary_op): Added a check for array notation expr inside
        op1 and op0.  If present, we call another function to find correct
        type.
        * 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_direct_declarator_inner): Added a check for array
        notation.
        (c_parser_compound_statement): Added a check for array notation in
        a stmt.  If one is present, then expand array notation expr.
        (c_parser_if_statement): Likewise.
        (c_parser_switch_statement): Added a check for array notations in
        a switch statement's condition.  If true, then output an error.
        (c_parser_while_statement): Similarly, but for a while.
        (c_parser_do_statement): Similarly, but for a do-while.
        (c_parser_for_statement): Similarly, but for a for-loop.
        (c_parser_unary_expression): Check if array notation is used in a
        pre-increment or pre-decrement expression.  If true, then expand
        them.
        (c_parser_array_notation): New function.
        * c-array-notation.c: New file.
        * c-tree.h (is_cilkplus_reduce_builtin): Protoize.

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

        * gcc.dg/cilk-plus/array_notation/compile/array_test2.c: New test.
        * gcc.dg/cilk-plus/array_notation/compile/array_test1.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/compile/array_test_ND.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/compile/builtin_func_double.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/compile/builtin_func_double2.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/compile/gather_scatter.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/compile/if_test.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/compile/sec_implicit_ex.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/dimensionless-arrays.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/fp_triplet_values.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/gather-scatter.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/misc.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/parser_errors.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/parser_errors2.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/parser_errors3.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/parser_errors4.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch2.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch3.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/sec_implicit.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/sec_implicit2.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/sec_reduce_max_min_ind.c:
        Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/tst_lngth.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/vla.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/an-if.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/array_test1.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/array_test2.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/array_test_ND.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/builtin_fn_custom.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/builtin_fn_mutating.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/builtin_func_double.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/builtin_func_double2.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/comma_exp.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/conditional.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/exec-once.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/exec-once2.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/gather_scatter.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/if_test.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/n-ptr-test.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/sec_implicit_ex.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/side-effects-1.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/test_builtin_return.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/test_sec_limits.c: Ditto.
        * gcc.dg/cilk-plus/array_notation/execute/cilkplus_AN_c_execute.exp:
        New script.
        * gcc.dg/cilk-plus/array_notation/compile/cilkplus_AN_c_compile.exp:
        Ditto.
        * gcc.dg/cilk-plus/array_notation/errors/cilkplus_AN_c_errors.exp:
        Ditto.

Thanks,

Balaji V. Iyer.

PS. I used "Ditto" instead of "Likewise" for the testsuite ChangeLog entries so that I can fix it all in 1 line. I hope that's OK.


> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Wednesday, May 22, 2013 1:34 PM
> To: Iyer, Balaji V
> Cc: 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 2013-05-22 08:18, Iyer, Balaji V wrote:
> > The overall function names are same, but the components inside it
> > function differs greatly from C and C++. For example, in C++ I can't
> > use build_modify_expr, but build_x_modify_expr. Also, I need to handle
> > overloaded function types, and that requires further checking.
> > Similarly, the trees are different from C and C++, and so I have to do
> > more checks and handle them differently in C++. In my mind, this seem
> > to be the most straight-forward and legible way to do it.
> 
> Really, that's surprising:
> 
> /* For use from the C common bits.  */
> tree
> build_modify_expr (location_t /*location*/,
>                     tree lhs, tree /*lhs_origtype*/,
>                     enum tree_code modifycode,
>                     location_t /*rhs_location*/, tree rhs,
>                     tree /*rhs_origtype*/) {
>    return cp_build_modify_expr (lhs, modifycode, rhs, tf_warning_or_error); }
> 
> I can definitely see how function overloading could potentially get in the way.
> 
> Although by delaying expansion of ARRAY_NOTATION_REF, given that the
> TREE_TYPE of the ARRAY_NOTATION_REF is not some complex artificial slice
> type but the element type of the array, I wonder how much of the overloaded
> function selection would just happen automatically?
> 
> Which reminds me: What is ARRAY_NOTATION_TYPE for?  Isn't it always the
> same as the TREE_TYPE of the ARRAY_NOTATION_REF?
> 
> > Before I started my implementation, I did briefly consider using
> > c_finish_loop, but adding multiple expressions got a bit complicated.
> > For example, if I have 2 increment/condition expressions (my initial
> > implementation had it but later on I eliminated that need), I had to
> > create a statement list and then add them using
> > append_to_statement_list() and then pass that into c_finish_loop.
> > Also, if I am not mistaken, I still had to create some of the
> > individual labels when using c_finish_loop(). Overall, I didn't find
> > much code-size decrease, and I found this a bit more straight forward for me. I
> apologize if I am mistaken.
> 
> Well, append_to_statement_list etc was certainly not needed.  Mirroring exactly
> how you'd write it in C, with a comma operator also works.  That's a
> COMPOUND_EXPR, fyi.
> 
> There are continue/break label arguments to c_finish_loop, but they may be
> (and often are) null.  They will only be non-null if the relevant statements
> actually exist inside the loop.  C.f. c_finish_bc_stmt and c_break_label.
> 
> You're right about it not saving *that* much code, but the point is more about if
> we make improvements to normal C loop representation -- such as to support
> #pragma simd -- then we want to be able to take advantage of that here.
> 
> > Are these changes a blocking issue for acceptance into trunk? Or,
> > would it be ok to accept it as-is and then I make these changes at a later date?
> 
> I'm leaning to honoring the advice you were given last year and accepting the
> patch more or less in its current form.
> 
> 
> r~

[-- Attachment #2: patch.txt.bz2 --]
[-- Type: application/octet-stream, Size: 32666 bytes --]

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jakub Jelinek]

On Thu, May 23, 2013 at 06:27:04PM +0000, Iyer, Balaji V wrote:
> gcc/testsuite/ChangeLog
> 2013-05-23  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> 
>         * gcc.dg/cilk-plus/array_notation/compile/array_test2.c: New test.

I have concerns about the test locations, to me this looks way too deep
tree, whether something is a compile test, or compile test expecting errors
or runtime test is easily determined by { dg-do compile } vs. { dg-do run }
and presence or lack of { dg-error ... } comments.  So IMHO that level
should be left out, plus I'd say the array_notation/ level is unnecessary as
well, just put everything into c-c++-common/cilk-plus/an-*.c
(except for tests that aren't going to be usable for C++, those can stay in
gcc.dg/cilk-plus/an-*.c).  Then gcc.dg/cilk-plus/*.exp would just ensure
that tests from that directory are run and also from c-c++-common/ and later
on the same would happen in g++.dg/cilk-plus/.  In the future when you will
need to link against runtime library cilk-plus.exp would just arrange for
that to be added to LD_LIBRARY_PATH, -L.../ etc.

>         * gcc.dg/cilk-plus/array_notation/compile/array_test1.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/compile/array_test_ND.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/compile/builtin_func_double.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/compile/builtin_func_double2.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/compile/gather_scatter.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/compile/if_test.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/compile/sec_implicit_ex.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/dimensionless-arrays.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/fp_triplet_values.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/gather-scatter.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/misc.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/parser_errors.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/parser_errors2.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/parser_errors3.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/parser_errors4.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch2.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch3.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/sec_implicit.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/sec_implicit2.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/sec_reduce_max_min_ind.c:
>         Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/tst_lngth.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/vla.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/an-if.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/array_test1.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/array_test2.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/array_test_ND.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/builtin_fn_custom.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/builtin_fn_mutating.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/builtin_func_double.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/builtin_func_double2.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/comma_exp.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/conditional.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/exec-once.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/exec-once2.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/gather_scatter.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/if_test.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/n-ptr-test.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/sec_implicit_ex.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/side-effects-1.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/test_builtin_return.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/test_sec_limits.c: Ditto.
>         * gcc.dg/cilk-plus/array_notation/execute/cilkplus_AN_c_execute.exp:
>         New script.
>         * gcc.dg/cilk-plus/array_notation/compile/cilkplus_AN_c_compile.exp:
>         Ditto.
>         * gcc.dg/cilk-plus/array_notation/errors/cilkplus_AN_c_errors.exp:
>         Ditto.

	Jakub

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Aldy Hernandez]

On 05/23/13 14:03, Jakub Jelinek wrote:
> On Thu, May 23, 2013 at 06:27:04PM +0000, Iyer, Balaji V wrote:
>> gcc/testsuite/ChangeLog
>> 2013-05-23  Balaji V. Iyer  <balaji.v.iyer@intel.com>
>>
>>          * gcc.dg/cilk-plus/array_notation/compile/array_test2.c: New test.
>
> I have concerns about the test locations, to me this looks way too deep
> tree, whether something is a compile test, or compile test expecting errors
> or runtime test is easily determined by { dg-do compile } vs. { dg-do run }
> and presence or lack of { dg-error ... } comments.  So IMHO that level
> should be left out, plus I'd say the array_notation/ level is unnecessary as
> well, just put everything into c-c++-common/cilk-plus/an-*.c
> (except for tests that aren't going to be usable for C++, those can stay in
> gcc.dg/cilk-plus/an-*.c).  Then gcc.dg/cilk-plus/*.exp would just ensure
> that tests from that directory are run and also from c-c++-common/ and later
> on the same would happen in g++.dg/cilk-plus/.  In the future when you will
> need to link against runtime library cilk-plus.exp would just arrange for
> that to be added to LD_LIBRARY_PATH, -L.../ etc.

For the record, I agree.

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Richard Henderson]

On 05/23/2013 11:27 AM, Iyer, Balaji V wrote:
> Hello Richard et al.,
> 	Attached, please find a fixed patch. I have done the following changes:
> 
> 1. Used the c_finish_loop (...) function instead of building the loop myself
> 2. Got rid of ARRAY_NOTATION_TYPE and just used TREE_TYPE ().
> 
> It is passing all the regression tests and not breaking/passing any other
> tests that were not already breaking/passing in the trunk.

Good to know A_N_T wasn't needed.  You failed to remove it completely though:

> +/* 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 array notation's type.  */
> +DEFTREECODE (ARRAY_NOTATION_REF, "array_notation_ref", tcc_reference, 5) 

> +@item ARRAY_NOTATION_REF
> +These nodes represent array notation expressions that are part of the
> +Cilk Plus language extensions (enabled by the @option{-fcilkplus}
> +flag).  The first operand is the array.  The second, third, and fourth
> +operands are the start-index, number of elements accessed (also called
> +length) and the stride, respectively.  The fifth operand holds the
> +array type.  Near the end of the parsing stage, these array notations
> +are broken up into array references (@code{ARRAY_REF}) enclosed inside
> +a loop iterating from 0 to the number of elements accessed.
> +

This really shouldn't go in generic.texi, because it's not a generic tree code.
 AFAIK, we have no texi documentation for the front-end specific stuff.

Otherwise I see nothing wrong with the patch vs compiler proper.


r~

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Thursday, May 23, 2013 3:04 PM
> To: Iyer, Balaji V
> Cc: Richard Henderson; 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On Thu, May 23, 2013 at 06:27:04PM +0000, Iyer, Balaji V wrote:
> > gcc/testsuite/ChangeLog
> > 2013-05-23  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> >
> >         * gcc.dg/cilk-plus/array_notation/compile/array_test2.c: New test.
> 
> I have concerns about the test locations, to me this looks way too deep tree,
> whether something is a compile test, or compile test expecting errors or runtime
> test is easily determined by { dg-do compile } vs. { dg-do run } and presence or
> lack of { dg-error ... } comments.  So IMHO that level should be left out, plus I'd
> say the array_notation/ level is unnecessary as well, just put everything into c-
> c++-common/cilk-plus/an-*.c (except for tests that aren't going to be usable for
> C++, those can stay in gcc.dg/cilk-plus/an-*.c).  Then gcc.dg/cilk-plus/*.exp
> would just ensure that tests from that directory are run and also from c-c++-
> common/ and later on the same would happen in g++.dg/cilk-plus/.  In the
> future when you will need to link against runtime library cilk-plus.exp would just
> arrange for that to be added to LD_LIBRARY_PATH, -L.../ etc.

Hi Jakub & Aldy,
	There are a couple reasons why I picked this hierarchy. I looked at gcc-c-torture directory and it has compile, execute etc. This is why I had execute, compile and errors directory. Also, we are planning to have some hybrid tests that will add array notation + cilk keywords, array notation + pragma simd, etc. Yes, I can see the deeply buried issue, but I once had long file names (~25-30 characters) that tells what kind of tests (when we first opened the branch) they are and someone in the mailing list complained that the file names were long and suggested that I use directories instead. If it is OK with you both I would like to keep this hierarchy

Thanks,

Balaji V. Iyer.

> 
> >         * gcc.dg/cilk-plus/array_notation/compile/array_test1.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/compile/array_test_ND.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/compile/builtin_func_double.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/compile/builtin_func_double2.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/compile/gather_scatter.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/compile/if_test.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/compile/sec_implicit_ex.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/dimensionless-arrays.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/fp_triplet_values.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/gather-scatter.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/misc.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/parser_errors.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/parser_errors2.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/parser_errors3.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/parser_errors4.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch2.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/rank_mismatch3.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/sec_implicit.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/sec_implicit2.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/sec_reduce_max_min_ind.c:
> >         Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/tst_lngth.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/vla.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/an-if.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/array_test1.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/array_test2.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/array_test_ND.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/builtin_fn_custom.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/builtin_fn_mutating.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/builtin_func_double.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/builtin_func_double2.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/comma_exp.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/conditional.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/exec-once.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/exec-once2.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/gather_scatter.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/if_test.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/n-ptr-test.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/sec_implicit_ex.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/side-effects-1.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/test_builtin_return.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/test_sec_limits.c: Ditto.
> >         * gcc.dg/cilk-plus/array_notation/execute/cilkplus_AN_c_execute.exp:
> >         New script.
> >         * gcc.dg/cilk-plus/array_notation/compile/cilkplus_AN_c_compile.exp:
> >         Ditto.
> >         * gcc.dg/cilk-plus/array_notation/errors/cilkplus_AN_c_errors.exp:
> >         Ditto.
> 
> 	Jakub

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jeff Law]

On 05/23/2013 02:38 PM, Iyer, Balaji V wrote:
>
> Hi Jakub & Aldy, There are a couple reasons why I picked this
> hierarchy. I looked at gcc-c-torture directory and it has compile,
> execute etc. This is why I had execute, compile and errors directory.
> Also, we are planning to have some hybrid tests that will add array
> notation + cilk keywords, array notation + pragma simd, etc. Yes, I
> can see the deeply buried issue, but I once had long file names
> (~25-30 characters) that tells what kind of tests (when we first
> opened the branch) they are and someone in the mailing list
> complained that the file names were long and suggested that I use
> directories instead. If it is OK with you both I would like to keep
> this hierarchy
c-torture is the oldest of our testing frameworks -- it goes back to 
separate c-torture testing releases from Tege.  IIRC those were 
originally just shell scripts which were invoked on every file in the 
directory.  Thus every file in a particular directory had to have the 
same characteristics (ie, it must compile, compile & run, not compile).

I'm guessing Aldy & Jakub want this stuff done in the dg-torture 
framework which would flatten out one of the directories.

As someone (rth?) mentioned elsewhere, we have some tests that can and 
should be shared between the C & C++ front-end.  Most if not all of 
these seem to fall into that category.   I'd separate them into
common to c/c++ (in the c-c++-common directory), c specific and c++ 
specific which would go into the gcc.dg and g++.dg directories.

I'd squash out the cilk-plus directory.  While this is currently an 
extension, this may ultimately end up being part of ISO-C rather than 
being an extension.

Jeff

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Joseph S. Myers]

On Wed, 22 May 2013, Jeff Law wrote:

> On 05/22/2013 03:58 PM, Joseph S. Myers wrote:
> > 
> > Regarding commonality between OpenMP and Cilk, note also the new C
> > Parallel Language Extensions WG14 study group chaired by Clark Nelson and
> > aiming to propose a standard set of C extensions for parallel programming,
> > announced yesterday on the WG14 reflector.  I don't know if anyone here is
> > intending to be involved in that.
> > 
> > http://www.open-std.org/mailman/listinfo/cplex
> > 
> > I'm OK with the array notation patch as it stands now, although certainly
> > as discussed today there is still some scope for further improvements.
> I'm aware of efforts to standardize the Cilk+ style extensions for ISO C.  Is
> the new study group effectively that effort or something different?

"This proposal is expected to combine the best ideas from Cilk and OpenMP, 
two of the most widely-used and well-established parallel language 
extensions for the C language family.".  See page 10 of the draft Delft 
minutes <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1693.pdf> for 
more discussion.

-- 
Joseph S. Myers
joseph@codesourcery.com

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Thursday, May 23, 2013 4:52 PM
> To: Iyer, Balaji V
> Cc: Jakub Jelinek; Richard Henderson; 'Joseph S. Myers'; 'Aldy Hernandez'; 'gcc-
> patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/23/2013 02:38 PM, Iyer, Balaji V wrote:
> >
> > Hi Jakub & Aldy, There are a couple reasons why I picked this
> > hierarchy. I looked at gcc-c-torture directory and it has compile,
> > execute etc. This is why I had execute, compile and errors directory.
> > Also, we are planning to have some hybrid tests that will add array
> > notation + cilk keywords, array notation + pragma simd, etc. Yes, I
> > can see the deeply buried issue, but I once had long file names
> > (~25-30 characters) that tells what kind of tests (when we first
> > opened the branch) they are and someone in the mailing list complained
> > that the file names were long and suggested that I use directories
> > instead. If it is OK with you both I would like to keep this hierarchy
> c-torture is the oldest of our testing frameworks -- it goes back to separate c-
> torture testing releases from Tege.  IIRC those were originally just shell scripts
> which were invoked on every file in the directory.  Thus every file in a particular
> directory had to have the same characteristics (ie, it must compile, compile &
> run, not compile).
> 
> I'm guessing Aldy & Jakub want this stuff done in the dg-torture framework
> which would flatten out one of the directories.
> 
> As someone (rth?) mentioned elsewhere, we have some tests that can and
> should be shared between the C & C++ front-end.  Most if not all of
> these seem to fall into that category.   I'd separate them into
> common to c/c++ (in the c-c++-common directory), c specific and c++ specific
> which would go into the gcc.dg and g++.dg directories.
> 
> I'd squash out the cilk-plus directory.  While this is currently an extension, this
> may ultimately end up being part of ISO-C rather than being an extension.

If I put things in c-c++-common, how do I test the code with different flags (I didn't see any .exp file there)? For example, how can I test if it works with "-O2" and then have another test for "-O2 -g" etc.? Do I just use multiple "dg-options" in my code? The way I have it right now, it uses several flags, and tries them in different combinations. I am if this is a trivial question, I am not very familiar with DejaGNU framework and I went through GCC and DejaGNU manual a while back and I couldn't find an answer for this.

Thanks,

Balaji V. Iyer.


> 
> Jeff

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Mike Stump]

On May 23, 2013, at 2:08 PM, "Iyer, Balaji V" <balaji.v.iyer@intel.com> wrote:
> If I put things in c-c++-common, how do I test the code with different flags (I didn't see any .exp file there)? For example, how can I test if it works with "-O2" and then have another test for "-O2 -g" etc.? Do I just use multiple "dg-options" in my code? The way I have it right now, it uses several flags, and tries them in different combinations. I am if this is a trivial question, I am not very familiar with DejaGNU framework and I went through GCC and DejaGNU manual a while back and I couldn't find an answer for this.

If you have 30+ tests, I'd do a .exp file and a new subdirectory.  If you only plan to have a few, you can

test-1.c:
// option -O2

test-1g.c
// option -O2 -g
#include "test-1.c"

I'm not worried about a new subdirectory as much as others are; indeed, I think I prefer it.  I kinda like the idea that one can run cilkplus_AN_c_compile.exp and feature test the compiler.

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

[I included Jeff Law also in this conversation]

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Aldy Hernandez
> Sent: Thursday, May 23, 2013 3:23 PM
> To: Jakub Jelinek
> Cc: Iyer, Balaji V; Richard Henderson; 'Joseph S. Myers'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/23/13 14:03, Jakub Jelinek wrote:
> > On Thu, May 23, 2013 at 06:27:04PM +0000, Iyer, Balaji V wrote:
> >> gcc/testsuite/ChangeLog
> >> 2013-05-23  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> >>
> >>          * gcc.dg/cilk-plus/array_notation/compile/array_test2.c: New test.
> >
> > I have concerns about the test locations, to me this looks way too
> > deep tree, whether something is a compile test, or compile test
> > expecting errors or runtime test is easily determined by { dg-do
> > compile } vs. { dg-do run } and presence or lack of { dg-error ... }
> > comments.  So IMHO that level should be left out, plus I'd say the
> > array_notation/ level is unnecessary as well, just put everything into
> > c-c++-common/cilk-plus/an-*.c (except for tests that aren't going to
> > be usable for C++, those can stay in gcc.dg/cilk-plus/an-*.c).  Then
> > gcc.dg/cilk-plus/*.exp would just ensure that tests from that
> > directory are run and also from c-c++-common/ and later on the same
> > would happen in g++.dg/cilk-plus/.  In the future when you will need
> > to link against runtime library cilk-plus.exp would just arrange for that to be
> added to LD_LIBRARY_PATH, -L.../ etc.
> 
> For the record, I agree.

I got all your responses and, if I remove the compile, execute and errors directories but keep cilk-plus and array notation, maybe even abbreviate array notation to "an", (in future cilk keywords to "ck", pragma simd to "ps" and  elemental function to "ef"), will that be OK with everyone? We removed one directory and moved 3 scripts to 1, and with abbreviated subdirectories the ChangeLog lines won't be long. I really like to put all the cilk-plus tests in one location for C compiler so that if anyone wants to get them I can point to one location.

Thanks,

Balaji V. Iyer.

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jeff Law]

On 05/23/2013 06:42 PM, Iyer, Balaji V wrote:
>
> I got all your responses and, if I remove the compile, execute and
> errors directories but keep cilk-plus and array notation, maybe even
> abbreviate array notation to "an", (in future cilk keywords to "ck",
> pragma simd to "ps" and  elemental function to "ef"), will that be OK
> with everyone? We removed one directory and moved 3 scripts to 1, and
> with abbreviated subdirectories the ChangeLog lines won't be long. I
> really like to put all the cilk-plus tests in one location for C
> compiler so that if anyone wants to get them I can point to one
> location.
Seems like a reasonable compromise..  Works for me.  I'll stop 
bikeshedding :-)

jeff

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

[-- Attachment #1: Type: text/plain, Size: 7481 bytes --]

Hello Richard, et al.,
	Attached please find a patch with the following changes:

1. Test-suite codes were moved to the appropriate location as suggested below

And the following modifications that RTH mentioned in (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html)

2. ARRAY_NOTATION_TYPE is completely removed (from the location in tree, its definition and usage) 
3. Generic.texi had documentation for ARRAY_NOTATION_REF. That is also removed.

It is passing all the regression tests and not breaking/passing any other tests that were not already breaking/passing in the trunk.

Here are the ChangeLog entries with the above updates reflected:

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

	* doc/extend.texi (C Extensions): Added documentation about Cilk Plus
	array notation built-in reduction functions.
	* doc/passes.texi (Passes): Added documentation about changes done
	for Cilk Plus.
	* doc/invoke.texi (C Dialect Options): Added documentation about
	the -fcilkplus flag.
	* Makefile.in (C_COMMON_OBJS): Added c-family/array-notation-common.o.
	(BUILTINS_DEF): Depend on cilkplus.def.
	* builtins.def: Include cilkplus.def.  Define DEF_CILKPLUS_BUILTIN.
	* builtin-types.def: Define BT_FN_INT_PTR_PTR_PTR.
	* cilkplus.def: New file.

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

	* c-common.c (c_define_builtins): When cilkplus is enabled, the
	function array_notation_init_builtins is called.
	(c_common_init_ts): Added ARRAY_NOTATION_REF as typed.
	* c-common.def (ARRAY_NOTATION_REF): New tree.
	* c-common.h (build_array_notation_expr): New function declaration.
	(build_array_notation_ref): Likewise.
	(extract_sec_implicit_index_arg): New extern declaration.
	(is_sec_implicit_index_fn): Likewise.
	(ARRAY_NOTATION_CHECK): New define.
	(ARRAY_NOTATION_ARRAY): Likewise.
	(ARRAY_NOTATION_START): Likewise.
	(ARRAY_NOTATION_LENGTH): Likewise.
	(ARRAY_NOTATION_STRIDE): Likewise.
	* c-pretty-print.c (pp_c_postifix_expression): Added a new case for
	ARRAY_NOTATION_REF.
	(pp_c_expression): Likewise.
	* c.opt (flag_enable_cilkplus): New flag.
	* array-notation-common.c: New file.

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

	* c-typeck.c (build_array_ref): Added a check to see if array's
	index is greater than one.  If true, then emit an error.
	(build_function_call_vec): Exclude error reporting and checking
	for builtin array-notation functions.
	(convert_arguments): Likewise.
	(c_finish_return): Added a check for array notations as a return
	expression.  If true, then emit an error.
	(c_finish_loop): Added a check for array notations in a loop
	condition.  If true then emit an error.
	(lvalue_p): Added a ARRAY_NOTATION_REF case.
	(build_binary_op): Added a check for array notation expr inside
	op1 and op0.  If present, we call another function to find correct
	type.
	* 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_direct_declarator_inner): Added a check for array
	notation.
	(c_parser_compound_statement): Added a check for array notation in
	a stmt.  If one is present, then expand array notation expr.
	(c_parser_if_statement): Likewise.
	(c_parser_switch_statement): Added a check for array notations in
	a switch statement's condition.  If true, then output an error.
	(c_parser_while_statement): Similarly, but for a while.
	(c_parser_do_statement): Similarly, but for a do-while.
	(c_parser_for_statement): Similarly, but for a for-loop.
	(c_parser_unary_expression): Check if array notation is used in a
	pre-increment or pre-decrement expression.  If true, then expand
	them.
	(c_parser_array_notation): New function.
	* c-array-notation.c: New file.
	* c-tree.h (is_cilkplus_reduce_builtin): Protoize.


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

	* gcc.dg/cilk-plus/AN/array_test1.c: New test.
	* gcc.dg/cilk-plus/AN/array_test2.c: Ditto.
	* gcc.dg/cilk-plus/AN/array_test_ND.c: Ditto.
	* gcc.dg/cilk-plus/AN/builtin_func_double.c: Ditto.
	* gcc.dg/cilk-plus/AN/builtin_func_double2.c: Ditto.
	* gcc.dg/cilk-plus/AN/gather-scatter-errors.c: Ditto.
	* gcc.dg/cilk-plus/AN/if_test.c: Ditto.
	* gcc.dg/cilk-plus/AN/sec_implicit_ex.c: Ditto.
	* gcc.dg/cilk-plus/AN/decl-ptr-colon.c: Ditto.
	* gcc.dg/cilk-plus/AN/dimensionless-arrays.c: Ditto.
	* gcc.dg/cilk-plus/AN/fn_ptr.c: Ditto.
	* gcc.dg/cilk-plus/AN/fp_triplet_values.c: Ditto.
	* gcc.dg/cilk-plus/AN/gather-scatter.c: Ditto.
	* gcc.dg/cilk-plus/AN/misc.c: Ditto.
	* gcc.dg/cilk-plus/AN/parser_errors.c: Ditto.
	* gcc.dg/cilk-plus/AN/parser_errors2.c: Ditto.
	* gcc.dg/cilk-plus/AN/parser_errors3.c: Ditto.
	* gcc.dg/cilk-plus/AN/parser_errors4.c: Ditto.
	* gcc.dg/cilk-plus/AN/rank_mismatch.c: Ditto.
	* gcc.dg/cilk-plus/AN/rank_mismatch2.c: Ditto.
	* gcc.dg/cilk-plus/AN/rank_mismatch3.c: Ditto.
	* gcc.dg/cilk-plus/AN/sec_implicit.c: Ditto.
	* gcc.dg/cilk-plus/AN/sec_implicit2.c: Ditto.
	* gcc.dg/cilk-plus/AN/sec_reduce_max_min_ind.c: Ditto.
	* gcc.dg/cilk-plus/AN/tst_lngth.c: Ditto.
	* gcc.dg/cilk-plus/AN/vla.c: Ditto.
	* gcc.dg/cilk-plus/AN/an-if.c: Ditto.
	* gcc.dg/cilk-plus/AN/builtin_fn_custom.c: Ditto.
	* gcc.dg/cilk-plus/AN/builtin_fn_mutating.c: Ditto.
	* gcc.dg/cilk-plus/AN/comma_exp.c: Ditto.
	* gcc.dg/cilk-plus/AN/conditional.c: Ditto.
	* gcc.dg/cilk-plus/AN/exec-once.c: Ditto.
	* gcc.dg/cilk-plus/AN/exec-once2.c: Ditto.
	* gcc.dg/cilk-plus/AN/gather_scatter.c: Ditto.
	* gcc.dg/cilk-plus/AN/n-ptr-test.c: Ditto.
	* gcc.dg/cilk-plus/AN/side-effects-1.c: Ditto.
	* gcc.dg/cilk-plus/AN/test_builtin_return.c: Ditto.
	* gcc.dg/cilk-plus/AN/test_sec_limits.c: Ditto.
	* gcc.dg/cilk-plus/AN/cilkplus_AN_c.exp: New script.


Is this OK for trunk?

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jeff Law
> Sent: Friday, May 24, 2013 12:21 PM
> To: Iyer, Balaji V
> Cc: Aldy Hernandez; Jakub Jelinek; Richard Henderson; 'Joseph S. Myers'; 'gcc-
> patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/23/2013 06:42 PM, Iyer, Balaji V wrote:
> >
> > I got all your responses and, if I remove the compile, execute and
> > errors directories but keep cilk-plus and array notation, maybe even
> > abbreviate array notation to "an", (in future cilk keywords to "ck",
> > pragma simd to "ps" and  elemental function to "ef"), will that be OK
> > with everyone? We removed one directory and moved 3 scripts to 1, and
> > with abbreviated subdirectories the ChangeLog lines won't be long. I
> > really like to put all the cilk-plus tests in one location for C
> > compiler so that if anyone wants to get them I can point to one
> > location.
> Seems like a reasonable compromise..  Works for me.  I'll stop bikeshedding :-)
> 
> jeff

[-- Attachment #2: patch.txt.bz2 --]
[-- Type: application/octet-stream, Size: 30811 bytes --]

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Aldy Hernandez]

I'm ok inasmuch as the relevant tests are shared between c/c++.

"Iyer, Balaji V" <balaji.v.iyer@intel.com> wrote:

[I included Jeff Law also in this conversation]

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Aldy Hernandez
> Sent: Thursday, May 23, 2013 3:23 PM
> To: Jakub Jelinek
> Cc: Iyer, Balaji V; Richard Henderson; 'Joseph S. Myers'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/23/13 14:03, Jakub Jelinek wrote:
> > On Thu, May 23, 2013 at 06:27:04PM +0000, Iyer, Balaji V wrote:
> >> gcc/testsuite/ChangeLog
> >> 2013-05-23  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> >>
> >>          * gcc.dg/cilk-plus/array_notation/compile/array_test2.c: New test.
> >
> > I have concerns about the test locations, to me this looks way too
> > deep tree, whether something is a compile test, or compile test
> > expecting errors or runtime test is easily determined by { dg-do
> > compile } vs. { dg-do run } and presence or lack of { dg-error ... }
> > comments.  So IMHO that level should be left out, plus I'd say the
> > array_notation/ level is unnecessary as well, just put everything into
> > c-c++-common/cilk-plus/an-*.c (except for tests that aren't going to
> > be usable for C++, those can stay in gcc.dg/cilk-plus/an-*.c).  Then
> > gcc.dg/cilk-plus/*.exp would just ensure that tests from that
> > directory are run and also from c-c++-common/ and later on the same
> > would happen in g++.dg/cilk-plus/.  In the future when you will need
> > to link against runtime library cilk-plus.exp would just arrange for that to be
> added to LD_LIBRARY_PATH, -L.../ etc.
> 
> For the record, I agree.

I got all your responses and, if I remove the compile, execute and errors directories but keep cilk-plus and array notation, maybe even abbreviate array notation to "an", (in future cilk keywords to "ck", pragma simd to "ps" and  elemental function to "ef"), will that be OK with everyone? We removed one directory and moved 3 scripts to 1, and with abbreviated subdirectories the ChangeLog lines won't be long. I really like to put all the cilk-plus tests in one location for C compiler so that if anyone wants to get them I can point to one location.

Thanks,

Balaji V. Iyer.

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jakub Jelinek]

On Fri, May 24, 2013 at 05:52:11PM +0000, Iyer, Balaji V wrote:
> 	* gcc.dg/cilk-plus/AN/array_test1.c: New test.
...
> 	* gcc.dg/cilk-plus/AN/cilkplus_AN_c.exp: New script.

Ok, I guess I can live with /AN/ extra level, but can you please
move it still to c-c++-common/cilk-plus/AN/ for all tests that can
be shared with C++?  Look at c-c++-common/ current tests, see
how they can use { target c } or { target c++ } etc. if there is say
small difference in diagnostics, but still most of the test is the same.

I don't like the cilkplus_AN_c.exp, IMNSHO you should just have
gcc.dg/cilk-plus/cilk-plus.exp that handles everything.  For how
is it supposed to load the files from subdirectories or how is it supposed
to load c-c++-common/cilk-plus/ testcases please look at g++.dg/dg.exp
and/or gcc.dg/dg.exp.

	Jakub

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

[-- Attachment #1: Type: text/plain, Size: 7697 bytes --]



> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jakub Jelinek
> Sent: Friday, May 24, 2013 2:03 PM
> To: Iyer, Balaji V
> Cc: Jeff Law; rth@redhat.com; Aldy Hernandez; 'Joseph S. Myers'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On Fri, May 24, 2013 at 05:52:11PM +0000, Iyer, Balaji V wrote:
> > 	* gcc.dg/cilk-plus/AN/array_test1.c: New test.
> ...
> > 	* gcc.dg/cilk-plus/AN/cilkplus_AN_c.exp: New script.
> 
> Ok, I guess I can live with /AN/ extra level, but can you please move it still to c-
> c++-common/cilk-plus/AN/ for all tests that can be shared with C++?  Look at c-
> c++-common/ current tests, see how they can use { target c } or { target c++ }
> etc. if there is say small difference in diagnostics, but still most of the test is the
> same.

> 
> I don't like the cilkplus_AN_c.exp, IMNSHO you should just have gcc.dg/cilk-
> plus/cilk-plus.exp that handles everything.  For how is it supposed to load the
> files from subdirectories or how is it supposed to load c-c++-common/cilk-plus/
> testcases please look at g++.dg/dg.exp and/or gcc.dg/dg.exp.

Hi Jakub,
	I moved all the tests from gcc.dg/cilk-plus/AN directory to c-c++-common/cilk-plus/AN directory. The gcc.dg/cilk-plus directory just contains cilk-plus.exp script, which will handle all the tests in cilk-plus, not just array notation (when the others are checked in that is). Is this what you requested?

I apologize for misunderstanding your request.

I am attaching the fixed patch and the ChangeLogs are below:

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

	* doc/extend.texi (C Extensions): Added documentation about Cilk Plus
	array notation built-in reduction functions.
	* doc/passes.texi (Passes): Added documentation about changes done
	for Cilk Plus.
	* doc/invoke.texi (C Dialect Options): Added documentation about
	the -fcilkplus flag.
	* Makefile.in (C_COMMON_OBJS): Added c-family/array-notation-common.o.
	(BUILTINS_DEF): Depend on cilkplus.def.
	* builtins.def: Include cilkplus.def.  Define DEF_CILKPLUS_BUILTIN.
	* builtin-types.def: Define BT_FN_INT_PTR_PTR_PTR.
	* cilkplus.def: New file.

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

	* c-common.c (c_define_builtins): When cilkplus is enabled, the
	function array_notation_init_builtins is called.
	(c_common_init_ts): Added ARRAY_NOTATION_REF as typed.
	* c-common.def (ARRAY_NOTATION_REF): New tree.
	* c-common.h (build_array_notation_expr): New function declaration.
	(build_array_notation_ref): Likewise.
	(extract_sec_implicit_index_arg): New extern declaration.
	(is_sec_implicit_index_fn): Likewise.
	(ARRAY_NOTATION_CHECK): New define.
	(ARRAY_NOTATION_ARRAY): Likewise.
	(ARRAY_NOTATION_START): Likewise.
	(ARRAY_NOTATION_LENGTH): Likewise.
	(ARRAY_NOTATION_STRIDE): Likewise.
	* c-pretty-print.c (pp_c_postifix_expression): Added a new case for
	ARRAY_NOTATION_REF.
	(pp_c_expression): Likewise.
	* c.opt (flag_enable_cilkplus): New flag.
	* array-notation-common.c: New file.

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

	* c-typeck.c (build_array_ref): Added a check to see if array's
	index is greater than one.  If true, then emit an error.
	(build_function_call_vec): Exclude error reporting and checking
	for builtin array-notation functions.
	(convert_arguments): Likewise.
	(c_finish_return): Added a check for array notations as a return
	expression.  If true, then emit an error.
	(c_finish_loop): Added a check for array notations in a loop
	condition.  If true then emit an error.
	(lvalue_p): Added a ARRAY_NOTATION_REF case.
	(build_binary_op): Added a check for array notation expr inside
	op1 and op0.  If present, we call another function to find correct
	type.
	* 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_direct_declarator_inner): Added a check for array
	notation.
	(c_parser_compound_statement): Added a check for array notation in
	a stmt.  If one is present, then expand array notation expr.
	(c_parser_if_statement): Likewise.
	(c_parser_switch_statement): Added a check for array notations in
	a switch statement's condition.  If true, then output an error.
	(c_parser_while_statement): Similarly, but for a while.
	(c_parser_do_statement): Similarly, but for a do-while.
	(c_parser_for_statement): Similarly, but for a for-loop.
	(c_parser_unary_expression): Check if array notation is used in a
	pre-increment or pre-decrement expression.  If true, then expand
	them.
	(c_parser_array_notation): New function.
	* c-array-notation.c: New file.
	* c-tree.h (is_cilkplus_reduce_builtin): Protoize.

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

	* c-c++-common/cilk-plus/AN/array_test1.c: New test.
	* c-c++-common/cilk-plus/AN/array_test2.c: Likewise.
	* c-c++-common/cilk-plus/AN/array_test_ND.c: Likewise.
	* c-c++-common/cilk-plus/AN/builtin_func_double.c: Likewise.
	* c-c++-common/cilk-plus/AN/builtin_func_double2.c: Likewise.
	* c-c++-common/cilk-plus/AN/gather-scatter-errors.c: Likewise.
	* c-c++-common/cilk-plus/AN/if_test.c: Likewise.
	* c-c++-common/cilk-plus/AN/sec_implicit_ex.c: Likewise.
	* c-c++-common/cilk-plus/AN/decl-ptr-colon.c: Likewise.
	* c-c++-common/cilk-plus/AN/dimensionless-arrays.c: Likewise.
	* c-c++-common/cilk-plus/AN/fn_ptr.c: Likewise.
	* c-c++-common/cilk-plus/AN/fp_triplet_values.c: Likewise.
	* c-c++-common/cilk-plus/AN/gather-scatter.c: Likewise.
	* c-c++-common/cilk-plus/AN/misc.c: Likewise.
	* c-c++-common/cilk-plus/AN/parser_errors.c: Likewise.
	* c-c++-common/cilk-plus/AN/parser_errors2.c: Likewise.
	* c-c++-common/cilk-plus/AN/parser_errors3.c: Likewise.
	* c-c++-common/cilk-plus/AN/parser_errors4.c: Likewise.
	* c-c++-common/cilk-plus/AN/rank_mismatch.c: Likewise.
	* c-c++-common/cilk-plus/AN/rank_mismatch2.c: Likewise.
	* c-c++-common/cilk-plus/AN/rank_mismatch3.c: Likewise.
	* c-c++-common/cilk-plus/AN/sec_implicit.c: Likewise.
	* c-c++-common/cilk-plus/AN/sec_implicit2.c: Likewise.
	* c-c++-common/cilk-plus/AN/sec_reduce_max_min_ind.c: Likewise.
	* c-c++-common/cilk-plus/AN/tst_lngth.c: Likewise.
	* c-c++-common/cilk-plus/AN/vla.c: Likewise.
	* c-c++-common/cilk-plus/AN/an-if.c: Likewise.
	* c-c++-common/cilk-plus/AN/builtin_fn_custom.c: Likewise.
	* c-c++-common/cilk-plus/AN/builtin_fn_mutating.c: Likewise.
	* c-c++-common/cilk-plus/AN/comma_exp.c: Likewise.
	* c-c++-common/cilk-plus/AN/conditional.c: Likewise.
	* c-c++-common/cilk-plus/AN/exec-once.c: Likewise.
	* c-c++-common/cilk-plus/AN/exec-once2.c: Likewise.
	* c-c++-common/cilk-plus/AN/gather_scatter.c: Likewise.
	* c-c++-common/cilk-plus/AN/n-ptr-test.c: Likewise.
	* c-c++-common/cilk-plus/AN/side-effects-1.c: Likewise.
	* c-c++-common/cilk-plus/AN/test_builtin_return.c: Likewise.
	* c-c++-common/cilk-plus/AN/test_sec_limits.c: Likewise.
	* gcc.dg/cilk-plus/cilk-plus.exp: New script.


Is it OK for trunk?

Thanks,

Balaji V. Iyer.

> 
> 	Jakub

[-- Attachment #2: patch.txt.bz2 --]
[-- Type: application/octet-stream, Size: 30742 bytes --]

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Aldy Hernandez]

On 05/24/13 13:43, Iyer, Balaji V wrote:

> Hi Jakub,
> 	I moved all the tests from gcc.dg/cilk-plus/AN directory to c-c++-common/cilk-plus/AN directory. The gcc.dg/cilk-plus directory just contains cilk-plus.exp script, which will handle all the tests in cilk-plus, not just array notation (when the others are checked in that is). Is this what you requested?
...
...
> 	* c-c++-common/cilk-plus/AN/gather_scatter.c: Likewise.
> 	* c-c++-common/cilk-plus/AN/n-ptr-test.c: Likewise.
> 	* c-c++-common/cilk-plus/AN/side-effects-1.c: Likewise.
> 	* c-c++-common/cilk-plus/AN/test_builtin_return.c: Likewise.
> 	* c-c++-common/cilk-plus/AN/test_sec_limits.c: Likewise.
> 	* gcc.dg/cilk-plus/cilk-plus.exp: New script.

Yes, I believe that's what Jakub is requesting.  Basically, what we want 
is that common tests that should work on both C and C++ go inside the 
c-c++-common infrastructure, and thus have one test that stresses both 
front ends.

However, you may still need some gcc.dg/cilk-plus/* directory with 
corresponding infrastructure if you have any tests that are exclusive to 
C (and when you have C++ specific tests, corresponding g++.dg/cilk-plus 
infrastructure).

I'm curious, since you currently don't have any array notation support 
for C++, the above patch should give lots of errors when it tries to 
test the cilkplus tests on C++.  Does it not give you errors?  For now 
you may need some conditional to avoid running the tests on C++ until 
you get the C++ FE changes reviewed and approved.

Aldy

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

Hi Aldy
	Please see my responses below.

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Aldy Hernandez
> Sent: Tuesday, May 28, 2013 12:38 PM
> To: Iyer, Balaji V
> Cc: Jakub Jelinek; Jeff Law; rth@redhat.com; 'Joseph S. Myers'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/24/13 13:43, Iyer, Balaji V wrote:
> 
> > Hi Jakub,
> > 	I moved all the tests from gcc.dg/cilk-plus/AN directory to c-c++-
> common/cilk-plus/AN directory. The gcc.dg/cilk-plus directory just contains cilk-
> plus.exp script, which will handle all the tests in cilk-plus, not just array notation
> (when the others are checked in that is). Is this what you requested?
> ...
> ...
> > 	* c-c++-common/cilk-plus/AN/gather_scatter.c: Likewise.
> > 	* c-c++-common/cilk-plus/AN/n-ptr-test.c: Likewise.
> > 	* c-c++-common/cilk-plus/AN/side-effects-1.c: Likewise.
> > 	* c-c++-common/cilk-plus/AN/test_builtin_return.c: Likewise.
> > 	* c-c++-common/cilk-plus/AN/test_sec_limits.c: Likewise.
> > 	* gcc.dg/cilk-plus/cilk-plus.exp: New script.
> 
> Yes, I believe that's what Jakub is requesting.  Basically, what we want is that
> common tests that should work on both C and C++ go inside the
> c-c++-common infrastructure, and thus have one test that stresses both
> front ends.
> 
> However, you may still need some gcc.dg/cilk-plus/* directory with
> corresponding infrastructure if you have any tests that are exclusive to
> C (and when you have C++ specific tests, corresponding g++.dg/cilk-plus
> infrastructure).
> 
> I'm curious, since you currently don't have any array notation support
> for C++, the above patch should give lots of errors when it tries to
> test the cilkplus tests on C++.  Does it not give you errors?  For now
> you may need some conditional to avoid running the tests on C++ until
> you get the C++ FE changes reviewed and approved.

	Jakub, in his previous email, pointed out to dg.exp in gcc.dg. It was organized in the following way: the .c files were put in c-c++-common. The C++ code (.cc and .C) were stored in c-c++-common/cpp directory. The gcc.dg/dg.exp calls the appropriate .c files. This script is executed. The same thing is going on in g++.dg/dg.exp.

	I put all the cilkplus tests in c-c++-common/cilk-plus/AN directory. I created a script called cilk-plus.exp in gcc.dg/cilk-plus/cilk-plus.exp that calls these c code from c-c++-common/cilk-plus/AN directory.

	At present, I did not have a g++.dg/cilk-plus/cilk-plus.exp script, thus C++ compiler does not execute these tests.

So, is this OK?

Thanks,

Balaji V. Iyer.


> 
> Aldy

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jakub Jelinek]

On Tue, May 28, 2013 at 11:37:32AM -0500, Aldy Hernandez wrote:
> >	I moved all the tests from gcc.dg/cilk-plus/AN directory to c-c++-common/cilk-plus/AN directory. The gcc.dg/cilk-plus directory just contains cilk-plus.exp script, which will handle all the tests in cilk-plus, not just array notation (when the others are checked in that is). Is this what you requested?
> ...
> ...
> >	* c-c++-common/cilk-plus/AN/gather_scatter.c: Likewise.
> >	* c-c++-common/cilk-plus/AN/n-ptr-test.c: Likewise.
> >	* c-c++-common/cilk-plus/AN/side-effects-1.c: Likewise.
> >	* c-c++-common/cilk-plus/AN/test_builtin_return.c: Likewise.
> >	* c-c++-common/cilk-plus/AN/test_sec_limits.c: Likewise.
> >	* gcc.dg/cilk-plus/cilk-plus.exp: New script.
> 
> Yes, I believe that's what Jakub is requesting.  Basically, what we
> want is that common tests that should work on both C and C++ go
> inside the c-c++-common infrastructure, and thus have one test that
> stresses both front ends.
> 
> However, you may still need some gcc.dg/cilk-plus/* directory with
> corresponding infrastructure if you have any tests that are
> exclusive to C (and when you have C++ specific tests, corresponding
> g++.dg/cilk-plus infrastructure).
> 
> I'm curious, since you currently don't have any array notation
> support for C++, the above patch should give lots of errors when it
> tries to test the cilkplus tests on C++.  Does it not give you
> errors?  For now you may need some conditional to avoid running the
> tests on C++ until you get the C++ FE changes reviewed and approved.

No, until g++.dg/cilk-plus/cilk-plus.exp is added, there is no *.exp
file that will run the c-c++-common/cilk-plus/ tests for C++, so they will
be run for C only.

	Jakub

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Aldy Hernandez]

On 05/28/13 11:49, Iyer, Balaji V wrote:

> 	At present, I did not have a g++.dg/cilk-plus/cilk-plus.exp script, thus C++ compiler does not execute these tests.

Ah, I see.  Perfect.

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

Hi Richard, Jakub et al..
	I think I have fixed everything requested by RTH (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html).  I think I have also moved the tests in the correct place Jakub requested. It is passing all the correct regression tests and not affecting others.

Is this patch OK for trunk?

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jakub Jelinek
> Sent: Tuesday, May 28, 2013 12:51 PM
> To: Aldy Hernandez
> Cc: Iyer, Balaji V; Jeff Law; rth@redhat.com; 'Joseph S. Myers'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On Tue, May 28, 2013 at 11:37:32AM -0500, Aldy Hernandez wrote:
> > >	I moved all the tests from gcc.dg/cilk-plus/AN directory to c-c++-
> common/cilk-plus/AN directory. The gcc.dg/cilk-plus directory just contains cilk-
> plus.exp script, which will handle all the tests in cilk-plus, not just array notation
> (when the others are checked in that is). Is this what you requested?
> > ...
> > ...
> > >	* c-c++-common/cilk-plus/AN/gather_scatter.c: Likewise.
> > >	* c-c++-common/cilk-plus/AN/n-ptr-test.c: Likewise.
> > >	* c-c++-common/cilk-plus/AN/side-effects-1.c: Likewise.
> > >	* c-c++-common/cilk-plus/AN/test_builtin_return.c: Likewise.
> > >	* c-c++-common/cilk-plus/AN/test_sec_limits.c: Likewise.
> > >	* gcc.dg/cilk-plus/cilk-plus.exp: New script.
> >
> > Yes, I believe that's what Jakub is requesting.  Basically, what we
> > want is that common tests that should work on both C and C++ go inside
> > the c-c++-common infrastructure, and thus have one test that stresses
> > both front ends.
> >
> > However, you may still need some gcc.dg/cilk-plus/* directory with
> > corresponding infrastructure if you have any tests that are exclusive
> > to C (and when you have C++ specific tests, corresponding
> > g++.dg/cilk-plus infrastructure).
> >
> > I'm curious, since you currently don't have any array notation support
> > for C++, the above patch should give lots of errors when it tries to
> > test the cilkplus tests on C++.  Does it not give you errors?  For now
> > you may need some conditional to avoid running the tests on C++ until
> > you get the C++ FE changes reviewed and approved.
> 
> No, until g++.dg/cilk-plus/cilk-plus.exp is added, there is no *.exp file that will
> run the c-c++-common/cilk-plus/ tests for C++, so they will be run for C only.
> 
> 	Jakub

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Richard Henderson]

On 05/28/2013 11:44 AM, Iyer, Balaji V wrote:
> i Richard, Jakub et al..
> 	I think I have fixed everything requested by RTH (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html).  I think I have also moved the tests in the correct place Jakub requested. It is passing all the correct regression tests and not affecting others.
> 
> Is this patch OK for trunk?

Yes, it's ok.


r~

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Richard Henderson
> Sent: Tuesday, May 28, 2013 2:52 PM
> To: Iyer, Balaji V
> Cc: Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S. Myers'; 'gcc-patches'
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/28/2013 11:44 AM, Iyer, Balaji V wrote:
> > i Richard, Jakub et al..
> > 	I think I have fixed everything requested by RTH
> (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html).  I think I have
> also moved the tests in the correct place Jakub requested. It is passing all the
> correct regression tests and not affecting others.
> >
> > Is this patch OK for trunk?
> 
> Yes, it's ok.

This patch is committed to trunk at revision 199389.

Thanks,

Balaji V. Iyer.

> 
> 
> r~

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[H.J. Lu]

On Tue, May 28, 2013 at 1:02 PM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>> owner@gcc.gnu.org] On Behalf Of Richard Henderson
>> Sent: Tuesday, May 28, 2013 2:52 PM
>> To: Iyer, Balaji V
>> Cc: Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S. Myers'; 'gcc-patches'
>> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
>>
>> On 05/28/2013 11:44 AM, Iyer, Balaji V wrote:
>> > i Richard, Jakub et al..
>> >     I think I have fixed everything requested by RTH
>> (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html).  I think I have
>> also moved the tests in the correct place Jakub requested. It is passing all the
>> correct regression tests and not affecting others.
>> >
>> > Is this patch OK for trunk?
>>
>> Yes, it's ok.
>
> This patch is committed to trunk at revision 199389.
>
> Thanks,
>

On Linux/x32, I got

FAIL: c-c++-common/cilk-plus/AN/if_test.c  -O0 -fcilkplus execution test
FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -O0 -std=c99
execution test
FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -g -O0 -std=c99
execution test
FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -g -std=c99 execution test
FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -std=c99 execution test
FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus execution test
FAIL: c-c++-common/cilk-plus/AN/if_test.c  -g -O0 -fcilkplus execution test
FAIL: c-c++-common/cilk-plus/AN/if_test.c  -g -fcilkplus execution test


[x32@gnu-35 gcc]$ /export/gnu/import/git/gcc-test-x32/bld/gcc/xgcc
-B/export/gnu/import/git/gcc-test-x32/bld/gcc/
/export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
 -fno-diagnostics-show-caret -fdiagnostics-color=never   -fcilkplus -g
-O0 -std=c99 -fcilkplus  -lm   -m32 -o ./if_test.exe
[x32@gnu-35 gcc]$ /export/gnu/import/git/gcc-test-x32/bld/gcc/xgcc
-B/export/gnu/import/git/gcc-test-x32/bld/gcc/
/export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
 -fno-diagnostics-show-caret -fdiagnostics-color=never   -fcilkplus -g
-O0 -std=c99 -fcilkplus  -lm   -mx32 -o ./if_test.exe
[x32@gnu-35 gcc]$ ./if_test.exe
[x32@gnu-35 gcc]$ echo $?
5
[x32@gnu-35 gcc]$

(gdb) r
Starting program:
/export/gnu/import/git/gcc-test-x32/bld/gcc/testsuite/gcc/if_test.exe

Breakpoint 1, main2 (argc=3, argv=0xffffd240)
    at /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c:131
131          return 5;
Missing separate debuginfos, use: debuginfo-install glibc-2.16-30.1.fc18.x32
(gdb) p ii
$1 = 0
(gdb) p array2_check
$2 = {5, 5, 5, 5, 5, 5, 5, 5, 5, 5}
(gdb) p array2
$3 = {10, 10, 10, 10, 10, 10, 10, 10, 10, 10}
(gdb)

Does cilkplus assume ptr_mode == word_mode?  On x32, ptr_mode == SImode
and word_mode == DImode.

--
H.J.

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[H.J. Lu]

On Tue, May 28, 2013 at 5:35 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, May 28, 2013 at 1:02 PM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>>> owner@gcc.gnu.org] On Behalf Of Richard Henderson
>>> Sent: Tuesday, May 28, 2013 2:52 PM
>>> To: Iyer, Balaji V
>>> Cc: Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S. Myers'; 'gcc-patches'
>>> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
>>>
>>> On 05/28/2013 11:44 AM, Iyer, Balaji V wrote:
>>> > i Richard, Jakub et al..
>>> >     I think I have fixed everything requested by RTH
>>> (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html).  I think I have
>>> also moved the tests in the correct place Jakub requested. It is passing all the
>>> correct regression tests and not affecting others.
>>> >
>>> > Is this patch OK for trunk?
>>>
>>> Yes, it's ok.
>>
>> This patch is committed to trunk at revision 199389.
>>
>> Thanks,
>>
>
> On Linux/x32, I got
>
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -O0 -fcilkplus execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -O0 -std=c99
> execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -g -O0 -std=c99
> execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -g -std=c99 execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -std=c99 execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -g -O0 -fcilkplus execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -g -fcilkplus execution test
>
>
> [x32@gnu-35 gcc]$ /export/gnu/import/git/gcc-test-x32/bld/gcc/xgcc
> -B/export/gnu/import/git/gcc-test-x32/bld/gcc/
> /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
>  -fno-diagnostics-show-caret -fdiagnostics-color=never   -fcilkplus -g
> -O0 -std=c99 -fcilkplus  -lm   -m32 -o ./if_test.exe
> [x32@gnu-35 gcc]$ /export/gnu/import/git/gcc-test-x32/bld/gcc/xgcc
> -B/export/gnu/import/git/gcc-test-x32/bld/gcc/
> /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
>  -fno-diagnostics-show-caret -fdiagnostics-color=never   -fcilkplus -g
> -O0 -std=c99 -fcilkplus  -lm   -mx32 -o ./if_test.exe
> [x32@gnu-35 gcc]$ ./if_test.exe
> [x32@gnu-35 gcc]$ echo $?
> 5
> [x32@gnu-35 gcc]$
>
> (gdb) r
> Starting program:
> /export/gnu/import/git/gcc-test-x32/bld/gcc/testsuite/gcc/if_test.exe
>
> Breakpoint 1, main2 (argc=3, argv=0xffffd240)
>     at /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c:131
> 131          return 5;
> Missing separate debuginfos, use: debuginfo-install glibc-2.16-30.1.fc18.x32
> (gdb) p ii
> $1 = 0
> (gdb) p array2_check
> $2 = {5, 5, 5, 5, 5, 5, 5, 5, 5, 5}
> (gdb) p array2
> $3 = {10, 10, 10, 10, 10, 10, 10, 10, 10, 10}
> (gdb)
>
> Does cilkplus assume ptr_mode == word_mode?  On x32, ptr_mode == SImode
> and word_mode == DImode.
>
> --
> H.J.

Comment out

  /* atoi(argv[1]) == 10, so it will convert all 10's to 5's */
  if (FourDArray[0:10:1][0:5:2][9:10:-1][x:y:z] +
      FourDArray[0:10:1][0:5:2][9:-10:1][x:y:z]  != 20)
    array2[:] = 10;
  else
    array2[:] = 5;

makes the problem to disappear.

--
H.J.

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: H.J. Lu [mailto:hjl.tools@gmail.com]
> Sent: Tuesday, May 28, 2013 8:48 PM
> To: Iyer, Balaji V
> Cc: Richard Henderson; Jakub Jelinek; Aldy Hernandez; Jeff Law; Joseph S.
> Myers; gcc-patches
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On Tue, May 28, 2013 at 5:35 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Tue, May 28, 2013 at 1:02 PM, Iyer, Balaji V <balaji.v.iyer@intel.com>
> wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >>> owner@gcc.gnu.org] On Behalf Of Richard Henderson
> >>> Sent: Tuesday, May 28, 2013 2:52 PM
> >>> To: Iyer, Balaji V
> >>> Cc: Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S. Myers'; 'gcc-patches'
> >>> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> >>>
> >>> On 05/28/2013 11:44 AM, Iyer, Balaji V wrote:
> >>> > i Richard, Jakub et al..
> >>> >     I think I have fixed everything requested by RTH
> >>> (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html).  I think
> >>> I have also moved the tests in the correct place Jakub requested. It
> >>> is passing all the correct regression tests and not affecting others.
> >>> >
> >>> > Is this patch OK for trunk?
> >>>
> >>> Yes, it's ok.
> >>
> >> This patch is committed to trunk at revision 199389.
> >>
> >> Thanks,
> >>
> >
> > On Linux/x32, I got
> >
> > FAIL: c-c++-common/cilk-plus/AN/if_test.c  -O0 -fcilkplus execution
> > test
> > FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -O0 -std=c99
> > execution test
> > FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -g -O0 -std=c99
> > execution test
> > FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -g -std=c99
> > execution test
> > FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -std=c99
> > execution test
> > FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus execution test
> > FAIL: c-c++-common/cilk-plus/AN/if_test.c  -g -O0 -fcilkplus execution
> > test
> > FAIL: c-c++-common/cilk-plus/AN/if_test.c  -g -fcilkplus execution
> > test
> >
> >
> > [x32@gnu-35 gcc]$ /export/gnu/import/git/gcc-test-x32/bld/gcc/xgcc
> > -B/export/gnu/import/git/gcc-test-x32/bld/gcc/
> > /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-
> common/cilk-plus/AN/if_test.c
> >  -fno-diagnostics-show-caret -fdiagnostics-color=never   -fcilkplus -g
> > -O0 -std=c99 -fcilkplus  -lm   -m32 -o ./if_test.exe
> > [x32@gnu-35 gcc]$ /export/gnu/import/git/gcc-test-x32/bld/gcc/xgcc
> > -B/export/gnu/import/git/gcc-test-x32/bld/gcc/
> > /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-
> common/cilk-plus/AN/if_test.c
> >  -fno-diagnostics-show-caret -fdiagnostics-color=never   -fcilkplus -g
> > -O0 -std=c99 -fcilkplus  -lm   -mx32 -o ./if_test.exe
> > [x32@gnu-35 gcc]$ ./if_test.exe
> > [x32@gnu-35 gcc]$ echo $?
> > 5
> > [x32@gnu-35 gcc]$
> >
> > (gdb) r
> > Starting program:
> > /export/gnu/import/git/gcc-test-x32/bld/gcc/testsuite/gcc/if_test.exe
> >
> > Breakpoint 1, main2 (argc=3, argv=0xffffd240)
> >     at /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-
> common/cilk-plus/AN/if_test.c:131
> > 131          return 5;
> > Missing separate debuginfos, use: debuginfo-install
> > glibc-2.16-30.1.fc18.x32
> > (gdb) p ii
> > $1 = 0
> > (gdb) p array2_check
> > $2 = {5, 5, 5, 5, 5, 5, 5, 5, 5, 5}
> > (gdb) p array2
> > $3 = {10, 10, 10, 10, 10, 10, 10, 10, 10, 10}
> > (gdb)
> >
> > Does cilkplus assume ptr_mode == word_mode?  On x32, ptr_mode ==
> > SImode and word_mode == DImode.
> >
> > --
> > H.J.
> 
> Comment out
> 
>   /* atoi(argv[1]) == 10, so it will convert all 10's to 5's */
>   if (FourDArray[0:10:1][0:5:2][9:10:-1][x:y:z] +
>       FourDArray[0:10:1][0:5:2][9:-10:1][x:y:z]  != 20)
>     array2[:] = 10;
>   else
>     array2[:] = 5;
> 
> makes the problem to disappear.

Thanks for reporting this. Let me investigate and I will get back ASAP.

> 
> --
> H.J.

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Rainer Orth]

"Iyer, Balaji V" <balaji.v.iyer@intel.com> writes:

>> -----Original Message-----
>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>> owner@gcc.gnu.org] On Behalf Of Richard Henderson
>> Sent: Tuesday, May 28, 2013 2:52 PM
>> To: Iyer, Balaji V
>> Cc: Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S. Myers'; 'gcc-patches'
>> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
>> 
>> On 05/28/2013 11:44 AM, Iyer, Balaji V wrote:
>> > i Richard, Jakub et al..
>> > 	I think I have fixed everything requested by RTH
>> (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html).  I think I have
>> also moved the tests in the correct place Jakub requested. It is passing
>> all the
>> correct regression tests and not affecting others.
>> >
>> > Is this patch OK for trunk?
>> 
>> Yes, it's ok.
>
> This patch is committed to trunk at revision 199389.

... and immediately broke Solaris bootstrap, cf. PR bootstrap/57450.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> "Iyer, Balaji V" <balaji.v.iyer@intel.com> writes:
[...]
>> This patch is committed to trunk at revision 199389.
>
> ... and immediately broke Solaris bootstrap, cf. PR bootstrap/57450.

Fixed implementing Richard's suggestion from the PR.  Bootstrapped
without regressions on i386-pc-solaris2.10 and x86_64-unknown-linux-gnu,
installed as obvious.

	Rainer


2013-05-29  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	PR bootstrap/57450
	* c-array-notation.c (length_mismatch_in_expr_p): Use absu_hwi.
	(build_array_notation_expr): Likewise.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr57450.patch --]
[-- Type: text/x-patch, Size: 1038 bytes --]

# HG changeset patch
# Parent 22d5dfeeb2006caf8be6803106b454a19cfe6513
Fix c/c-array-notation.c compilation failure (PR bootstrap/57450)

diff --git a/gcc/c/c-array-notation.c b/gcc/c/c-array-notation.c
--- a/gcc/c/c-array-notation.c
+++ b/gcc/c/c-array-notation.c
@@ -116,7 +116,7 @@ length_mismatch_in_expr_p (location_t lo
 		{
 		  l_node = int_cst_value (list[ii][jj]);
 		  l_start = int_cst_value (start);
-		  if (abs (l_start) != abs (l_node))
+		  if (absu_hwi (l_start) != absu_hwi (l_node))
 		    {
 		      error_at (loc, "length mismatch in expression");
 		      return true;
@@ -1561,7 +1561,7 @@ build_array_notation_expr (location_t lo
       HOST_WIDE_INT r_length = int_cst_value (rhs_length[0][0]);
       /* Length can be negative or positive.  As long as the magnitude is OK,
 	 then the array notation is valid.  */
-      if (abs (l_length) != abs (r_length))
+      if (absu_hwi (l_length) != absu_hwi (r_length))
 	{
 	  error_at (location, "length mismatch between LHS and RHS");
 	  pop_stmt_list (an_init);

[-- Attachment #3: Type: text/plain, Size: 143 bytes --]


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[H.J. Lu]

On Tue, May 28, 2013 at 5:35 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, May 28, 2013 at 1:02 PM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>>> owner@gcc.gnu.org] On Behalf Of Richard Henderson
>>> Sent: Tuesday, May 28, 2013 2:52 PM
>>> To: Iyer, Balaji V
>>> Cc: Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S. Myers'; 'gcc-patches'
>>> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
>>>
>>> On 05/28/2013 11:44 AM, Iyer, Balaji V wrote:
>>> > i Richard, Jakub et al..
>>> >     I think I have fixed everything requested by RTH
>>> (http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01400.html).  I think I have
>>> also moved the tests in the correct place Jakub requested. It is passing all the
>>> correct regression tests and not affecting others.
>>> >
>>> > Is this patch OK for trunk?
>>>
>>> Yes, it's ok.
>>
>> This patch is committed to trunk at revision 199389.
>>
>> Thanks,
>>
>
> On Linux/x32, I got
>
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -O0 -fcilkplus execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -O0 -std=c99
> execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -g -O0 -std=c99
> execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -g -std=c99 execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus -std=c99 execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -fcilkplus execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -g -O0 -fcilkplus execution test
> FAIL: c-c++-common/cilk-plus/AN/if_test.c  -g -fcilkplus execution test
>
>
> [x32@gnu-35 gcc]$ /export/gnu/import/git/gcc-test-x32/bld/gcc/xgcc
> -B/export/gnu/import/git/gcc-test-x32/bld/gcc/
> /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
>  -fno-diagnostics-show-caret -fdiagnostics-color=never   -fcilkplus -g
> -O0 -std=c99 -fcilkplus  -lm   -m32 -o ./if_test.exe
> [x32@gnu-35 gcc]$ /export/gnu/import/git/gcc-test-x32/bld/gcc/xgcc
> -B/export/gnu/import/git/gcc-test-x32/bld/gcc/
> /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
>  -fno-diagnostics-show-caret -fdiagnostics-color=never   -fcilkplus -g
> -O0 -std=c99 -fcilkplus  -lm   -mx32 -o ./if_test.exe
> [x32@gnu-35 gcc]$ ./if_test.exe
> [x32@gnu-35 gcc]$ echo $?
> 5
> [x32@gnu-35 gcc]$
>
> (gdb) r
> Starting program:
> /export/gnu/import/git/gcc-test-x32/bld/gcc/testsuite/gcc/if_test.exe
>
> Breakpoint 1, main2 (argc=3, argv=0xffffd240)
>     at /export/gnu/import/git/gcc-test-x32/src-trunk/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c:131
> 131          return 5;
> Missing separate debuginfos, use: debuginfo-install glibc-2.16-30.1.fc18.x32
> (gdb) p ii
> $1 = 0
> (gdb) p array2_check
> $2 = {5, 5, 5, 5, 5, 5, 5, 5, 5, 5}
> (gdb) p array2
> $3 = {10, 10, 10, 10, 10, 10, 10, 10, 10, 10}
> (gdb)
>
> Does cilkplus assume ptr_mode == word_mode?  On x32, ptr_mode == SImode
> and word_mode == DImode.
>

I opened:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57452

There is a bug in the testcase.  The problem is the second
element in array notation is not the size of array and GCC
doesn't detect the problem.

--
H.J.

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[David Edelsohn]

On Thu, May 30, 2013 at 6:10 PM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:

> I think David is getting the correct output but just that dg-error is not catching it correctly.

What version of expect are you using?

Fedora 18 apparently has 5.45, others are using 5.44, and AIX 7.1
provides 5.42.1.

Thanks, David

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[David Edelsohn]

On Thu, May 30, 2013 at 6:10 PM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:

> I think David is getting the correct output but just that dg-error is not catching it correctly.

Is there an updated Tcl or runtest pre-req?

- David

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Thursday, May 30, 2013 5:02 PM
> To: Iyer, Balaji V
> Cc: David Edelsohn; Richard Henderson; Jakub Jelinek; Aldy Hernandez; 'Joseph
> S. Myers'; gcc-patches
> Subject: Re: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> On 05/30/2013 02:13 PM, Iyer, Balaji V wrote:
> > Hi David,
> > 	Please see my response below:
> >
> >> -----Original Message-----
> >> From: David Edelsohn [mailto:dje.gcc@gmail.com]
> >> Sent: Wednesday, May 29, 2013 11:44 AM
> >> To: Iyer, Balaji V
> >> Cc: Richard Henderson; Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S.
> >> Myers'; gcc-patches
> >> Subject: RE: [PING]RE: [patch] cilkplus: Array notation for C patch
> >>
> >> Balaji,
> >>
> >> Thanks for this new feature and I am relieved that so much of it
> >> works successfully on PowerLinux and AIX.
> >>
> >> I know that you have received a deluge of reports of issues with the
> >> cilkplus support that you slowly are working through.  I am seeing
> >> the following new testsuite failures on AIX:
> >>
> >> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
> plus/AN/sec_implicit2.c:
> >> In function 'main2':
> >> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
> >> plus/AN/sec_implicit2.c:23:15:
> >> error: __sec_implicit_index parameter must be an integer constant
> >> expression
> >
> > I have this line flagged for error using dg-error.
> > Here is the cut and paste of it:
> >
> > =======================================================
> >    array[:][:] = __sec_implicit_index(argc) + array[:][:]; /* { dg-error
> "__sec_implicit_index parameter" } */
> >    return 0;
> >
> ================================================================
> Looking at my tree, dg-error refers to "argument" not "parameter"


Hmm.. I have used "__sec_implicit_index parameter" in dg-error. Here is the link from gcc-git web portal (http://gcc.gnu.org/git/?p=gcc.git;a=blob_plain;f=gcc/testsuite/c-c%2B%2B-common/cilk-plus/AN/sec_implicit2.c;hb=4a2ca8f34c2b84bbd04eff0d22f97046dfbd0f07)

Also, it should hit the error in function extract_sec_implicit_index_arg in the file array-notation-common.c (http://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/c-family/array-notation-common.c;h=1d288464eee6a8c805ef627d58c14249da5ae143;hb=4a2ca8f34c2b84bbd04eff0d22f97046dfbd0f07)  and it does have "__sec_implicit_index parameter..." 

I think David is getting the correct output but just that dg-error is not catching it correctly.

Thanks,

Balaji V. Iyer.

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jakub Jelinek]

On Thu, May 30, 2013 at 03:02:27PM -0600, Jeff Law wrote:
> >>error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have arrays
> >>with dimension greater than 1
> >
> >Same as above for this also.
> Not sure about this one.  It's possible (I'd have to sit down with
> dg.exp for a while) that it's looking for "error: " immediately
> followed by your string.   If that's the case, then it's looking for
> "error: cannot have arrays with dimension greater than"
> 
> which won't match
> "error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have
> arrays with dimension greater than"

No, if the dg-error regexp doesn't start with number followed by :
(column), then it is matched as:
\[0-9\]+: error:\[^\n\]*
followed by the provided regexp.

	Jakub

Re: [PING]RE: [patch] cilkplus: Array notation for C patch

[Jeff Law]

On 05/30/2013 02:13 PM, Iyer, Balaji V wrote:
> Hi David,
> 	Please see my response below:
>
>> -----Original Message-----
>> From: David Edelsohn [mailto:dje.gcc@gmail.com]
>> Sent: Wednesday, May 29, 2013 11:44 AM
>> To: Iyer, Balaji V
>> Cc: Richard Henderson; Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S.
>> Myers'; gcc-patches
>> Subject: RE: [PING]RE: [patch] cilkplus: Array notation for C patch
>>
>> Balaji,
>>
>> Thanks for this new feature and I am relieved that so much of it works
>> successfully on PowerLinux and AIX.
>>
>> I know that you have received a deluge of reports of issues with the cilkplus
>> support that you slowly are working through.  I am seeing the following new
>> testsuite failures on AIX:
>>
>> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_implicit2.c:
>> In function 'main2':
>> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
>> plus/AN/sec_implicit2.c:23:15:
>> error: __sec_implicit_index parameter must be an integer constant expression
>
> I have this line flagged for error using dg-error.
> Here is the cut and paste of it:
>
> =======================================================
>    array[:][:] = __sec_implicit_index(argc) + array[:][:]; /* { dg-error "__sec_implicit_index parameter" } */
>    return 0;
> ================================================================
Looking at my tree, dg-error refers to "argument" not "parameter"


>
> I just have a partial message inside quotes, not the whole thing. My x86_64 machine seem to accept it. I am not very familiar with deja-gnu, but does dg-error require the complete error message?

>>
>> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
>> plus/AN/sec_reduce_max_min_ind.c:
>> In function 'main2':
>> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
>> plus/AN/sec_reduce_max_min_ind.c:23:28:
>> error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have arrays
>> with dimension greater than 1
>> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
>> plus/AN/sec_reduce_max_min_ind.c:27:28:
>> error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have arrays
>> with dimension greater than 1
>
> Same as above for this also.
Not sure about this one.  It's possible (I'd have to sit down with 
dg.exp for a while) that it's looking for "error: " immediately followed 
by your string.   If that's the case, then it's looking for
"error: cannot have arrays with dimension greater than"

which won't match
"error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have arrays 
with dimension greater than"

jeff

>

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

Hi David, 
	Please see my response below:

> -----Original Message-----
> From: David Edelsohn [mailto:dje.gcc@gmail.com]
> Sent: Wednesday, May 29, 2013 11:44 AM
> To: Iyer, Balaji V
> Cc: Richard Henderson; Jakub Jelinek; Aldy Hernandez; Jeff Law; 'Joseph S.
> Myers'; gcc-patches
> Subject: RE: [PING]RE: [patch] cilkplus: Array notation for C patch
> 
> Balaji,
> 
> Thanks for this new feature and I am relieved that so much of it works
> successfully on PowerLinux and AIX.
> 
> I know that you have received a deluge of reports of issues with the cilkplus
> support that you slowly are working through.  I am seeing the following new
> testsuite failures on AIX:
> 
> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_implicit2.c:
> In function 'main2':
> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
> plus/AN/sec_implicit2.c:23:15:
> error: __sec_implicit_index parameter must be an integer constant expression

I have this line flagged for error using dg-error.
Here is the cut and paste of it:

=======================================================
  array[:][:] = __sec_implicit_index(argc) + array[:][:]; /* { dg-error "__sec_implicit_index parameter" } */
  return 0;
================================================================

I just have a partial message inside quotes, not the whole thing. My x86_64 machine seem to accept it. I am not very familiar with deja-gnu, but does dg-error require the complete error message?

> 
> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
> plus/AN/sec_reduce_max_min_ind.c:
> In function 'main2':
> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
> plus/AN/sec_reduce_max_min_ind.c:23:28:
> error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have arrays
> with dimension greater than 1
> /nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-
> plus/AN/sec_reduce_max_min_ind.c:27:28:
> error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have arrays
> with dimension greater than 1

Same as above for this also.

> 
> I hope that you can help resolve these errors.
> 
> Thanks, David

RE: [PING]RE: [patch] cilkplus: Array notation for C patch

[David Edelsohn]

Balaji,

Thanks for this new feature and I am relieved that so much of it works
successfully on PowerLinux and AIX.

I know that you have received a deluge of reports of issues with the
cilkplus support that you slowly are working through.  I am seeing the
following new testsuite failures on AIX:

/nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_implicit2.c:
In function 'main2':
/nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_implicit2.c:23:15:
error: __sec_implicit_index parameter must be an integer constant
expression

/nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_reduce_max_min_ind.c:
In function 'main2':
/nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_reduce_max_min_ind.c:23:28:
error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have arrays
with dimension greater than 1
/nasfarm/dje/src/src/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_reduce_max_min_ind.c:27:28:
error: __sec_reduce_min_ind or __sec_reduce_max_ind cannot have arrays
with dimension greater than 1

I hope that you can help resolve these errors.

Thanks, David

[PING]RE: [patch] cilkplus: Array notation for C patch

[Iyer, Balaji V]

Hello Joseph,
	Did you get a chance to look at this patch?

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: Iyer, Balaji V
> Sent: Friday, March 29, 2013 5:58 PM
> To: 'Joseph Myers'; 'Aldy Hernandez'
> Cc: 'gcc-patches'
> Subject: RE: [patch] cilkplus: Array notation for C patch
> 
> Hello Joseph, Aldy et al.,
> 	I reworded couple comments (e.g changed builtin with built-in, etc) and
> added a header comment to the c-array-notation.c that explains the overall
> process. I am attaching  a fixed patch.
> 
> Thanks,
> 
> Balaji V. Iyer.
> 
> Here are the Changelog entries again:
> 
> gcc/ChangeLog
> +2013-03-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +       * doc/extend.texi (C Extensions): Added documentation about Cilk Plus
> +       array notation built-in reduction functions.
> +       * doc/passes.texi (Passes): Added documentation about changes done
> +       for Cilk Plus.
> +       * doc/invoke.texi (C Dialect Options): Added documentation about
> +       the -fcilkplus flag.
> +       * doc/generic.texi (Storage References): Added documentation for
> +       ARRAY_NOTATION_REF storage.
> +       * Makefile.in (C_COMMON_OBJS): Added c-family/array-notation-
> common.o.
> +       * tree-pretty-print.c (dump_generic_node): Add case for
> +       ARRAY_NOTATION_REF.
> +       (BUILTINS_DEF): Depend on cilkplus.def.
> +       * builtins.def: Include cilkplus.def.
> +       Define DEF_CILKPLUS_BUILTIN.
> +       * builtin-types.def: Define BT_FN_INT_PTR_PTR_PTR.
> +       * cilkplus.def: New file.
> 
> gcc/c-family/ChangeLog
> +2013-03-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +	* c-common.c (c_define_builtins): When cilkplus is enabled, the
> +	function array_notation_init_builtins is called.
> +	(c_common_init_ts): Added ARRAY_NOTATION_REF as typed.
> +	* c-common.def (ARRAY_NOTATION_REF): New tree.
> +	* c-common.h (build_array_notation_expr): New function declaration.
> +	(build_array_notation_ref): Likewise.
> +	(extract_sec_implicit_index_arg): New extern declaration.
> +	(is_sec_implicit_index_fn): Likewise.
> +	(ARRAY_NOTATION_CHECK): New define.
> +	(ARRAY_NOTATION_ARRAY): Likewise.
> +	(ARRAY_NOTATION_START): Likewise.
> +	(ARRAY_NOTATION_LENGTH): Likewise.
> +	(ARRAY_NOTATION_STRIDE): Likewise.
> +	(ARRAY_NOTATION_TYPE): Likewise.
> +	* c-pretty-print.c (pp_c_postifix_expression): Added a new case for
> +	ARRAY_NOTATION_REF.
> +	(pp_c_expression): Likewise.
> +	* c.opt (flag_enable_cilkplus): New flag.
> +	* array-notation-common.c: New file.
> 
> gcc/c/ChangeLog
> +2013-03-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +	* c-typeck.c (build_array_ref): Added a check to see if array's
> +	index is greater than one.  If true, then emit an error.
> +	(build_function_call_vec): Exclude error reporting and checking
> +	for builtin array-notation functions.
> +	(convert_arguments): Likewise.
> +	(c_finish_return): Added a check for array notations as a return
> +	expression.  If true, then emit an error.
> +	(c_finish_loop): Added a check for array notations in a loop
> +	condition.  If true then emit an error.
> +	(lvalue_p): Added a ARRAY_NOTATION_REF case.
> +	(build_binary_op): Added a check for array notation expr inside
> +	op1 and op0.  If present, we call another function to find correct
> +	type.
> +	* 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_direct_declarator_inner): Added a check for array
> +	notation.
> +	(c_parser_compound_statement): Added a check for array notation in
> +	a stmt.  If one is present, then expand array notation expr.
> +	(c_parser_if_statement): Likewise.
> +	(c_parser_switch_statement): Added a check for array notations in
> +	a switch statement's condition.  If true, then output an error.
> +	(c_parser_while_statement): Similarly, but for a while.
> +	(c_parser_do_statement): Similarly, but for a do-while.
> +	(c_parser_for_statement): Similarly, but for a for-loop.
> +	(c_parser_unary_expression): Check if array notation is used in a
> +	pre-increment or pre-decrement expression.  If true, then expand
> +	them.
> +	(c_parser_array_notation): New function.
> +	* c-array-notation.c: New file.
> +	* c-tree.h (is_cilkplus_reduce_builtin): Protoize.
> 
> > -----Original Message-----
> > From: Iyer, Balaji V
> > Sent: Thursday, March 28, 2013 1:07 PM
> > To: Joseph Myers; Aldy Hernandez
> > Cc: gcc-patches
> > Subject: [patch] cilkplus: Array notation for C patch
> >
> > Hello Joseph, Aldy et al.,
> > 	Attached, please find a fixed patch (bzipped) that implements array
> > notation for C. To my best knowledge, I have fixed all the changes
> > Joseph and Aldy have mentioned in the previous email threads
> > (http://gcc.gnu.org/ml/gcc- patches/2013-03/msg01182.html,
> > http://gcc.gnu.org/ml/gcc-patches/2013-
> > 03/msg01173.html, http://gcc.gnu.org/ml/gcc-patches/2013-
> > 03/msg00748.html, etc). Is it OK for trunk?
> >
> > Thanks,
> >
> > Balaji V. Iyer.
> >
> > Here are the Changelog entries:
> >
> > gcc/ChangeLog
> > +2013-03-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> > +
> > +       * doc/extend.texi (C Extensions): Added documentation about Cilk Plus
> > +       array notation built-in reduction functions.
> > +       * doc/passes.texi (Passes): Added documentation about changes done
> > +       for Cilk Plus.
> > +       * doc/invoke.texi (C Dialect Options): Added documentation about
> > +       the -fcilkplus flag.
> > +       * doc/generic.texi (Storage References): Added documentation for
> > +       ARRAY_NOTATION_REF storage.
> > +       * Makefile.in (C_COMMON_OBJS): Added c-family/array-notation-
> > common.o.
> > +       * tree-pretty-print.c (dump_generic_node): Add case for
> > +       ARRAY_NOTATION_REF.
> > +       (BUILTINS_DEF): Depend on cilkplus.def.
> > +       * builtins.def: Include cilkplus.def.
> > +       Define DEF_CILKPLUS_BUILTIN.
> > +       * builtin-types.def: Define BT_FN_INT_PTR_PTR_PTR.
> > +       * cilkplus.def: New file.
> >
> > gcc/c-family/ChangeLog
> > +2013-03-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> > +
> > +	* c-common.c (c_define_builtins): When cilkplus is enabled, the
> > +	function array_notation_init_builtins is called.
> > +	(c_common_init_ts): Added ARRAY_NOTATION_REF as typed.
> > +	* c-common.def (ARRAY_NOTATION_REF): New tree.
> > +	* c-common.h (build_array_notation_expr): New function declaration.
> > +	(build_array_notation_ref): Likewise.
> > +	(extract_sec_implicit_index_arg): New extern declaration.
> > +	(is_sec_implicit_index_fn): Likewise.
> > +	(ARRAY_NOTATION_CHECK): New define.
> > +	(ARRAY_NOTATION_ARRAY): Likewise.
> > +	(ARRAY_NOTATION_START): Likewise.
> > +	(ARRAY_NOTATION_LENGTH): Likewise.
> > +	(ARRAY_NOTATION_STRIDE): Likewise.
> > +	(ARRAY_NOTATION_TYPE): Likewise.
> > +	* c-pretty-print.c (pp_c_postifix_expression): Added a new case for
> > +	ARRAY_NOTATION_REF.
> > +	(pp_c_expression): Likewise.
> > +	* c.opt (flag_enable_cilkplus): New flag.
> > +	* array-notation-common.c: New file.
> >
> > gcc/c/ChangeLog
> > +2013-03-28  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> > +
> > +	* c-typeck.c (build_array_ref): Added a check to see if array's
> > +	index is greater than one.  If true, then emit an error.
> > +	(build_function_call_vec): Exclude error reporting and checking
> > +	for builtin array-notation functions.
> > +	(convert_arguments): Likewise.
> > +	(c_finish_return): Added a check for array notations as a return
> > +	expression.  If true, then emit an error.
> > +	(c_finish_loop): Added a check for array notations in a loop
> > +	condition.  If true then emit an error.
> > +	(lvalue_p): Added a ARRAY_NOTATION_REF case.
> > +	(build_binary_op): Added a check for array notation expr inside
> > +	op1 and op0.  If present, we call another function to find correct
> > +	type.
> > +	* 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_direct_declarator_inner): Added a check for array
> > +	notation.
> > +	(c_parser_compound_statement): Added a check for array notation in
> > +	a stmt.  If one is present, then expand array notation expr.
> > +	(c_parser_if_statement): Likewise.
> > +	(c_parser_switch_statement): Added a check for array notations in
> > +	a switch statement's condition.  If true, then output an error.
> > +	(c_parser_while_statement): Similarly, but for a while.
> > +	(c_parser_do_statement): Similarly, but for a do-while.
> > +	(c_parser_for_statement): Similarly, but for a for-loop.
> > +	(c_parser_unary_expression): Check if array notation is used in a
> > +	pre-increment or pre-decrement expression.  If true, then expand
> > +	them.
> > +	(c_parser_array_notation): New function.
> > +	* c-array-notation.c: New file.
> > +	* c-tree.h (is_cilkplus_reduce_builtin): Protoize.
> >
> >
> > > -----Original Message-----
> > > From: Joseph Myers [mailto:joseph@codesourcery.com]
> > > Sent: Wednesday, March 27, 2013 6:11 PM
> > > To: Aldy Hernandez
> > > Cc: gcc-patches; Iyer, Balaji V
> > > Subject: Re: [cilkplus-merge] rewrite __sec_reduce* builtin handling
> > > to use <.def> infrastructure
> > >
> > > On Wed, 27 Mar 2013, Aldy Hernandez wrote:
> > >
> > > > Is this OK for the branch?  I'd like to get a nod of approval
> > > > before committing to the branch, as to aid in review, but if you
> > > > prefer that I just commit at will, and then post a big "cilk plus array
> notation"
> > > > patch to the list, I can do this instead.  It just seems easier to
> > > > fix stuff incrementally, as suggestions and fixes are piling up
> > > > for both Balaji and
> > > myself.
> > >
> > > I'm expecting the two of you to deal with reviews for the branch and
> > > then repost a big patch against trunk once you think all the
> > > comments have
> > been addressed.
> > > But this looks like the sort of thing I was expecting for cleaning
> > > up the built-in function handling.
> > >
> > > --
> > > Joseph S. Myers
> > > joseph@codesourcery.com