Removal of a few parameters in two SRA functions

Hi,

I almost added a location parameter to load_assign_lhs_subreplacements
before I realized that the location and also two other parameters
could be derived from others.  For some time I have also intended to
make the lacc parameter of that function the root of the tree that is
to be processed rather than the first child which is perhaps a bit
confusing to readers.  The patch below makes these changes, in
load_assign_lhs_subreplacements and one of its helper functions.

I have wanted to change the first parameter generate_subtree_copies in
the same way (root instead of first child) but the code that
initializes replacements of parameters at the beginning of the
function or every other use of the function would get unnecessarily
more complicated so I only updated the comment to stress what the
parameter really is.

Bootstrapped and tested on x86_64-linux, I hope it will make the code
a little bit more readable.  OK for trunk?

Thanks,

Martin


2010-09-14  Martin Jambor  <mjambor@suse.cz>

	* tree-sra.c (generate_subtree_copies): Updated comment.
	(handle_unscalarized_data_in_subtree): Removed parameter lhs which is
	obtained from the statement iterator instead.
	(load_assign_lhs_subreplacements): Removed parameters lhs and
	right_offset, which is obtained from top_racc instead.  Parameter lacc
	is now expected to be the root of the processed tree rather than root's
	first child.  Updated all callers.

On Tue, Sep 14, 2010 at 8:40 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> I almost added a location parameter to load_assign_lhs_subreplacements
> before I realized that the location and also two other parameters
> could be derived from others.  For some time I have also intended to
> make the lacc parameter of that function the root of the tree that is
> to be processed rather than the first child which is perhaps a bit
> confusing to readers.  The patch below makes these changes, in
> load_assign_lhs_subreplacements and one of its helper functions.
>
> I have wanted to change the first parameter generate_subtree_copies in
> the same way (root instead of first child) but the code that
> initializes replacements of parameters at the beginning of the
> function or every other use of the function would get unnecessarily
> more complicated so I only updated the comment to stress what the
> parameter really is.
>
> Bootstrapped and tested on x86_64-linux, I hope it will make the code
> a little bit more readable.  OK for trunk?

Ok.

Thanks,
Richard.

> Thanks,
>
> Martin
>
>
> 2010-09-14  Martin Jambor  <mjambor@suse.cz>
>
>        * tree-sra.c (generate_subtree_copies): Updated comment.
>        (handle_unscalarized_data_in_subtree): Removed parameter lhs which is
>        obtained from the statement iterator instead.
>        (load_assign_lhs_subreplacements): Removed parameters lhs and
>        right_offset, which is obtained from top_racc instead.  Parameter lacc
>        is now expected to be the root of the processed tree rather than root's
>        first child.  Updated all callers.
>
> Index: mine/gcc/tree-sra.c
> ===================================================================
> --- mine.orig/gcc/tree-sra.c
> +++ mine/gcc/tree-sra.c
> @@ -2169,12 +2169,12 @@ analyze_all_variable_accesses (void)
>  }
>
>  /* Generate statements copying scalar replacements of accesses within a subtree
> -   into or out of AGG.  ACCESS is the first child of the root of the subtree to
> -   be processed.  AGG is an aggregate type expression (can be a declaration but
> -   does not have to be, it can for example also be a mem_ref or a series of
> -   handled components).  TOP_OFFSET is the offset of the processed subtree
> -   which has to be subtracted from offsets of individual accesses to get
> -   corresponding offsets for AGG.  If CHUNK_SIZE is non-null, copy only
> +   into or out of AGG.  ACCESS, all its children, siblings and their children
> +   are to be processed.  AGG is an aggregate type expression (can be a
> +   declaration but does not have to be, it can for example also be a mem_ref or
> +   a series of handled components).  TOP_OFFSET is the offset of the processed
> +   subtree which has to be subtracted from offsets of individual accesses to
> +   get corresponding offsets for AGG.  If CHUNK_SIZE is non-null, copy only
>    replacements in the interval <start_offset, start_offset + chunk_size>,
>    otherwise copy all.  GSI is a statement iterator used to place the new
>    statements.  WRITE should be true when the statements should write from AGG
> @@ -2405,11 +2405,11 @@ enum unscalarized_data_handling { SRA_UD
>                                  SRA_UDH_LEFT }; /* Data flushed to the LHS. */
>
>  /* Store all replacements in the access tree rooted in TOP_RACC either to their
> -   base aggregate if there are unscalarized data or directly to LHS
> -   otherwise.  */
> +   base aggregate if there are unscalarized data or directly to LHS of the
> +   statement that is pointed to by GSI otherwise.  */
>
>  static enum unscalarized_data_handling
> -handle_unscalarized_data_in_subtree (struct access *top_racc, tree lhs,
> +handle_unscalarized_data_in_subtree (struct access *top_racc,
>                                     gimple_stmt_iterator *gsi)
>  {
>   if (top_racc->grp_unscalarized_data)
> @@ -2421,6 +2421,7 @@ handle_unscalarized_data_in_subtree (str
>     }
>   else
>     {
> +      tree lhs = gimple_assign_lhs (gsi_stmt (*gsi));
>       generate_subtree_copies (top_racc->first_child, lhs, top_racc->offset,
>                               0, 0, gsi, false, false,
>                               gimple_location (gsi_stmt (*gsi)));
> @@ -2429,33 +2430,30 @@ handle_unscalarized_data_in_subtree (str
>  }
>
>
> -/* Try to generate statements to load all sub-replacements in an access
> -   (sub)tree (LACC is the first child) from scalar replacements in the TOP_RACC
> -   (sub)tree.  If that is not possible, refresh the TOP_RACC base aggregate and
> -   load the accesses from it.  LEFT_OFFSET is the offset of the left whole
> -   subtree being copied, RIGHT_OFFSET is the same thing for the right subtree.
> -   NEW_GSI is stmt iterator used for statement insertions after the original
> -   assignment, OLD_GSI is used to insert statements before the assignment.
> -   *REFRESHED keeps the information whether we have needed to refresh
> -   replacements of the LHS and from which side of the assignments this takes
> -   place.  */
> +/* Try to generate statements to load all sub-replacements in an access subtree
> +   formed by children of LACC from scalar replacements in the TOP_RACC subtree.
> +   If that is not possible, refresh the TOP_RACC base aggregate and load the
> +   accesses from it.  LEFT_OFFSET is the offset of the left whole subtree being
> +   copied. NEW_GSI is stmt iterator used for statement insertions after the
> +   original assignment, OLD_GSI is used to insert statements before the
> +   assignment.  *REFRESHED keeps the information whether we have needed to
> +   refresh replacements of the LHS and from which side of the assignments this
> +   takes place.  */
>
>  static void
>  load_assign_lhs_subreplacements (struct access *lacc, struct access *top_racc,
>                                 HOST_WIDE_INT left_offset,
> -                                HOST_WIDE_INT right_offset,
>                                 gimple_stmt_iterator *old_gsi,
>                                 gimple_stmt_iterator *new_gsi,
> -                                enum unscalarized_data_handling *refreshed,
> -                                tree lhs)
> +                                enum unscalarized_data_handling *refreshed)
>  {
>   location_t loc = gimple_location (gsi_stmt (*old_gsi));
> -  do
> +  for (lacc = lacc->first_child; lacc; lacc = lacc->next_sibling)
>     {
>       if (lacc->grp_to_be_replaced)
>        {
>          struct access *racc;
> -         HOST_WIDE_INT offset = lacc->offset - left_offset + right_offset;
> +         HOST_WIDE_INT offset = lacc->offset - left_offset + top_racc->offset;
>          gimple stmt;
>          tree rhs;
>
> @@ -2471,7 +2469,7 @@ load_assign_lhs_subreplacements (struct
>              /* No suitable access on the right hand side, need to load from
>                 the aggregate.  See if we have to update it first... */
>              if (*refreshed == SRA_UDH_NONE)
> -               *refreshed = handle_unscalarized_data_in_subtree (top_racc, lhs,
> +               *refreshed = handle_unscalarized_data_in_subtree (top_racc,
>                                                                  old_gsi);
>
>              if (*refreshed == SRA_UDH_LEFT)
> @@ -2490,16 +2488,13 @@ load_assign_lhs_subreplacements (struct
>        }
>       else if (*refreshed == SRA_UDH_NONE
>               && lacc->grp_read && !lacc->grp_covered)
> -       *refreshed = handle_unscalarized_data_in_subtree (top_racc, lhs,
> +       *refreshed = handle_unscalarized_data_in_subtree (top_racc,
>                                                          old_gsi);
>
>       if (lacc->first_child)
> -       load_assign_lhs_subreplacements (lacc->first_child, top_racc,
> -                                        left_offset, right_offset,
> -                                        old_gsi, new_gsi, refreshed, lhs);
> -      lacc = lacc->next_sibling;
> +       load_assign_lhs_subreplacements (lacc, top_racc, left_offset,
> +                                        old_gsi, new_gsi, refreshed);
>     }
> -  while (lacc);
>  }
>
>  /* Result code for SRA assignment modification.  */
> @@ -2714,14 +2709,12 @@ sra_modify_assign (gimple *stmt, gimple_
>          enum unscalarized_data_handling refreshed;
>
>          if (lacc->grp_read && !lacc->grp_covered)
> -           refreshed = handle_unscalarized_data_in_subtree (racc, lhs, gsi);
> +           refreshed = handle_unscalarized_data_in_subtree (racc, gsi);
>          else
>            refreshed = SRA_UDH_NONE;
>
> -         load_assign_lhs_subreplacements (lacc->first_child, racc,
> -                                          lacc->offset, racc->offset,
> -                                          &orig_gsi, gsi, &refreshed,
> -                                          lhs);
> +         load_assign_lhs_subreplacements (lacc, racc, lacc->offset,
> +                                          &orig_gsi, gsi, &refreshed);
>          if (refreshed != SRA_UDH_RIGHT)
>            {
>              gsi_next (gsi);
>