[graphite] Remove sese_adjust_liveout_phis

Hi,

On Sat, Jun 12, 2010 at 02:53, Sebastian Pop <sebpop@gmail.com> wrote:
> Hi,
>
> with the attached patches we no longer need to use the rename_map to
> substitute the arguments of the phi nodes created after the code
> generated by Graphite.  I committed these patches to the graphite
> branch, and I will commit them to trunk once they pass the usual
> graphite branch tests.
>
> The aim is to replace the rename_map with the map that CLooG exposes
> for each user statement: associating to a loop level an expression.
> The expand_* functions will be replaced by the code generation from a
> chrec that is obtained from the original scev by chrec_applying the
> loop level -> expression map on each varying loop.  This will then
> allow us to remove the call to the IV canonicalization from Graphite.
>

One of these patches broke 464.h264ref: here is a reduced testcase

/* { dg-options "-O3 -fgraphite-identity -ffast-math" } */

typedef enum
{
  I_SLICE,
} SliceType;
typedef struct
{
  int type;
} ImageParameters;
extern ImageParameters *img;
int A[64], B[64], C[13][8][8], D[13][8][8];

void
foo (int q, int temp)
{
  int i, j, k;
  for(k=0; k<13; k++)
    for(j=0; j<8; j++)
      for(i=0; i<8; i++)
	{
	  if (img->type == I_SLICE)
	    C[k][j][i] = A[temp] << q;
	  D[k][j][i] = B[temp] << q;
	}
}

The problem is that img->type is moved out of the loop, making it a
parameter of the loop, and then this parameter is copied inside the
first loop:

p = img->type;
loop_1
  q = p
  loop_2
    loop_3
       if (q == 0) etc.

Now because we rewrite the scalar dependence into a data dependence,

p = img->type;
loop_1
  q = p
  cross_bb_dependence[0] = q
  loop_2
    loop_3
       if (cross_bb_dependence[0] == 0) etc.

scev is not smart enough to infer that cross_bb_dependence[0] is a
parameter defined outside the scop.

A possible fix for this is to run copy propagation before Graphite:



I don't know how to make the execution of copy_prop conditional to the
execution of Graphite.  Any suggestions?

Thanks,
Sebastian

On Mon, 14 Jun 2010, Sebastian Pop wrote:

> Hi,
> 
> On Sat, Jun 12, 2010 at 02:53, Sebastian Pop <sebpop@gmail.com> wrote:
> > Hi,
> >
> > with the attached patches we no longer need to use the rename_map to
> > substitute the arguments of the phi nodes created after the code
> > generated by Graphite.  I committed these patches to the graphite
> > branch, and I will commit them to trunk once they pass the usual
> > graphite branch tests.
> >
> > The aim is to replace the rename_map with the map that CLooG exposes
> > for each user statement: associating to a loop level an expression.
> > The expand_* functions will be replaced by the code generation from a
> > chrec that is obtained from the original scev by chrec_applying the
> > loop level -> expression map on each varying loop.  This will then
> > allow us to remove the call to the IV canonicalization from Graphite.
> >
> 
> One of these patches broke 464.h264ref: here is a reduced testcase
> 
> /* { dg-options "-O3 -fgraphite-identity -ffast-math" } */
> 
> typedef enum
> {
>   I_SLICE,
> } SliceType;
> typedef struct
> {
>   int type;
> } ImageParameters;
> extern ImageParameters *img;
> int A[64], B[64], C[13][8][8], D[13][8][8];
> 
> void
> foo (int q, int temp)
> {
>   int i, j, k;
>   for(k=0; k<13; k++)
>     for(j=0; j<8; j++)
>       for(i=0; i<8; i++)
> 	{
> 	  if (img->type == I_SLICE)
> 	    C[k][j][i] = A[temp] << q;
> 	  D[k][j][i] = B[temp] << q;
> 	}
> }
> 
> The problem is that img->type is moved out of the loop, making it a
> parameter of the loop, and then this parameter is copied inside the
> first loop:
> 
> p = img->type;
> loop_1
>   q = p
>   loop_2
>     loop_3
>        if (q == 0) etc.
> 
> Now because we rewrite the scalar dependence into a data dependence,
> 
> p = img->type;
> loop_1
>   q = p
>   cross_bb_dependence[0] = q
>   loop_2
>     loop_3
>        if (cross_bb_dependence[0] == 0) etc.
> 
> scev is not smart enough to infer that cross_bb_dependence[0] is a
> parameter defined outside the scop.
> 
> A possible fix for this is to run copy propagation before Graphite:
> 
> diff --git a/gcc/passes.c b/gcc/passes.c
> index ad444fd..a646d19 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
> @@ -897,6 +897,7 @@ init_optimization_passes (void)
>  	  NEXT_PASS (pass_check_data_deps);
>  	  NEXT_PASS (pass_loop_distribution);
>  	  NEXT_PASS (pass_linear_transform);
> +	  NEXT_PASS (pass_copy_prop);
>  	  NEXT_PASS (pass_graphite_transforms);
>  	    {
>  	      struct opt_pass **p = &pass_graphite_transforms.pass.sub;
> 
> 
> I don't know how to make the execution of copy_prop conditional to the
> execution of Graphite.  Any suggestions?

Make pass_graphite_transforms a wrapper only (similar to 
pass_tree_loop), put the real graphite-transforms pass inside,
preceeded by copyprop.

Richard.