Re: [PATCH, PR68756] Fix typo in copy_cond_phi_args

[Richard Biener <rguenther at suse dot de>]

On Tue, 12 Apr 2016, Tom de Vries wrote:

> [adding cc gcc-patches ]
> 
> On 12/04/16 09:25, Tom de Vries wrote:
> > Hi,
> > 
> > when compiling the test-case in the patch, we run into an ICE in
> > sese_build_liveouts_use when evaluating 'TREE_CODE (use)' because use is
> > NULL_TREE:
> > ...
> > static void
> > sese_build_liveouts_use (sese_info_p region, bitmap liveouts,
> >                           basic_block bb, tree use)
> > {
> >    gcc_assert (!bb_in_sese_p (bb, region->region));
> >    if (TREE_CODE (use) != SSA_NAME)
> >      return;
> > ...
> > 
> > I've tracked down the origin of the NULL_TREE use to copy_cond_phi_args,
> > to this snippet:
> > ...
> >        if (postpone)
> >          {
> >            /* If the phi-arg is scev-analyzeable but only in the first
> >               stage.  */
> >            if (is_gimple_reg (old_name)
> >                && scev_analyzable_p (old_name, region->region))
> >              {
> >                gimple_seq stmts;
> >                tree new_expr = get_rename_from_scev (old_name, &stmts,
> >                                                      loop, new_bb,
> >                                                      old_bb, iv_map);
> >                if (codegen_error_p ())
> >          return false;
> > 
> >                gcc_assert (new_expr);
> >                if (dump_file)
> >                  {
> >                    fprintf (dump_file,
> >                             "[codegen] scev analyzeable, new_expr: ");
> >                    print_generic_expr (dump_file, new_expr, 0);
> >                    fprintf (dump_file, "\n");
> >                  }
> >                gsi_insert_earliest (stmts);
> >                new_phi_args[i] = new_name;
> >                continue;
> >              }
> > 
> >            /* Postpone code gen for later for back-edges.  */
> >            region->incomplete_phis.safe_push (std::make_pair (phi,
> >                                                               new_phi));
> > ...
> > 
> > My observations are:
> > - new_name is guaranteed to be NULL_TREE at this point.
> > - the 'new_phi_args[i] = new_name' assignment should set to something
> >    non-NULL, given that we're not postponing code generation for later
> > - the whole point of the snippet seems to be to set up an expression
> >    new_expr, so probably that's the expression we want to assign to
> >    new_phi_args[i].
> > 
> > Bootstrapped and reg-tested on x86_64.
> > 
> > Ok for stage4 trunk?

Yup, looks obvious if it even works ;)

Thanks,
Richard.

> > Thanks,
> > - Tom
> > 
> > 0001-Fix-typo-in-copy_cond_phi_args.patch
> > 
> > 
> > Fix typo in copy_cond_phi_args
> > 
> > 2016-04-12  Tom de Vries  <tom@codesourcery.com>
> > 
> > 	PR tree-optimization/68756
> > 	* graphite-isl-ast-to-gimple.c (copy_cond_phi_args): Use new_expr
> > 	instead of new_name.
> > 
> > 	* gcc.dg/graphite/pr68756.c: New test.
> > 
> > ---
> >   gcc/graphite-isl-ast-to-gimple.c        |  2 +-
> >   gcc/testsuite/gcc.dg/graphite/pr68756.c | 26 ++++++++++++++++++++++++++
> >   2 files changed, 27 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gcc/graphite-isl-ast-to-gimple.c
> > b/gcc/graphite-isl-ast-to-gimple.c
> > index 8dd5dc8..88609c0 100644
> > --- a/gcc/graphite-isl-ast-to-gimple.c
> > +++ b/gcc/graphite-isl-ast-to-gimple.c
> > @@ -2439,7 +2439,7 @@ copy_cond_phi_args (gphi *phi, gphi *new_phi,
> > vec<tree> iv_map, bool postpone)
> >   		  fprintf (dump_file, "\n");
> >   		}
> >   	      gsi_insert_earliest (stmts);
> > -	      new_phi_args [i] = new_name;
> > +	      new_phi_args[i] = new_expr;
> >   	      continue;
> >   	    }
> > 
> > diff --git a/gcc/testsuite/gcc.dg/graphite/pr68756.c
> > b/gcc/testsuite/gcc.dg/graphite/pr68756.c
> > new file mode 100644
> > index 0000000..ddb9789
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/graphite/pr68756.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O1 -floop-nest-optimize" } */
> > +
> > +unsigned int z4, pz;
> > +int nn[2];
> > +
> > +static unsigned int
> > +xq (unsigned int dj)
> > +{
> > +  return dj > 1 ? z4 : z4 + dj;
> > +}
> > +
> > +void
> > +la (void)
> > +{
> > +  int hd, dl;
> > +  unsigned int hn = 0;
> > +
> > +  for (hd = 0; hd < 2; ++hd)
> > +    {
> > +      for (dl = 0; dl < 2; ++dl)
> > +	nn[dl] = 0;
> > +      --hn;
> > +      pz = xq (hn);
> > +    }
> > +}
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)