This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [patch] cilkplus array notation for C (clean, independent patchset, take 1)


Hello Joseph, Aldy et al.,
   Attached, please find a fixed patch. This patch should fix all the issues that Joseph mentioned in both the emails with the exception of the built-in __sec_reduce functions. Aldy is currently looking into those. The changes that Aldy mentioned below have also been fixed with the appropriate comments below:

My current email client is not putting the '>' correctly, so all my comments are in CAPS below. I am currently looking into fixing that. Till then, I am sorry for the inconvenience.

Thanks,

Balaji V. Iyer.


________________________________________
From: Aldy Hernandez [aldyh@redhat.com]
Sent: Monday, March 25, 2013 12:45 PM
To: Iyer, Balaji V
Cc: gcc-patches@gcc.gnu.org; Joseph Myers [joseph@codesourcery.com]
Subject: Re: [patch] cilkplus array notation for C (clean, independent patchset, take 1)

On 03/22/13 17:03, Iyer, Balaji V wrote:

> I have not fixed all the issues below (the big one that is left is the bultin function representation that Joseph Pointed out). I have fixed most of the other issues. All the things I have fixed are marked by "FIXED!"

Don't worry, I can work on the builtin function representation.

I am keeping a list of pending issues on the wiki
(http://gcc.gnu.org/wiki/cilkplus-merge) with my name in parenthesis for
items I am working on.  Particularly, I have added a sub-section for
array notation items that have been pointed out in reviews but have not
been completed.  I suggest you keep this list up to date as well, so we
don't loose track of what has been pointed out.

> diff --git a/gcc/c-family/ChangeLog.cilkplus b/gcc/c-family/ChangeLog.cilkplus
> index 6591fd1..10db29b 100644
> --- a/gcc/c-family/ChangeLog.cilkplus
> +++ b/gcc/c-family/ChangeLog.cilkplus
> @@ -1,7 +1,11 @@
> +2013-03-22  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> +
> +     * c-pretty-print.c (pp_c_expression): Added ARRAY_NOTATION_REF case.
> +
>  2013-03-20  Balaji V. Iyer  <balaji.v.iyer@intel.com>
>
>       * c-common.c (c_define_builtins): When cilkplus is enabled, the

You can combine changelog entries into one entry.  This will make it
easier when we merge into mainline.  So basically, add the
c-pretty-print.c entry to the entry below it.

FIXED!!

>> Non-static function declarations like this should not be inside a .c file.
>> If these functions are used outside this file, there should be an associated
>> header that declares them; include it in the .c file.  If only used inside the .c file
>> that defines them, make them static (and topologically sort static functions
>> inside a source file so that forward static declarations are only needed for cases
>> of recursion).
>>
>>> +/* Mark the FNDECL as cold, meaning that the function specified by FNDECL
>> is
>>> +   not run as is.  */
>>
>> The cold attribute means unlikely to be executed rather than "not run as is".
>> Maybe "not run as is" is what's relevant here, but I'm not clear why this attribute
>> would be useful for built-in functions at all - the documentation suggests it's
>> only relevant when a user defines a function themselves, and affects the code
>> generated for that function, so wouldn't be relevant at all for built-in functions.

I see you fixed this.  Since you are only fixing some of the items
Joseph pointed out in this patch, please put "FIXED" below each item you
did to aid in reviewing.

YES. THIS IS FIXED ALSO!

>>
>>> +void
>>> +array_notation_init_builtins (void)
>>
>> Other built-in functions use various .def files (builtins.def and the files it includes)
>> to avoid lots of repetitive code like this - can you integrate this with that
>> mechanism?  If you do so, then you should be able to avoid (or massively
>> simplify) functions like:
>>
>>> +/* Returns true if the function call specified in FUNC_NAME is
>>> +   __sec_implicit_index.  */
>>> +
>>> +bool
>>> +is_sec_implicit_index_fn (tree func_name)
>>
>> because code can use the BUILT_IN_* enum values to test whether a particular
>> function is in use - which is certainly cleaner than using strcmp against the
>> function name.

And here put "FIXED" if fixed, or "Aldy is going to work on this" or
remove it altogether so it's not assumed that it was fixed by this patch
since you're quoting it.

OK.

>>
>>> +/* Returns the first and only argument for FN, which should be a
>>> +   sec_implicit_index function.  FN's location in the source file is is
>>> +   indicated by LOCATION.  */
>>> +
>>> +int
>>> +extract_sec_implicit_index_arg (location_t location, tree fn) {
>>> +  tree fn_arg;
>>> +  HOST_WIDE_INT return_int = 0;
>>> +  if (!fn)
>>> +    return -1;
>>
>> Why the random check for a NULL argument?  If a NULL argument is valid
>> (meaning that it makes the code cleaner to allow such arguments rather than
>> making sure the function isn't called with them), this should be documented in
>> the comment above the function; otherwise, if such an argument isn't valid,
>> there is no need to check for it.
>
> I always tend to check for a null pointer before I access the fields in the structure. In this case it is unnecessary. In some cases (e.g. find_rank) there is a good chance a null pointer will be passed into the function and we need to check that and reject those.

I think what Joseph is suggesting is that if NULL is not valid, then the
caller should check this.  But if NULL is valid, then it should be
documented in the function comment at the top.

FIXED!

>>> +  if (TREE_CODE (fn) == CALL_EXPR)
>>> +    {
>>> +      fn_arg = CALL_EXPR_ARG (fn, 0);
>>> +      if (really_constant_p (fn_arg))
>>
>> I don't think really_constant_p is what's wanted;
>> <http://software.intel.com/sites/default/files/m/4/e/7/3/1/40297-
>> Intel_Cilk_plus_lang_spec_2.htm>
>> says "The argument shall be an integer constant expression.", and such
>> expressions always appear in the C front end as INTEGER_CST.  So you can just
>> check for INTEGER_CST.
>
> What about C++? This function is shared by both C and C++.

Same thing for C++, but...

>
>>
>> Now a subtlety here is that the function argument will have been folded by this
>> point, meaning that cases that aren't integer constant expressions in C standard
>> terms will be wrongly allowed (both by the original code and by a version
>> checking against INTEGER_CST).  In such cases, the way to get things checked
>> correctly is to use a keyword rather than a built-in function - as with
>> __builtin_choose_expr or __builtin_shuffle, for example.  Since this operation
>> seems special in ways that built-in functions generally aren't, that seems
>> reasonable anyway.  So the code parsing this keyword would check that the
>> argument is an INTEGER_CST, of integer type (since INTEGER_CSTs can have
>> pointer type in GCC), like that for __builtin_choose_expr does.  It would then
>> quite likely create its own tree code for the operation, rather than using a
>> CALL_EXPR at all.  (It would need to manage converting to int, given how the
>> specification defines things in terms of a prototype for type int - so e.g. a
>> constant 1ULL << 32 would act like 0 if int is 32 bits, under the present
>> specification.)
>>
>> The specification doesn't seem very clear on to what extent the __sec_*
>> operations must act like functions (what happens if someone puts parentheses
>> around the __sec_* name, for example - that wouldn't work with the keyword
>> approach).  So the specification should be clarified there, but I think saying the
>> __sec_* operations are syntactically special, like keywords, is more appropriate
>> than requiring other uses to work.
>>
>>> +   return_int = (int) int_cst_value (fn_arg);
>>> +      else
>>> +   {
>>> +     if (location == UNKNOWN_LOCATION && EXPR_HAS_LOCATION (fn))
>>> +       location = EXPR_LOCATION (fn);
>>> +     error_at (location, "__sec_implicit_index parameter must be a "
>>> +               "constant integer expression");
>>
>> The term is "integer constant expression" not "constant integer expression".
>
> FIXED!

...it looks like you're going to have to rework all this as a keyword.

OK, CAN I LOOK AT THIS AFTER WE FINISH THE BUILTIN FUNCTION IMPLEMENTATION FIX?

For now, I suggest you check for INTEGER_CST as suggested, put a FIXME
comment explaining that we need to rework this as a keyword, and add an
entry to the wiki as a TODO item.  This way you can attack the rest of
the easier/cosmetic changes Joseph is suggesting without getting bogged
down by the keyword.

Also, can you follow up with the specification changes suggested?

YES, I HAVE CONTACTED THE APPRIOPRIATE PEOPLE. I WILL TRY TO GET ANSWERS TO IT AS SOON AS I GET IT.

> +   parts that should be executed only once that comes with array notation
> +   expressions.  */

You probably want a comma in there somewhere, or split this sentence
somehow.

FIXED!

>
>>
>>> +/* Returns the rank of ARRAY through the *RANK.  The user can specify
>> whether
>>> +   (s)he wants to step into array_notation-specific builtin functions
>>> +   (specified by the IGNORE_BUILTIN_FN).
>>
>> The wording seems awkward; "Set *RANK to the rank of ARRAY, ignoring array-
>> notation-specific built-in functions if IGNORE_BUILTIN_FN." would be better.
>
> Yes, I agree with your wording. Thanks! and FIXED!

> +/* Sets *RANK of expression ARRAY, ignoring array notation specific built-in

^^^^^^^^^^^
Almost, "Set *RANK to the rank of ARRAY, ignoring...".

FIXED!

> +/* Extracts all array notations in NODE ans stores in ARRAY_LIST.  If
> +   IGNORE_BUILTIN_FN is set, then array notations inside array notation
> +   specific builtin functions are ignored.  The NODE can be anything from a
> +   full function to a single variable.  */

s/ans/and

s/stores in/stores them in/

FIXED!

>
>>
>>> +{
>>> +  size_t ii = 0;
>>> +  an_reduce_type dummy_type = REDUCE_UNKNOWN;
>>> +
>>> +  if (!node)
>>> +    return;
>>
>> Again, check for NULL argument without any mention in the comment that such
>> arguments are valid; remove unless there is a reason to make them valid.
>>
>>> +  else if (TREE_CODE (node) == TREE_LIST)
>>
>> What's NODE?  My first guess would have been an expression, but if a TREE_LIST
>> is possible that's clearly not the answer, so explain in the comment above the
>> function what NODE is.  (If a TREE_LIST is being used within expressions to store
>> something specific to array notation, don't do so - TREE_LIST is deprecated,
>> existing uses should be phased out in favour of more specific and less memory-
>> hungry datastructures and new uses should not be added.)
>
> FIXED! What is replacing tree-list? I have used tree-list in my later patches and in my Cilk Plus branch.

For list of things, probably vectors, etc.

> +/* Find all the scalar expressions in *TP and push it in DATA struct,

s/it/them

FIXED!

>>> +bool
>>> +is_builtin_array_notation_fn (tree func_name, an_reduce_type *type) {
>>> +  const char *function_name = NULL;
>>> +
>>> +  if (!func_name)
>>> +    return false;
>>
>> Another unexplained test for a NULL argument.  Again, explain what sort of
>> things FUNC_NAME may be.  (This is another function that should be using
>> BUILT_IN_* enum values rather than strcmp, if you rework how the built-in
>> functions are implemented.)

Still present in the current patch ??

FIXED! I removed the check for !func_name

>
>>
>>> +{
>>> +  if (!t || !contains_array_notation_expr (t))
>>> +    return t;
>>
>> Another check for NULL without a comment saying NULL is a valid argument.
>
> This function also can receive a null pointer.

Then, document that NULL is a valid argument please.

FIXED!

> +  /* Here we assign the array notation components to variable so that we can
> +     satisfy the exec once rule.  */
> +  for (ii = 0; ii < lhs_list_size; ii++)
> +    {
> +      tree array_node = (*lhs_list)[ii];
> +      tree array_begin = ARRAY_NOTATION_START (array_node);
> +      tree array_lngth = ARRAY_NOTATION_LENGTH (array_node);
> +      tree array_strde = ARRAY_NOTATION_STRIDE (array_node);
> +
> +      begin_var = build_decl (location, VAR_DECL, NULL_TREE,
> +                           integer_type_node);
> +      lngth_var = build_decl (location, VAR_DECL, NULL_TREE,
...
...

There's a big chunk of code here that AFAICT is not part of the review.
  Did I miss something, or did this creep in somehow?

THIS CHANGE IS BASED ON A PREVIOUS EMAIL FROM YOU (ALDY) ABOUT CHECKING FOR EXECUTE-ONCE RULE (I.E. ARAY NOTATION TRIPLET VARIABLES ARE ONLY EXECUTED ONCE)

> +
> +  loop = push_stmt_list ();

Similarly here.  What's this bit part of?

THIS CHANGE IS TO ACCOMODATE THE CHANGE ABOVE.

> @@ -2407,7 +2533,42 @@ fix_array_notation_call_expr (tree arg)
>      count_down[ii] = XNEWVEC (bool, rank);
>
>    array_var = XNEWVEC (tree, rank);
> +
> +  loop = push_stmt_list ();
> +  for (ii = 0; ii < list_size; ii++)
> +    {
> +      tree array_node = (*array_list)[ii];
> +      if (array_node && TREE_CODE (array_node) == ARRAY_NOTATION_REF)
> +     {
> +       tree array_begin = ARRAY_NOTATION_START (array_node);
> +       tree array_lngth = ARRAY_NOTATION_LENGTH (array_node);

Also here.  Did I miss something?

PLEASE SEE MY COMMENTS ABOVE:

> +* Cilk Plus Builtins::  Built-in functions that are part of Cilk Plus language
> +                        extension.

Should be "Built-in functions for the Cilk Plus language extension."

FIXED!

> diff --git a/gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
> index 272ef41..82008c0 100644
> --- a/gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
> +++ b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
> @@ -3,16 +3,15 @@ typedef int (*foo)(int);
>  int main(int argc, char **argv)
>  {
>    int array[10], array2[10][10];
> -  // int array[10], array2[10], value, ii = 0;

Do not add commented out code.

OK. I BELIEVE THE PATCH IS SUPPOSED TO REMOVE THE COMMENT OUT. PLEASE LET ME KNOW IF I HAVE MISTAKEN SOMETHING.

> diff --git a/gcc/testsuite/gcc.dg/cilk-plus/array_notation/compile/array_test2.c b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/compile/array_test2.c
> index 5fb3680..fd128b1 100644
> --- a/gcc/testsuite/gcc.dg/cilk-plus/array_notation/compile/array_test2.c
> +++ b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/compile/array_test2.c
> @@ -26,7 +26,7 @@ int main(int argc, char **argv)
>        array[ii] = 10;
>        array2[ii] = 5000000;
>      }
> -  array2[0:10:2] = array[0:10:2];
> +  array2[0:5:2] = array[0:5:2];

Is this change part of this review cycle, or is this something else?  If
the latter, then submit it as a separate patch.

THIS WAS A SMALL BUG FIX IN THE TEST CODE.

> diff --git a/gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
> index 272ef41..82008c0 100644
> --- a/gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
> +++ b/gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
> @@ -3,16 +3,15 @@ typedef int (*foo)(int);
>  int main(int argc, char **argv)
>  {
>    int array[10], array2[10][10];
> -  // int array[10], array2[10], value, ii = 0;

Do not add commented out code.

FIXED!

Thanks for following up on this.

Could you tackle the changes I have suggested and repost the patch?

Thanks.
diff --git gcc/c-family/ChangeLog.cilkplus gcc/c-family/ChangeLog.cilkplus
index 6591fd1..eb7ff47 100644
--- gcc/c-family/ChangeLog.cilkplus
+++ gcc/c-family/ChangeLog.cilkplus
@@ -1,7 +1,7 @@
 2013-03-20  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.
+	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.
@@ -18,5 +18,6 @@
 	(array_notation_reduce_type): New enumerator.
 	* 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.
diff --git gcc/c-family/array-notation-common.c gcc/c-family/array-notation-common.c
index 7089c8e..b775225 100644
--- gcc/c-family/array-notation-common.c
+++ gcc/c-family/array-notation-common.c
@@ -29,19 +29,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-iterator.h"
 #include "diagnostic-core.h"
 
-int extract_sec_implicit_index_arg (location_t, tree);
-bool is_sec_implicit_index_fn (tree);
-void array_notation_init_builtins (void);
-
-/* Mark the FNDECL as cold, meaning that the function specified by FNDECL is
-   not run as is.  */
-
-static void
-mark_cold (tree fndecl)
-{
-  DECL_ATTRIBUTES (fndecl) = tree_cons (get_identifier ("cold"), NULL_TREE,
-					DECL_ATTRIBUTES (fndecl));
-}
 
 /* This function inititializes array notation specific builtin information.  */
 
@@ -54,67 +41,56 @@ array_notation_init_builtins (void)
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_add", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_mul", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_all_zero", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_any_zero", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_max", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
   
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_min", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_min_ind", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_max_ind", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 				       NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_any_nonzero", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_reduce_all_nonzero", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
   
   func_type = build_function_type_list (integer_type_node, integer_type_node,
 					NULL_TREE);
   new_func = build_fn_decl ("__sec_implicit_index", func_type);
-  mark_cold (new_func);
   new_func = lang_hooks.decls.pushdecl (new_func);
 
   func_type = build_function_type_list (integer_type_node, ptr_type_node,
@@ -167,25 +143,23 @@ is_sec_implicit_index_fn (tree func_name)
    sec_implicit_index function.  FN's location in the source file is is 
    indicated by LOCATION.  */
 
-int
+HOST_WIDE_INT
 extract_sec_implicit_index_arg (location_t location, tree fn)
 {
   tree fn_arg;
   HOST_WIDE_INT return_int = 0;
-  if (!fn)
-    return -1;
 
   if (TREE_CODE (fn) == CALL_EXPR)
     {
       fn_arg = CALL_EXPR_ARG (fn, 0);
       if (really_constant_p (fn_arg))
-	return_int = (int) int_cst_value (fn_arg);
+	return_int = int_cst_value (fn_arg);
       else
 	{
 	  if (location == UNKNOWN_LOCATION && EXPR_HAS_LOCATION (fn))
 	    location = EXPR_LOCATION (fn);
 	  error_at (location, "__sec_implicit_index parameter must be a " 
-		    "constant integer expression");
+		    "integer constant expression");
 	  return -1;
 	}
     }
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index edcff2e..001e0c4 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -541,7 +541,7 @@ extern tree build_modify_expr (location_t, tree, tree, enum tree_code,
 extern tree build_array_notation_expr (location_t, tree, tree, enum tree_code,
 				       location_t, tree, tree);
 extern tree build_array_notation_ref (location_t, tree, tree, tree, tree, tree);
-extern void find_rank (tree, bool, size_t *);
+extern bool find_rank (location_t, tree, tree, bool, size_t *);
 extern tree build_indirect_ref (location_t, tree, ref_operator);
 
 extern int field_decl_cmp (const void *, const void *);
@@ -1173,5 +1173,10 @@ typedef enum array_notation_reduce_type {
 extern int extract_sec_implicit_index_arg (location_t, tree);
 extern bool is_sec_implicit_index_fn (tree);
 extern void array_notation_init_builtins (void);
-
+extern struct c_expr fix_array_notation_expr (location_t, enum tree_code, 
+					      struct c_expr);
+extern bool contains_array_notation_expr (tree);
+extern tree expand_array_notation_exprs (tree);
+extern tree fix_conditional_array_notations (tree);
+extern tree find_correct_array_notation_type (tree);
 #endif /* ! GCC_C_COMMON_H */
diff --git gcc/c-family/c-pretty-print.c gcc/c-family/c-pretty-print.c
index 30c8e80..b8af90c 100644
--- gcc/c-family/c-pretty-print.c
+++ gcc/c-family/c-pretty-print.c
@@ -1479,11 +1479,11 @@ pp_c_postfix_expression (c_pretty_printer *pp, tree e)
     case ARRAY_NOTATION_REF:
       pp_postfix_expression (pp, ARRAY_NOTATION_ARRAY (e));
       pp_c_left_bracket (pp);
-      pp_postfix_expression (pp, ARRAY_NOTATION_START (e));
+      pp_expression (pp, ARRAY_NOTATION_START (e));
       pp_colon (pp);
-      pp_postfix_expression (pp, ARRAY_NOTATION_LENGTH (e));
+      pp_expression (pp, ARRAY_NOTATION_LENGTH (e));
       pp_colon (pp);
-      pp_postfix_expression (pp, ARRAY_NOTATION_STRIDE (e));
+      pp_expression (pp, ARRAY_NOTATION_STRIDE (e));
       pp_c_right_bracket (pp);
       break;
       
@@ -2161,6 +2161,7 @@ pp_c_expression (c_pretty_printer *pp, tree e)
     case POSTINCREMENT_EXPR:
     case POSTDECREMENT_EXPR:
     case ARRAY_REF:
+    case ARRAY_NOTATION_REF:
     case CALL_EXPR:
     case COMPONENT_REF:
     case BIT_FIELD_REF:
diff --git gcc/c/c-array-notation.c gcc/c/c-array-notation.c
index 38ed7ea..9ee281e 100755
--- gcc/c/c-array-notation.c
+++ gcc/c/c-array-notation.c
@@ -31,19 +31,17 @@
 #include "gcc.h"
 #include "c-family/c-common.h"
 
-void replace_array_notations (tree *, bool, vec<tree, va_gc> *,
+static void replace_array_notations (tree *, bool, vec<tree, va_gc> *,
 			      vec<tree, va_gc> *);
-void find_rank (tree, bool, size_t *);
-void extract_array_notation_exprs (tree, bool, vec<tree, va_gc> **);
-tree fix_conditional_array_notations (tree);
-struct c_expr fix_array_notation_expr (location_t, enum tree_code,
-				       struct c_expr);
-bool is_builtin_array_notation_fn (tree func_name, an_reduce_type *type);
+static void extract_array_notation_exprs (tree, bool, vec<tree, va_gc> **);
+static bool is_builtin_array_notation_fn (tree func_name, an_reduce_type *type);
 static tree fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var);
-bool contains_array_notation_expr (tree expr);
-tree expand_array_notation_exprs (tree t);
 
 
+/* This structure holds all the scalar values and its appropriate variable 
+   replacment.  It is mainly used by the function that pulls all the invariant
+   parts that should be executed only once, which comes with array notation 
+   expressions.  */
 struct inv_list
 {
   vec<tree, va_gc> *list_values;
@@ -51,72 +49,84 @@ struct inv_list
 };
 
 
-/* Returns the rank of ARRAY through the *RANK.  The user can specify whether
-   (s)he wants to step into array_notation-specific builtin functions
-   (specified by the IGNORE_BUILTIN_FN).
+/* 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.  If ARRAY can be NULL, since it is
+   recursively accessing all the fields in a subtree.  If so, then just return
+   true.
 
    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.  */
 
-void
-find_rank (tree array, bool ignore_builtin_fn, size_t *rank)
+bool
+find_rank (location_t loc, tree orig_expr, tree array, bool ignore_builtin_fn,
+	   size_t *rank)
 {
   tree ii_tree;
-  size_t current_rank = 0, ii = 0;
+  size_t ii = 0, current_rank = 0;
   an_reduce_type dummy_type = REDUCE_UNKNOWN;
+  
   if (!array)
-    return;
+    return true;
   else if (TREE_CODE (array) == ARRAY_NOTATION_REF)
     {
       for (ii_tree = array;
 	   ii_tree && TREE_CODE (ii_tree) == ARRAY_NOTATION_REF;
 	   ii_tree = ARRAY_NOTATION_ARRAY (ii_tree))
 	current_rank++;
-      if (*rank == 0)
-	*rank = current_rank;
+	if (*rank == 0)
+	  *rank = current_rank;
+	else if (*rank != current_rank)
+	  {
+	    error_at (loc, "rank mismatch in expression %qE", orig_expr);
+	    return false;
+	  }
     }
   else if (TREE_CODE (array) == STATEMENT_LIST)
     {
       tree_stmt_iterator ii_tsi;
       for (ii_tsi = tsi_start (array); !tsi_end_p (ii_tsi);
 	   tsi_next (&ii_tsi))
-	find_rank (*tsi_stmt_ptr (ii_tsi), ignore_builtin_fn, rank);
+	if (!find_rank (loc, orig_expr, *tsi_stmt_ptr (ii_tsi),
+			ignore_builtin_fn, rank))
+	  return false;
     }
   else
     {
       if (TREE_CODE (array) == CALL_EXPR)
 	{
 	  tree func_name = CALL_EXPR_FN (array);
+	  tree arg;
+	  call_expr_arg_iterator iter;
 	  if (TREE_CODE (func_name) == ADDR_EXPR)
 	    if (!ignore_builtin_fn)
 	      if (is_builtin_array_notation_fn (func_name, &dummy_type))
-		/* If it is a builtin function, then we know it returns a 
-		   scalar.  */
-		return;
-	  if (TREE_CODE (TREE_OPERAND (array, 0)) == INTEGER_CST)
-	    {
-	      int length = TREE_INT_CST_LOW (TREE_OPERAND (array, 0));
-	      for (ii = 0; ii < (size_t) length; ii++)
-		find_rank (TREE_OPERAND (array, ii), ignore_builtin_fn, rank);
-	    }
-	  else
-	    gcc_unreachable ();
+		/* If it is a builtin function, then it returns a scalar.  */
+		return true;
+
+	  FOR_EACH_CALL_EXPR_ARG (arg, iter, array)
+	    if (!find_rank (loc, orig_expr, arg, ignore_builtin_fn, rank))
+	      return false;
 	}
       else 
 	for (ii = 0; ii < TREE_CODE_LENGTH (TREE_CODE (array)); ii++) 
-	  find_rank (TREE_OPERAND (array, ii), ignore_builtin_fn, rank);
+	  if (!find_rank (loc, orig_expr, TREE_OPERAND (array, ii),
+			  ignore_builtin_fn, rank))
+	    return false;
     }
-  return;
+  return true;
 }
   
 
-/* Extracts all the array notations specified in NODE and stores them in a
-   dynamic tree array of ARRAY_LIST whose size is stored in *LIST_SIZE.  The
-   user can specify if (s)he wants to ignore the array notations inside the
-   array-notation specific builtin functions (by setting IGNORE_BUILTIN_FN to
-   true).  */
+/* 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 builtin functions are ignored.  The NODE can be anything from a
+   full function to a single variable.  Since function is stepping through all
+   the subtrees, there is a good chance that node can be a NULL_TREE.  If so,
+   then the function just returns.  */
 
-void
+static void
 extract_array_notation_exprs (tree node, bool ignore_builtin_fn,
 			      vec<tree, va_gc> **array_list)
 {
@@ -148,6 +158,8 @@ extract_array_notation_exprs (tree node, bool ignore_builtin_fn,
     }
   else if (TREE_CODE (node) == CALL_EXPR)
     {
+      tree arg;
+      call_expr_arg_iterator iter;
       if (is_builtin_array_notation_fn (CALL_EXPR_FN (node), &dummy_type))
 	{
 	  if (ignore_builtin_fn)
@@ -163,17 +175,9 @@ extract_array_notation_exprs (tree node, bool ignore_builtin_fn,
 	  vec_safe_push (*array_list, node);
 	  return;
 	}
-      if (TREE_CODE (TREE_OPERAND (node, 0)) == INTEGER_CST)
-	{
-	  int length = TREE_INT_CST_LOW (TREE_OPERAND (node, 0));
 
-	  for (ii = 0; ii < (size_t) length; ii++)
-	    extract_array_notation_exprs
-	      (TREE_OPERAND (node, ii), ignore_builtin_fn, array_list);
-	}
-      else
-	gcc_unreachable (); /* We should not get here.  */
-	  
+      FOR_EACH_CALL_EXPR_ARG (arg, iter, node)
+	extract_array_notation_exprs (arg, ignore_builtin_fn, array_list);
     } 
   else 
     for (ii = 0; ii < TREE_CODE_LENGTH (TREE_CODE (node)); ii++) 
@@ -183,15 +187,15 @@ extract_array_notation_exprs (tree node, bool ignore_builtin_fn,
 }
 
 
-/* Replaces all occurances of array notations in tree ORIG that matches the
-   ones in LIST with the one in ARRAY_OPERAND.  The size of list and
-   ARRAY_OPERAND is ARRAY_SIZE.  For example, ARRAY_OPERAND[x] for some index
-   'x' will have the equivalent ARRAY_REF for the ARRAY_NOTATION_REF specified
-   in LIST[x].   The  user can specify if (s)he wants to ignore the array
-   notations inside the array-notation specific builtin functions (using the
-   bool variable IGNORE_BUILTIN_FN).  */
+/* Replaces all the occurances of array notations in *LIST with the appropriate
+   one in ARRAY_OPERAND.  If IGNORE_BUILTIN_FN is set, then array notations
+   inside array-notation specific builtin functions are ignored.  ORIG can be
+   anything from a collection of statement lists to a single variable.  Since
+   this function steps through all the subtrees, it is probable that *ORIG can
+   be a NULL_TREE.  If so, then the function just returns.
+*/
 
-void
+static void
 replace_array_notations (tree *orig, bool ignore_builtin_fn,
 			 vec<tree, va_gc> *list,
 			 vec<tree, va_gc> *array_operand)
@@ -221,6 +225,8 @@ replace_array_notations (tree *orig, bool ignore_builtin_fn,
     }
   else if (TREE_CODE (*orig) == CALL_EXPR)
     {
+      tree arg;
+      call_expr_arg_iterator iter;
       if (is_builtin_array_notation_fn (CALL_EXPR_FN (*orig), &dummy_type))
 	{
 	  if (!ignore_builtin_fn)
@@ -244,16 +250,14 @@ replace_array_notations (tree *orig, bool ignore_builtin_fn,
 	      }
 	  return;
 	}
-      if (TREE_CODE (TREE_OPERAND (*orig, 0)) == INTEGER_CST)
+      ii = 0;
+      FOR_EACH_CALL_EXPR_ARG (arg, iter, *orig)
 	{
-	  int length = TREE_INT_CST_LOW (TREE_OPERAND (*orig, 0));
-	  for (ii = 0; ii < (size_t) length; ii++)
-	    replace_array_notations
-	      (&TREE_OPERAND (*orig, ii), ignore_builtin_fn, list,
-	       array_operand);
-	}
-      else
-	gcc_unreachable (); /* We should not get here!  */
+	  replace_array_notations (&arg, ignore_builtin_fn, list,
+				   array_operand);
+	  CALL_EXPR_ARG (*orig, ii) = arg;
+	  ii++;
+	}     
     }
   else
     {
@@ -264,9 +268,11 @@ replace_array_notations (tree *orig, bool ignore_builtin_fn,
   return;
 }
 
-/* This function will find all the scalar expressions in *TP and push it in
-   DATA struct, typecasted to (void *).  If *WALK_SUBTREES is set to 0 then
-   we have do not go into the *TP's subtrees.  */
+/* Find all the scalar expressions in *TP and push them in DATA struct, 
+   typecasted to (void *).  If *WALK_SUBTREES is set to 0 then do not go into 
+   the *TP's subtrees.  Since this function steps through all the subtrees, *TP
+   and TP can be both NULL_TREE and NULL respectively.  If so, then the function
+   just returns NULL_TREE.  */
 
 static tree
 find_inv_trees (tree *tp, int *walk_subtrees, void *data)
@@ -322,7 +328,11 @@ replace_inv_trees (tree *tp, int *walk_subtrees, void *data)
   return NULL_TREE;
 }
 
-/* Replaces all the scalar expressions in *NODE. */
+/* Replaces all the scalar expressions in *NODE.  Returns a STATEMENT_LIST that
+   holds the NODE along with variables that holds the results of the invariant
+   expressions.  Since this function steps through all the subtrees, it is
+   probable than *NODE or NODE can be NULL_TREE and NULL respectively.  If so,
+   then the function just returns NULL_TREE.  */
 
 tree
 replace_invariant_exprs (tree *node)
@@ -331,6 +341,7 @@ replace_invariant_exprs (tree *node)
   tree node_list = NULL_TREE;
   tree t = NULL_TREE, new_var = NULL_TREE, new_node; 
   struct inv_list data;
+
   if (!node || !*node)
     return NULL_TREE;
 
@@ -398,7 +409,9 @@ build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype,
      already.  Not necessary to send out multiple error messages.  */
   if (lhs == error_mark_node || rhs == error_mark_node)
     return error_mark_node;
-  find_rank (rhs, false, &rhs_rank);
+  
+  if (!find_rank (location, rhs, rhs, false, &rhs_rank))
+    return error_mark_node;
   
   extract_array_notation_exprs (rhs, false, &rhs_list);
   rhs_list_size = vec_safe_length (rhs_list);
@@ -435,8 +448,11 @@ build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype,
 
   lhs_rank = 0;
   rhs_rank = 0;
-  find_rank (lhs, true, &lhs_rank);
-  find_rank (rhs, true, &rhs_rank);
+  if (!find_rank (location, lhs, lhs, true, &lhs_rank))
+    return error_mark_node;
+  
+  if (!find_rank (location, rhs, rhs, true, &rhs_rank))
+    return error_mark_node;
 
   if (lhs_rank == 0 && rhs_rank == 0)
     {
@@ -467,12 +483,22 @@ build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype,
   if (lhs_rank == 0 && rhs_rank != 0 && TREE_CODE (rhs) != CALL_EXPR)
     {
       tree rhs_base = rhs;
-      for (ii = 0; ii < (size_t) rhs_rank; ii++)
-	rhs_base = ARRAY_NOTATION_ARRAY (rhs);
+      if (TREE_CODE (rhs_base) == ARRAY_NOTATION_REF)
+	{
+	  for (ii = 0; ii < (size_t) rhs_rank; ii++)
+	    rhs_base = ARRAY_NOTATION_ARRAY (rhs);
       
-      error_at (location, "%qD cannot be scalar when %qD is not", lhs,
-		rhs_base);
-      return error_mark_node;
+	  error_at (location, "%qE cannot be scalar when %qE is not", lhs,
+		    rhs_base);
+	  return error_mark_node;
+	}
+      else
+	{
+	  error_at (location, "%qE cannot be scalar when %qE is not", lhs,
+		    rhs_base);
+	  return error_mark_node;
+	}
+
     }
   if (lhs_rank != 0 && rhs_rank != 0 && lhs_rank != rhs_rank)
     {
@@ -745,8 +771,6 @@ build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype,
 	  rhs_vector[ii][jj] = false;
     }
 
-
-
   for (ii = 0; ii < lhs_rank; ii++)
     {
       if (lhs_vector[0][ii])
@@ -758,7 +782,6 @@ build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype,
 	     NOP_EXPR,
 	     location, build_zero_cst (TREE_TYPE (lhs_var[ii])),
 	     TREE_TYPE (lhs_var[ii]));
-	  
 	}
     }
 
@@ -1032,19 +1055,19 @@ build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype,
     }
   
   /* The following statements will do the following:
-   * <if_stmt_label>: (in order from outermost to innermost)
-   *                  if (cond_expr) then go to body_label
-   *                  else                go to exit_label
-   * <body_label>:
-   *                  array expression
-   *
-   *                  (the increment, goto and exit_label goes from innermost to
-   *                   outermost).
-   *                  ii++ and jj++
-   *                  go to if_stmt_label
-   * <exit_label>:
-   *                  <REST OF CODE>
-   */
+     <if_stmt_label>: (in order from outermost to innermost)
+                      if (cond_expr) then go to body_label
+                      else                go to exit_label
+     <body_label>:
+                      array expression
+   
+     (the increment, goto and exit_label goes from innermost to
+     outermost).
+                      ii++ and jj++
+                      go to if_stmt_label
+     <exit_label>:
+     <REST OF CODE>
+  */
 
   
   for (ii = 0; ii < MAX (lhs_rank, rhs_rank); ii++)
@@ -1079,7 +1102,9 @@ build_array_notation_expr (location_t location, tree lhs, tree lhs_origtype,
 
 /* Encloses the conditional statement passed in STMT with a loop around it
    and replaces the condition in STMT with a ARRAY_REF tree-node to the array.
-   The condition must have a ARRAY_NOTATION_REF tree.  */
+   The condition must have a ARRAY_NOTATION_REF tree.  An expansion of array
+   notation in STMT is returned in a STATEMENT_LIST.  */
+   
 
 static tree
 fix_conditional_array_notations_1 (tree stmt)
@@ -1105,7 +1130,11 @@ fix_conditional_array_notations_1 (tree stmt)
     /* Otherwise dont even touch the statement.  */
     return stmt;
 
-  find_rank (cond, false, &rank);
+  location = EXPR_LOCATION (stmt);
+  
+  if (!find_rank (location, cond, cond, false, &rank))
+    return error_mark_node;
+  
   extract_array_notation_exprs (cond, false, &array_list);
   loop = push_stmt_list ();
   for (ii = 0; ii < vec_safe_length (array_list); ii++)
@@ -1130,7 +1159,9 @@ fix_conditional_array_notations_1 (tree stmt)
 	    }
 	}
     }
-  find_rank (cond, true, &rank);
+
+  if (!find_rank (location, cond, cond, true, &rank))
+    return error_mark_node;
   if (rank == 0)
     {
       add_stmt (stmt);
@@ -1144,7 +1175,6 @@ fix_conditional_array_notations_1 (tree stmt)
     return stmt;
 
   list_size = vec_safe_length (array_list);
-  location = EXPR_LOCATION (stmt);
 
   array_ops =  XNEWVEC (tree *, list_size);
   for (ii = 0; ii < list_size; ii++)
@@ -1278,7 +1308,6 @@ fix_conditional_array_notations_1 (tree stmt)
 			   location,
 			   build_int_cst (TREE_TYPE (array_var[ii]), 0),
 			   TREE_TYPE (array_var[ii]));
-	
     }
 
   for (ii = 0; ii < rank ; ii++)
@@ -1457,7 +1486,15 @@ fix_array_notation_expr (location_t location, enum tree_code code,
   tree *compare_expr, *if_stmt_label, *expr_incr, *ind_init;
   bool **count_down, **array_vector;
   
-  find_rank (arg.value, false, &rank);
+  if (!find_rank (location, arg.value, arg.value, false, &rank))
+    {
+      /* If this function returns a NULL, we convert the tree value in the
+	 structure to error_mark_node and the parser should take care of the
+	 rest.  */
+      arg.value = error_mark_node;
+      return arg;
+    }
+  
   if (rank == 0)
     return arg;
   
@@ -1634,12 +1671,10 @@ fix_array_notation_expr (location_t location, enum tree_code code,
   replace_array_notations (&arg.value, true, array_list, array_operand);
 
   for (ii = 0; ii < rank; ii++)
-    {
-      expr_incr[ii] =
-	build2 (MODIFY_EXPR, void_type_node, array_var[ii],
-		build2 (PLUS_EXPR, TREE_TYPE (array_var[ii]), array_var[ii],
-			build_int_cst (TREE_TYPE (array_var[ii]), 1)));
-    }
+    expr_incr[ii] =
+      build2 (MODIFY_EXPR, void_type_node, array_var[ii],
+	      build2 (PLUS_EXPR, TREE_TYPE (array_var[ii]), array_var[ii],
+		      build_int_cst (TREE_TYPE (array_var[ii]), 1)));
   
   for (jj = 0; jj < rank; jj++)
     {
@@ -1656,7 +1691,6 @@ fix_array_notation_expr (location_t location, enum tree_code code,
 				       array_var[jj], array_length[0][jj]);
 	}
     }
-
   
   for (ii = 0; ii < rank; ii++)
     {
@@ -1772,10 +1806,11 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
 	 || TREE_CODE (func_parm) == EXCESS_PRECISION_EXPR
 	 || TREE_CODE (func_parm) == NOP_EXPR)
     func_parm = TREE_OPERAND (func_parm, 0);
-  
-  find_rank (an_builtin_fn, true, &rank);
 
   location = EXPR_LOCATION (an_builtin_fn);
+  
+  if (!find_rank (location, an_builtin_fn, an_builtin_fn, true, &rank))
+    return error_mark_node;
  
   if (rank == 0)
     return an_builtin_fn;
@@ -1783,7 +1818,7 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
 	   && (an_type == REDUCE_MAX_INDEX  || an_type == REDUCE_MIN_INDEX))
     {
       error_at (location, "__sec_reduce_min_ind or __sec_reduce_max_ind cannot"
-		" have arrays with dimension greater than 1.");
+		" have arrays with dimension greater than 1");
       return error_mark_node;
     }
   
@@ -1815,7 +1850,7 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
       new_var_type = NULL_TREE;
       break;
     default:
-      gcc_unreachable ();  /* You should not reach here.  */
+      gcc_unreachable (); 
     }
   
   array_ops = XNEWVEC (tree *, list_size);
@@ -1982,12 +2017,11 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
     }
   replace_array_notations (&func_parm, true, array_list, array_operand);
   for (ii = 0; ii < rank; ii++)
-    {
-      expr_incr[ii] =
-	build2 (MODIFY_EXPR, void_type_node, array_var[ii],
-		build2 (PLUS_EXPR, TREE_TYPE (array_var[ii]), array_var[ii],
-			build_int_cst (TREE_TYPE (array_var[ii]), 1)));
-    }
+    expr_incr[ii] =
+      build2 (MODIFY_EXPR, void_type_node, array_var[ii],
+	      build2 (PLUS_EXPR, TREE_TYPE (array_var[ii]), array_var[ii],
+		      build_int_cst (TREE_TYPE (array_var[ii]), 1)));
+    
   
   for (jj = 0; jj < rank; jj++)
     {
@@ -2321,17 +2355,14 @@ fix_builtin_array_notation_fn (tree an_builtin_fn, tree *new_var)
   return loop;
 }
 
-/* Returns true of FUNC_NAME is a builtin array notation function.  The type of
+/* Returns true if FUNC_NAME is a builtin array notation function.  The type of
    function is returned in *TYPE.  */
 
-bool
+static bool
 is_builtin_array_notation_fn (tree func_name, an_reduce_type *type)
 {
   const char *function_name = NULL;
 
-  if (!func_name)
-    return false;
-
   if (TREE_CODE (func_name) == IDENTIFIER_NODE)
     function_name = IDENTIFIER_POINTER (func_name);
   else if (TREE_CODE (func_name) == ADDR_EXPR)
@@ -2413,7 +2444,7 @@ is_builtin_array_notation_fn (tree func_name, an_reduce_type *type)
 }
 
 
-/* Returns true of EXPR (and its subtrees) contain ARRAY_NOTATION_EXPR node.  */
+/* Returns true if EXPR (and its subtrees) contain ARRAY_NOTATION_EXPR node.  */
 
 bool
 contains_array_notation_expr (tree expr)
@@ -2434,9 +2465,8 @@ contains_array_notation_expr (tree expr)
     return true;
 }
 
-/* Replaces array notations in void function call arguments in ARG with loop and
-   tree-node ARRAY_REF and returns that value in a tree node variable called
-   LOOP.  */
+/* Replaces array notations in void function call arguments in ARG and returns
+   a STATEMENT_LIST.  */
 
 static tree
 fix_array_notation_call_expr (tree arg)
@@ -2463,12 +2493,13 @@ fix_array_notation_call_expr (tree arg)
       return loop;
     }
   
-  find_rank (arg, false, &rank);
+  if (!find_rank (location, arg, arg, false, &rank))
+    return error_mark_node;
+  
   if (rank == 0)
     return arg;
   
   extract_array_notation_exprs (arg, true, &array_list);
-
   if (vec_safe_length (array_list) == 0)
     return arg;
   
@@ -2747,7 +2778,10 @@ fix_array_notation_call_expr (tree arg)
 
 
 /* Walks through tree node T and find all the call-statments that do not return
-   anything and fix up any array notations they may carry.  */
+   anything and fix up any array notations they may carry.  The return value
+   is the same type as T but with all array notations replaced with appropriate
+   STATEMENT_LISTS.  Since this function recursively steps through all the
+   subtrees, t can be a NULL_TREE.  If so, then the function just returns.  */
 
 tree
 expand_array_notation_exprs (tree t)
@@ -2802,20 +2836,22 @@ build_array_notation_ref (location_t loc, tree array, tree start_index,
   tree array_ntn_tree = NULL_TREE;
   size_t stride_rank = 0, length_rank = 0, start_rank = 0;
   
-  if (!TREE_TYPE (start_index) || !INTEGRAL_TYPE_P (TREE_TYPE (start_index)))
+  if (!INTEGRAL_TYPE_P (TREE_TYPE (start_index)))
     {
       error_at (loc,
-		"start-index of array notation triplet is not an integer.");
+		"start-index of array notation triplet is not an integer");
       return error_mark_node;
     }
-  if (!TREE_TYPE (length) || !INTEGRAL_TYPE_P (TREE_TYPE (length)))
+  if (!INTEGRAL_TYPE_P (TREE_TYPE (length)))
     {
-      error_at (loc, "length of array notation triplet is not an integer.");
+      error_at (loc, "length of array notation triplet is not an integer");
       return error_mark_node;
     }
-  if (stride && (!TREE_TYPE (stride) || !INTEGRAL_TYPE_P (TREE_TYPE (stride))))
+
+  /* The stride is an optional field.  */
+  if (stride && !INTEGRAL_TYPE_P (TREE_TYPE (stride)))
     {
-      error_at (loc, "stride of array notation triplet is not an integer.");
+      error_at (loc, "stride of array notation triplet is not an integer");
       return error_mark_node;
     }  
   if (!stride)
@@ -2827,24 +2863,27 @@ build_array_notation_ref (location_t loc, tree array, tree start_index,
 	stride = build_int_cst (TREE_TYPE (start_index), 1);
     }	      
 
-  find_rank (start_index, false, &start_rank);
-  find_rank (length, false, &length_rank);
-  find_rank (stride, false, &stride_rank);
+  if (!find_rank (loc, start_index, start_index, false, &start_rank))
+    return error_mark_node;
+  if (!find_rank (loc, length, length, false, &length_rank))
+    return error_mark_node;
+  if (!find_rank (loc, stride, stride, false, &stride_rank))
+    return error_mark_node;
 
   if (start_rank != 0)
     {
       error_at (loc, "rank of an array notation triplet's start-index is not "
-		"zero.");
+		"zero");
       return error_mark_node;
     }
   if (length_rank != 0)
     {
-      error_at (loc, "rank of an array notation triplet's length is not zero.");
+      error_at (loc, "rank of an array notation triplet's length is not zero");
       return error_mark_node;
     }
   if (stride_rank != 0)
     {
-      error_at (loc, "rank of array notation triplet's stride is not zero.");
+      error_at (loc, "rank of array notation triplet's stride is not zero");
       return error_mark_node;
     }
   
@@ -2884,4 +2923,3 @@ find_correct_array_notation_type (tree op)
     } 
   return return_type;
 }
-
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index ba0a7f9..f051ab5 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -56,12 +56,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "plugin.h"
 
 
-extern bool contains_array_notation_expr (tree);
-extern struct c_expr fix_array_notation_expr (location_t, enum tree_code,
-					      struct c_expr);
-extern tree fix_conditional_array_notations (tree);
-extern tree expand_array_notation_exprs (tree);
-
 
 /* Initialization routine for this file.  */
 
@@ -3082,7 +3076,7 @@ c_parser_direct_declarator_inner (c_parser *parser, bool id_present,
 	      dimen = error_mark_node;
 	      star_seen = false;
 	      error_at (c_parser_peek_token (parser)->location,
-			"array notations cannot be used in declaration.");
+			"array notations cannot be used in declaration");
 	      c_parser_consume_token (parser);
 	    }   
 	  else if (c_parser_next_token_is (parser, CPP_MULT))
@@ -3111,7 +3105,7 @@ c_parser_direct_declarator_inner (c_parser *parser, bool id_present,
 	       && c_parser_next_token_is (parser, CPP_COLON))
 	{
 	  error_at (c_parser_peek_token (parser)->location,
-		    "array notations cannot be used in declaration.");
+		    "array notations cannot be used in declaration");
 	  c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, NULL);
 	  return NULL;
 	}
@@ -5068,10 +5062,11 @@ c_parser_for_statement (c_parser *parser)
 	      if (flag_enable_cilkplus && contains_array_notation_expr (cond))
 		{
 		  error_at (loc, "array notations cannot be used in a "
-			    "condition for a for-loop.");
+			    "condition for a for-loop");
 		  cond = error_mark_node;
 		}
-	      c_parser_skip_until_found (parser, CPP_SEMICOLON, "expected %<;%>");
+	      c_parser_skip_until_found (parser, CPP_SEMICOLON,
+					 "expected %<;%>");
 	    }
 	}
       /* Parse the increment expression (the third expression in a
@@ -11023,9 +11018,8 @@ c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
   tree start_index = NULL_TREE, end_index = NULL_TREE, stride = NULL_TREE;
   tree value_tree = NULL_TREE, type = NULL_TREE, array_type = NULL_TREE;
   tree array_type_domain = NULL_TREE; 
-  double_int x;
 
-  if (!array_value || array_value == error_mark_node)
+  if (array_value == error_mark_node)
     {
       /* No need to continue.  If either of these 2 were true, then an error
 	 must be emitted already.  Thus, no need to emit them twice.  */
@@ -11038,7 +11032,7 @@ c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
   type = TREE_TYPE (array_type);
   token = c_parser_peek_token (parser);
    
-  if (token == NULL)
+  if (token->type == CPP_EOF)
     {
       c_parser_error (parser, "expected %<:%> or numeral");
       return value_tree;
@@ -11052,14 +11046,14 @@ c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
 	  if (TREE_CODE (array_type) == POINTER_TYPE)
 	    {
 	      error_at (loc, "start-index and length fields necessary for "
-			"using array notations in pointers.");
+			"using array notations in pointers");
 	      c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, NULL);
 	      return error_mark_node;
 	    }
 	  if (TREE_CODE (array_type) == FUNCTION_TYPE)
 	    {
 	      error_at (loc, "array notations cannot be used with function "
-			"type.");
+			"type");
 	      c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, NULL);
 	      return error_mark_node;
 	    }
@@ -11074,7 +11068,7 @@ c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
 		  if (subtype && TREE_CODE (subtype) == FUNCTION_TYPE)
 		    {
 		      error_at (loc, "array notations cannot be used with "
-				"function pointer arrays.");
+				"function pointer arrays");
 		      c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE,
 						 NULL);
 		      return error_mark_node;
@@ -11086,7 +11080,7 @@ c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
 	  if (!array_type_domain)
 	    {
 	      error_at (loc, "start-index and length fields necessary for "
-			"using array notations in dimensionless arrays.");
+			"using array notations in dimensionless arrays");
 	      c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, NULL);
 	      return error_mark_node;
 	    }
@@ -11098,13 +11092,13 @@ c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
 	      || !TREE_CONSTANT (TYPE_MAXVAL (array_type_domain)))
 	    {
 	      error_at (loc, "start-index and length fields necessary for "
-			"using array notations in variable-length arrays.");
+			"using array notations in variable-length arrays");
 	      c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, NULL);
 	      return error_mark_node;
 	    }
-	  x = TREE_INT_CST (TYPE_MAXVAL (array_type_domain));
-	  x.low++;
-	  end_index = double_int_to_tree (integer_type_node, x);
+	  end_index = TYPE_MAXVAL (array_type_domain);
+	  end_index = fold_build2 (PLUS_EXPR, TREE_TYPE (end_index),
+				   end_index, integer_one_node);
 	  end_index = fold_build1 (CONVERT_EXPR, ptrdiff_type_node, end_index);
 	  stride = build_int_cst (integer_type_node, 1);
 	  stride = fold_build1 (CONVERT_EXPR, ptrdiff_type_node, stride);
@@ -11120,7 +11114,7 @@ c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
 	  if (TREE_CODE (array_type) == FUNCTION_TYPE)
 	    {
 	      error_at (loc, "array notations cannot be used with function "
-			"type.");
+			"type");
 	      c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE, NULL);
 	      return error_mark_node;
 	    }
@@ -11138,7 +11132,7 @@ c_parser_array_notation (location_t loc, c_parser *parser, tree initial_index,
 		  if (subtype && TREE_CODE (subtype) == FUNCTION_TYPE)
 		    {
 		      error_at (loc, "array notations cannot be used with "
-				"function pointer arrays.");
+				"function pointer arrays");
 		      c_parser_skip_until_found (parser, CPP_CLOSE_SQUARE,
 						 NULL);
 		      return error_mark_node;
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 15dc83d..59658b7 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -103,8 +103,6 @@ static void readonly_warning (tree, enum lvalue_use);
 static int lvalue_or_else (location_t, const_tree, enum lvalue_use);
 static void record_maybe_used_decl (tree);
 static int comptypes_internal (const_tree, const_tree, bool *, bool *);
-extern bool contains_array_notation_expr (tree);
-extern tree find_correct_array_notation_type (tree);
 
 /* Return true if EXP is a null pointer constant, false otherwise.  */
 
@@ -2305,14 +2303,15 @@ build_array_ref (location_t loc, tree array, tree index)
   if (TREE_TYPE (array) == error_mark_node
       || TREE_TYPE (index) == error_mark_node)
     return error_mark_node;
-  
+
   if (flag_enable_cilkplus && contains_array_notation_expr (index))
     {
       size_t rank = 0;
-      find_rank (index, true, &rank);
+      if (!find_rank (loc, index, index, true, &rank))
+	return error_mark_node;
       if (rank > 1)
 	{
-	  error_at (loc, "rank of the array's index is greater than 1.");
+	  error_at (loc, "rank of the array's index is greater than 1");
 	  return error_mark_node;
 	}
     }
@@ -5130,6 +5129,7 @@ convert_for_assignment (location_t location, tree type, tree rhs,
   enum tree_code coder;
   tree rname = NULL_TREE;
   bool objc_ok = false;
+
   if (errtype == ic_argpass)
     {
       tree selector;
@@ -9035,13 +9035,13 @@ c_finish_loop (location_t start_locus, tree cond, tree incr, tree body,
   if (flag_enable_cilkplus && contains_array_notation_expr (cond))
     {
       error_at (start_locus, "array notation expression cannot be used in a "
-		"loop's condition");
+		"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.");
+		"loop's increment expression");
       return;
     }
   
diff --git gcc/doc/extend.texi gcc/doc/extend.texi
index 627bf69..b22eb79 100644
--- gcc/doc/extend.texi
+++ gcc/doc/extend.texi
@@ -82,6 +82,7 @@ extensions, accepted by GCC in C90 mode and in C++.
 * x86 specific memory model extensions for transactional memory:: x86 memory models.
 * Object Size Checking:: Built-in functions for limited buffer overflow
                         checking.
+* Cilk Plus Builtins::  Built-in functions for the Cilk Plus language extension.
 * Other Builtins::      Other built-in functions.
 * Target Builtins::     Built-in functions specific to particular targets.
 * Target Format Checks:: Format checks specific to particular targets.
@@ -8762,6 +8763,31 @@ Similar to @code{__builtin_bswap32}, except the argument and return types
 are 64 bit.
 @end deftypefn
 
+@node Cilk Plus Builtins
+@section Cilk Plus C/C++ language extension Built-in Functions.
+
+GCC provides support for the following built-in reduction funtions if Cilk Plus
+is enabled. Cilk Plus can be enabled using the @option{-fcilkplus} flag.
+
+@itemize @bullet
+@item __sec_reduce
+@item __sec_reduce_add
+@item __sec_reduce_all_nonzero
+@item __sec_reduce_all_zero
+@item __sec_reduce_any_nonzero
+@item __sec_reduce_any_zero
+@item __sec_reduce_max
+@item __sec_reduce_min
+@item __sec_reduce_max_ind
+@item __sec_reduce_min_ind
+@item __sec_reduce_mul
+@item __sec_reduce_mutating
+@end itemize
+
+Further details and examples about these built-in functions are described 
+in the Cilk Plus language manual which can be found at 
+@uref{http://www.cilkplus.org}.
+
 @node Target Builtins
 @section Built-in Functions Specific to Particular Target Machines
 
diff --git gcc/doc/passes.texi gcc/doc/passes.texi
index 81b6502..045f964 100644
--- gcc/doc/passes.texi
+++ gcc/doc/passes.texi
@@ -125,25 +125,7 @@ inside conditions, they are transformed using the function
 @code{fix_conditional_array_notations}.  The C language-specific routines are 
 located in @file{c/c-array-notation.c} and the equivalent C++ routines are in 
 file @file{cp/cp-array-notation.c}.  Common routines such as functions to 
-initialize builtin functions are stored in @file{array-notation-common.c}.  In
-the current array notation implementation there are 12 builtin reduction
-operations.  Details about these functions and their usage are available in
-the Cilk Plus language specification at @w{@uref{http://www.cilkplus.org}}.
-
-@itemize @bullet
-@item __sec_reduce_add
-@item __sec_reduce_mul
-@item __sec_reduce_max
-@item __sec_reduce_min
-@item __sec_reduce_max_ind
-@item __sec_reduce_min_ind
-@item __sec_reduce_all_zero
-@item __sec_reduce_all_nonzero
-@item __sec_reduce_any_zero
-@item __sec_reduce_any_nonzero
-@item __sec_reduce
-@item __sec_reduce_mutating
-@end itemize
+initialize builtin functions are stored in @file{array-notation-common.c}.
 @end itemize
 
 Detailed information about Cilk Plus and language specification is provided in 
diff --git gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c
index 44e7361..4035ed5 100644
--- gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c
+++ gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/decl-ptr-colon.c
@@ -1,16 +1,16 @@
 int main(void)
 {
   extern int func(int);
-  int array3[:], x, q; /* { dg-error "array notations cannot be used in declaration." } */
-  int  array3[1:2:x]; /* { dg-error "array notations cannot be used in declaration." } */
-  extern char array3[1:func(x)]; /* { dg-error "array notations cannot be used in declaration." } */
+  int array3[:], x, q; /* { dg-error "array notations cannot be used in declaration" } */
+  int  array3[1:2:x]; /* { dg-error "array notations cannot be used in declaration" } */
+  extern char array3[1:func(x)]; /* { dg-error "array notations cannot be used in declaration" } */
   int *a, ***b;
   extern char *c;
   int array2[10];
 
-  a[:] = 5; /* { dg-error  "start-index and length fields necessary for using array notations in pointers." } */
+  a[:] = 5; /* { dg-error  "start-index and length fields necessary for using array notations in pointers" } */
   c[1:2] =  3; /* This is OK.  */
   (array2)[:] = 5; /* This is OK.  */
-  b[1:2][1:func(x)][:] = 3; /*  { dg-error  "start-index and length fields necessary for using array notations in pointers." }  */
+  b[1:2][1:func(x)][:] = 3; /*  { dg-error  "start-index and length fields necessary for using array notations in pointers" }  */
 }
 
diff --git gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
index 272ef41..82008c0 100644
--- gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
+++ gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fn_ptr.c
@@ -3,16 +3,15 @@ typedef int (*foo)(int);
 int main(int argc, char **argv)
 {
   int array[10], array2[10][10];
-  // int array[10], array2[10], value, ii = 0;
   foo func_array[10];
   foo func_array2[10][10];
   foo ***func_array_ptr;
 
-  array[:] =  func_array[:](10); /* { dg-error "array notations cannot be used with function pointer arrays." } */
-  func_array[0:5](10); /* { dg-error "array notations cannot be used with function pointer arrays." } */
-  func_array2[0:5][:](10); /* { dg-error "array notations cannot be used with function pointer arrays." } */
-  array2[0:5][:] = func_array2[0:5][:](10); /* { dg-error "array notations cannot be used with function pointer arrays." } */
-  func_array_ptr[0:5][0:4][0:argc:2](argc); /* { dg-error "array notations cannot be used with function pointer arrays." } */
+  array[:] =  func_array[:](10); /* { dg-error "array notations cannot be used with function pointer arrays" } */
+  func_array[0:5](10); /* { dg-error "array notations cannot be used with function pointer arrays" } */
+  func_array2[0:5][:](10); /* { dg-error "array notations cannot be used with function pointer arrays" } */
+  array2[0:5][:] = func_array2[0:5][:](10); /* { dg-error "array notations cannot be used with function pointer arrays" } */
+  func_array_ptr[0:5][0:4][0:argc:2](argc); /* { dg-error "array notations cannot be used with function pointer arrays" } */
 
   return 0;
 }
diff --git gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fp_triplet_values.c gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fp_triplet_values.c
index ef39b2b..59c2d1e 100644
--- gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fp_triplet_values.c
+++ gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/fp_triplet_values.c
@@ -25,18 +25,18 @@ void func (int *x)
 int main2 (int argc, char **argv)
 {
   int array[10], array2[10];
-  array2[:] = array[1.5:2]; /* { dg-error "start-index of array notation triplet is not an integer." } */
-  array2[:] = array[1:2.32333333333]; /* { dg-error "length of array notation triplet is not an integer." } */
-  array2[1:2:1.5] = array[:]; /* { dg-error "stride of array notation triplet is not an integer." } */
-  func (&array2[1:2.34:3]); /* { dg-error "length of array notation triplet is not an integer." } */
-  array2[1.43:9]++; /* { dg-error "start-index of array notation triplet is not an integer." } */
-  array2[1:9.3]++; /* { dg-error "length of array notation triplet is not an integer." } */
-  array2[1:9:0.3]++; /* { dg-error "stride of array notation triplet is not an integer." } */
+  array2[:] = array[1.5:2]; /* { dg-error "start-index of array notation triplet is not an integer" } */
+  array2[:] = array[1:2.32333333333]; /* { dg-error "length of array notation triplet is not an integer" } */
+  array2[1:2:1.5] = array[:]; /* { dg-error "stride of array notation triplet is not an integer" } */
+  func (&array2[1:2.34:3]); /* { dg-error "length of array notation triplet is not an integer" } */
+  array2[1.43:9]++; /* { dg-error "start-index of array notation triplet is not an integer" } */
+  array2[1:9.3]++; /* { dg-error "length of array notation triplet is not an integer" } */
+  array2[1:9:0.3]++; /* { dg-error "stride of array notation triplet is not an integer" } */
   
-  ++array2[1:q:3]; /* { dg-error "length of array notation triplet is not an integer." } */
-  array2[:] = array[q:1:3]; /* { dg-error "start-index of array notation triplet is not an integer." } */
-  array2[:] = array[1:q:3]; /* { dg-error "length of array notation triplet is not an integer." } */
-  array2[:] = array[1:3:q]; /* { dg-error "stride of array notation triplet is not an integer." } */
-  func (&array2[1:q:3]); /* { dg-error "length of array notation triplet is not an integer." } */
+  ++array2[1:q:3]; /* { dg-error "length of array notation triplet is not an integer" } */
+  array2[:] = array[q:1:3]; /* { dg-error "start-index of array notation triplet is not an integer" } */
+  array2[:] = array[1:q:3]; /* { dg-error "length of array notation triplet is not an integer" } */
+  array2[:] = array[1:3:q]; /* { dg-error "stride of array notation triplet is not an integer" } */
+  func (&array2[1:q:3]); /* { dg-error "length of array notation triplet is not an integer" } */
   return 0;
 } 
diff --git gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/misc.c gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/misc.c
index 5a987d7..90a22eb 100644
--- gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/misc.c
+++ gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/misc.c
@@ -47,17 +47,17 @@ int main (int argc, char **argv)
     x = 9;
   }
 
-  for (ii = 0; ii < array[:]; ii++) /* { dg-error "array notations cannot be used in a condition for a for-loop." } */
+  for (ii = 0; ii < array[:]; ii++) /* { dg-error "array notations cannot be used in a condition for a for-loop" } */
     {
       x = 2;
     }
 
-  for (ii = 0; ii < array2[:][:]; ii++) /* { dg-error "array notations cannot be used in a condition for a for-loop." } */
+  for (ii = 0; ii < array2[:][:]; ii++) /* { dg-error "array notations cannot be used in a condition for a for-loop" } */
     {
       x = 3;
     }
 
-  for (; array2[:][:] < 2;) /* { dg-error "array notations cannot be used in a condition for a for-loop." } */
+  for (; array2[:][:] < 2;) /* { dg-error "array notations cannot be used in a condition for a for-loop" } */
     x = 4;
 
 
diff --git gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/rank_mismatch2.c gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/rank_mismatch2.c
new file mode 100644
index 0000000..77955a3
--- /dev/null
+++ gcc/testsuite/gcc.dg/cilk-plus/array_notation/errors/rank_mismatch2.c
@@ -0,0 +1,23 @@
+int function_call (int x)
+{
+  return x;
+}
+
+int main(int argc, char **argv)
+{
+  int array[100], array2[100][100];
+
+  array[:] = array[:] + array2[:][:]; /* { dg-error "rank mismatch in expression" } */
+
+  if (array[:] + array2[:][:]) /* { dg-error "rank mismatch in expression" } */
+    return argc == 5;
+
+  argc += function_call (array[:] + array2[5:10:2][:]); /* { dg-error "rank mismatch in expression" } */
+
+  argc += function_call (function_call (array[:] + array2[5:10:2][:])); /* { dg-error "rank mismatch in expression" } */
+
+   argc += __sec_reduce_add (array[:], array2[:][:]); /* { dg-error "rank mismatch in expression" } */
+
+   argc += __sec_reduce_add (array2[:][:]) + argc; /* This is OK.  */
+  return argc;
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]