Re: [PATCH 3/4] Do not check whether memory references accessed in every iteration trap.

[Richard Guenther <rguenther at suse dot de>]

On Wed, 18 Aug 2010, Sebastian Pop wrote:

> On Wed, Aug 18, 2010 at 04:32, Richard Guenther <rguenther@suse.de> wrote:
> >> >> + ? ? ?}
> >> >> +
> >> >> + ?return false;
> >> >> +}
> >> >> +
> >> >> +/* Returns true when the memory references of STMT are read or written
> >> >> + ? unconditionally. ?*/
> >> >> +
> >> >> +static bool
> >> >> +memrefs_read_or_written_unconditionally (gimple stmt,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?VEC (data_reference_p, heap) *drs)
> >> >> +{
> >> >> + ?int i;
> >> >> + ?data_reference_p a;
> >> >> + ?tree ca = bb_predicate (gimple_bb (stmt));
> >> >> +
> >> >> + ?for (i = 0; VEC_iterate (data_reference_p, drs, i, a); i++)
> >> >> + ? ?if (DR_STMT (a) == stmt
> >> >
> >> > Ick. ?Why not check DR_STMT (b) != stmt in
> >> > memref_read_or_written_unconditionally and avoid all this linear
> >> > searching mess?
> >> >
> >>
> >> Could you please be more specific: I do not understand what you
> >> are asking here.
> >
> >> >> + ?for (i = 0; VEC_iterate (data_reference_p, drs, i, a); i++)
> >> >> + ? ?if (DR_STMT (a) == stmt
> >
> > Here you find the data-ref with DR_STMT == stmt, just to call
> > memref_read_or_written_unconditionally with it's index in the
> > DR vector. ?Why not simply do
> >
> > ?memref_read_or_written_unconditionally (stmt, drs, ca)
> >
> > and in that function instead of the i != pos check
> >
> >> + ?for (i = 0; VEC_iterate (data_reference_p, drs, i, b); i++)
> >> + ? ?if (i != pos && same_data_refs (a, b))
> >
> > do DR_STMT (b) != stmt? ?I guess I am asking to fold the two
> > functions into one to make the quadraticness obvious and avoid
> > passing down indices. ?I'm not sure if the result is more
> > readable but certainly the current version looked odd to me.
> >
> 
> I modified both memrefs_read_or_written_unconditionally and
> write_memrefs_written_at_least_once.  What do you think about this form?
> 
> /* Returns true when the memory references of STMT are read or written
>    unconditionally.  In other words, this function returns true when
>    for every data reference A in STMT there exist other accesses to
>    the same data reference with predicates that add up (OR-up) to the
>    true predicate: this ensures that the data reference A is touched
>    (read or written) on every iteration of the if-converted loop.  */
> 
> static bool
> memrefs_read_or_written_unconditionally (gimple stmt,
> 					 VEC (data_reference_p, heap) *drs)
> {
>   int i, j;
>   data_reference_p a, b;
>   tree ca = bb_predicate (gimple_bb (stmt));
> 
>   for (i = 0; VEC_iterate (data_reference_p, drs, i, a); i++)
>     if (DR_STMT (a) == stmt)
>       {
> 	bool found = false;
> 	int x = DR_RW_UNCONDITIONALLY (a);
> 
> 	if (x == 0)
> 	  return false;
> 
> 	if (x == 1)
> 	  continue;
> 
> 	for (j = 0; VEC_iterate (data_reference_p, drs, j, b); j++)
> 	  if (DR_STMT (b) != stmt
> 	      && same_data_refs (a, b))
> 	    {
> 	      tree cb = bb_predicate (gimple_bb (DR_STMT (b)));
> 
> 	      if (DR_RW_UNCONDITIONALLY (b) == 1
> 		  || is_true_predicate (cb)
> 		  || is_true_predicate (ca = fold_or_predicates (EXPR_LOCATION (cb),
> 								 ca, cb)))
> 		{
> 		  DR_RW_UNCONDITIONALLY (a) = 1;
> 		  DR_RW_UNCONDITIONALLY (b) = 1;
> 		  found = true;
> 		  break;
> 		}
> 	    }
> 
> 	if (!found)
> 	  {
> 	    DR_RW_UNCONDITIONALLY (a) = 0;
> 	    return false;
> 	  }
>       }
> 
>   return true;
> }
> 
> /* Returns true when the memory references of STMT are unconditionally
>    written.  In other words, this function returns true when for every
>    data reference A written in STMT, there exist other writes to the
>    same data reference with predicates that add up (OR-up) to the true
>    predicate: this ensures that the data reference A is written on
>    every iteration of the if-converted loop.  */
> 
> static bool
> write_memrefs_written_at_least_once (gimple stmt,
> 				     VEC (data_reference_p, heap) *drs)
> {
>   int i, j;
>   data_reference_p a, b;
>   tree ca = bb_predicate (gimple_bb (stmt));
> 
>   for (i = 0; VEC_iterate (data_reference_p, drs, i, a); i++)
>     if (DR_STMT (a) == stmt
> 	&& !DR_IS_READ (a))
>       {
> 	bool found = false;
> 	int x = DR_WRITTEN_AT_LEAST_ONCE (a);
> 
> 	if (x == 0)
> 	  return false;
> 
> 	if (x == 1)
> 	  continue;
> 
> 	for (j = 0; VEC_iterate (data_reference_p, drs, j, b); j++)
> 	  if (DR_STMT (b) != stmt
> 	      && !DR_IS_READ (b)
> 	      && same_data_refs_base_objects (a, b))
> 	    {
> 	      tree cb = bb_predicate (gimple_bb (DR_STMT (b)));
> 
> 	      if (DR_WRITTEN_AT_LEAST_ONCE (b) == 1
> 		  || is_true_predicate (cb)
> 		  || is_true_predicate (ca = fold_or_predicates (EXPR_LOCATION (cb),
> 								 ca, cb)))
> 		{
> 		  DR_WRITTEN_AT_LEAST_ONCE (a) = 1;
> 		  DR_WRITTEN_AT_LEAST_ONCE (b) = 1;
> 		  found = true;
> 		  break;
> 		}
> 	    }
> 
> 	if (!found)
> 	  {
> 	    DR_WRITTEN_AT_LEAST_ONCE (a) = 0;
> 	    return false;
> 	  }
>       }
> 
>   return true;
> }
> 
> I will prepare the combined patch if you think that this is a better
> form.

Yes, I like that better.

Thanks,
Richard.