[PATCH] Limit VDEF walking in IPA-SRA when looking for potential modifications

[Martin Jambor]

Hi,

On Tue, Nov 03, 2009 at 11:03:01AM +0100, Richard Guenther wrote:
> On Mon, 2 Nov 2009, Martin Jambor wrote:
> 
> > Hi,
> > 
> > the patch below limits walking of VDEFs when searching for potential
> > modifications of what a parameter points to by providing a common
> > bitmap to various invocations of walk_aliased_vdefs which look for
> > alias vdefs of the same thing...
> 
> I wonder if it is correct.

It is not.  Unfortunately I suffer a weird form of blindness whenever
I look at this function and see what I want to see rather than what is
actually there.  The function, as it is now, is bogus, albeit in a
conservative way and so does not cause trouble.

I intended to loop over accesses of a group and analyze them for
potential modifications together, instead the loop is now over all
accesses of the same base.  In order to change this, I have built a
linked list of group members, overloading the next_sibling access
field which is not used in IPA-SRA.  I also moved ao_ref_init outside
the inner loop.

I have successfully bootstrapped and tested a very similiar patch, I
am bootstrapping and testing this exact version now.  OK if it passes?

Thanks,

Martin


2009-11-04  Martin Jambor  <mjambor@suse.cz>

	* tree-sra.c (struct access): Changed comment of next_sibling field.
	(analyze_modified_params): Loop over accesses of a group rather than
	over all with the ame base, pass a common bitmap to
	walk_aliased_vdefs.
	(unmodified_by_ref_scalar_representative): Build link lists of
	accesses of a group.
	(splice_param_accesses): Likewise.


Index: mine/gcc/tree-sra.c
===================================================================
*** mine.orig/gcc/tree-sra.c
--- mine/gcc/tree-sra.c
*************** struct access
*** 144,150 ****
       points to the first one.  */
    struct access *first_child;
  
!   /* Pointer to the next sibling in the access tree as described above.  */
    struct access *next_sibling;
  
    /* Pointers to the first and last element in the linked list of assign
--- 144,152 ----
       points to the first one.  */
    struct access *first_child;
  
!   /* In intraprocedural SRA, pointer to the next sibling in the access tree as
!      described above.  In IPA-SRA this is a pointer to the next access
!      belonging to the same group (having the same representative).  */
    struct access *next_sibling;
  
    /* Pointers to the first and last element in the linked list of assign
*************** analyze_modified_params (VEC (access_p,
*** 2824,2856 ****
  	   repr;
  	   repr = repr->next_grp)
  	{
! 	  VEC (access_p, heap) *access_vec;
! 	  int j, access_count;
! 	  tree parm;
  
  	  if (no_accesses_p (repr))
  	    continue;
! 	  parm = repr->base;
! 	  if (!POINTER_TYPE_P (TREE_TYPE (parm))
  	      || repr->grp_maybe_modified)
  	    continue;
  
! 	  access_vec = get_base_access_vector (parm);
! 	  access_count = VEC_length (access_p, access_vec);
! 	  for (j = 0; j < access_count; j++)
  	    {
- 	      struct access *access;
- 	      ao_ref ar;
- 
  	      /* All accesses are read ones, otherwise grp_maybe_modified would
  		 be trivially set.  */
- 	      access = VEC_index (access_p, access_vec, j);
- 	      ao_ref_init (&ar, access->expr);
  	      walk_aliased_vdefs (&ar, gimple_vuse (access->stmt),
! 				  mark_maybe_modified, repr, NULL);
  	      if (repr->grp_maybe_modified)
  		break;
  	    }
  	}
      }
  }
--- 2826,2853 ----
  	   repr;
  	   repr = repr->next_grp)
  	{
! 	  struct access *access;
! 	  bitmap visited;
! 	  ao_ref ar;
  
  	  if (no_accesses_p (repr))
  	    continue;
! 	  if (!POINTER_TYPE_P (TREE_TYPE (repr->base))
  	      || repr->grp_maybe_modified)
  	    continue;
  
! 	  ao_ref_init (&ar, repr->expr);
! 	  visited = BITMAP_ALLOC (NULL);
! 	  for (access = repr; access; access = access->next_sibling)
  	    {
  	      /* All accesses are read ones, otherwise grp_maybe_modified would
  		 be trivially set.  */
  	      walk_aliased_vdefs (&ar, gimple_vuse (access->stmt),
! 				  mark_maybe_modified, repr, &visited);
  	      if (repr->grp_maybe_modified)
  		break;
  	    }
+ 	  BITMAP_FREE (visited);
  	}
      }
  }
*************** static struct access *
*** 3019,3042 ****
  unmodified_by_ref_scalar_representative (tree parm)
  {
    int i, access_count;
!   struct access *access;
    VEC (access_p, heap) *access_vec;
  
    access_vec = get_base_access_vector (parm);
    gcc_assert (access_vec);
!   access_count = VEC_length (access_p, access_vec);
  
!   for (i = 0; i < access_count; i++)
      {
!       access = VEC_index (access_p, access_vec, i);
        if (access->write)
  	return NULL;
      }
  
!   access = VEC_index (access_p, access_vec, 0);
!   access->grp_read = 1;
!   access->grp_scalar_ptr = 1;
!   return access;
  }
  
  /* Sort collected accesses for parameter PARM, identify representatives for
--- 3016,3045 ----
  unmodified_by_ref_scalar_representative (tree parm)
  {
    int i, access_count;
!   struct access *repr;
    VEC (access_p, heap) *access_vec;
  
    access_vec = get_base_access_vector (parm);
    gcc_assert (access_vec);
!   repr = VEC_index (access_p, access_vec, 0);
!   if (repr->write)
!     return NULL;
!   repr->group_representative = repr;
  
!   access_count = VEC_length (access_p, access_vec);
!   for (i = 1; i < access_count; i++)
      {
!       struct access *access = VEC_index (access_p, access_vec, i);
        if (access->write)
  	return NULL;
+       access->group_representative = repr;
+       access->next_sibling = repr->next_sibling;
+       repr->next_sibling = access;
      }
  
!   repr->grp_read = 1;
!   repr->grp_scalar_ptr = 1;
!   return repr;
  }
  
  /* Sort collected accesses for parameter PARM, identify representatives for
*************** splice_param_accesses (tree parm, bool *
*** 3091,3096 ****
--- 3094,3102 ----
  	    return NULL;
  
  	  modification |= ac2->write;
+ 	  ac2->group_representative = access;
+ 	  ac2->next_sibling = access->next_sibling;
+ 	  access->next_sibling = ac2;
  	  j++;
  	}