This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Improve VRP assert creation for loops


On Thu, 7 Nov 2013, Jakub Jelinek wrote:

> On Thu, Nov 07, 2013 at 10:32:10AM +0100, Richard Biener wrote:
> > I'm looking for adjusting of live compute - either by adjusting the
> > algorithm or by special casing the latches.  Like for example
> > with the following (untested, needs cleanup - the PHI processing
> > can be factored out from find_assert_locations_1 and re-used):
> > 
> > @@ -5904,6 +5898,27 @@ find_assert_locations (void)
> >    for (i = 0; i < rpo_cnt; ++i)
> >      bb_rpo[rpo[i]] = i;
> >  
> > +  /* Pre-seed loop latch liveness from loop header PHI nodes.  Due to
> > +     the order we compute liveness and insert asserts we otherwise
> > +     fail to insert asserts into the loop latch.  */
> > +  loop_p loop;
> > +  loop_iterator li;
> > +  FOR_EACH_LOOP (li, loop, 0)
> > +    {
> > +      i = loop->latch->index;
> > +      live[i] = sbitmap_alloc (num_ssa_names);
> > +      bitmap_clear (live[i]);
> > +      for (gimple_stmt_iterator gsi = gsi_start_phis (loop->header);
> > +          !gsi_end_p (gsi); gsi_next (&gsi))
> > +       {
> > +         gimple phi = gsi_stmt (gsi);
> > +         if (virtual_operand_p (gimple_phi_result (phi)))
> > +           continue;
> > +         for (unsigned j = 0; j < gimple_phi_num_args (phi); ++j)
> > +           if (TREE_CODE (gimple_phi_arg_def (phi, j)) == SSA_NAME)
> > +             bitmap_set_bit (live[i], SSA_NAME_VERSION 
> > (gimple_phi_arg_def (phi, j)));
> > +       }
> > +    }
> 
> LGTM, except that I think we should only care about phi args from the
> latch -> header edge, not others and IMHO it is memory waste to allocate
> sbitmap even when we wouldn't add anything.  So, I'm going to
> rebootstrap/regtest the following (including new testcase Jeff asked for):
> 
> 2013-11-07  Richard Biener  <rguenther@suse.de>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	* tree-vrp.c (find_assert_locations): Pre-seed live bitmaps for loop
> 	latches from header PHI arguments from the latch edge.
> 
>         * gcc.dg/unroll_1.c: Add -fno-tree-vrp to dg-options.
>         * gcc.dg/unroll_2.c: Likewise.
>         * gcc.dg/unroll_3.c: Likewise.
>         * gcc.dg/unroll_4.c: Likewise.
> 	* gcc.dg/vrp90.c: New test.
> 
> --- gcc/tree-vrp.c.jj	2013-11-06 16:58:04.675678567 +0100
> +++ gcc/tree-vrp.c	2013-11-07 10:59:38.841283931 +0100
> @@ -5906,6 +5906,34 @@ find_assert_locations (void)
>    for (i = 0; i < rpo_cnt; ++i)
>      bb_rpo[rpo[i]] = i;
>  
> +  /* Pre-seed loop latch liveness from loop header PHI nodes.  Due to
> +     the order we compute liveness and insert asserts we otherwise
> +     fail to insert asserts into the loop latch.  */
> +  loop_p loop;
> +  loop_iterator li;
> +  FOR_EACH_LOOP (li, loop, 0)
> +    {
> +      i = loop->latch->index;
> +      unsigned int j = find_edge (loop->latch, loop->header)->dest_idx;

Actually there is only one edge from latch to header by definition,
so single_succ_edge (loop->latch) would be better?

Otherwise ok.

Thanks,
Richard.

> +      for (gimple_stmt_iterator gsi = gsi_start_phis (loop->header);
> +	   !gsi_end_p (gsi); gsi_next (&gsi))
> +	{
> +	  gimple phi = gsi_stmt (gsi);
> +	  if (virtual_operand_p (gimple_phi_result (phi)))
> +	    continue;
> +	  tree arg = gimple_phi_arg_def (phi, j);
> +	  if (TREE_CODE (arg) == SSA_NAME)
> +	    {
> +	      if (live[i] == NULL)
> +		{
> +		  live[i] = sbitmap_alloc (num_ssa_names);
> +		  bitmap_clear (live[i]);
> +		}
> +	      bitmap_set_bit (live[i], SSA_NAME_VERSION (arg));
> +	    }
> +	}
> +    }
> +
>    need_asserts = false;
>    for (i = rpo_cnt - 1; i >= 0; --i)
>      {
> --- gcc/testsuite/gcc.dg/unroll_1.c.jj	2013-09-09 11:32:36.000000000 +0200
> +++ gcc/testsuite/gcc.dg/unroll_1.c	2013-11-06 17:10:52.900722932 +0100
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-rtl-loop2_unroll=stderr -fno-peel-loops -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll" } */
> +/* { dg-options "-O2 -fdump-rtl-loop2_unroll=stderr -fno-peel-loops -fno-tree-vrp -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll" } */
>  
>  unsigned a[100], b[100];
>  inline void bar()
> --- gcc/testsuite/gcc.dg/unroll_2.c.jj	2013-08-30 14:38:39.000000000 +0200
> +++ gcc/testsuite/gcc.dg/unroll_2.c	2013-11-06 17:10:30.751845504 +0100
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fdisable-tree-cunroll=foo -fdisable-tree-cunrolli=foo -fenable-rtl-loop2_unroll" } */
> +/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fno-tree-vrp -fdisable-tree-cunroll=foo -fdisable-tree-cunrolli=foo -fenable-rtl-loop2_unroll" } */
>  
>  unsigned a[100], b[100];
>  inline void bar()
> --- gcc/testsuite/gcc.dg/unroll_3.c.jj	2013-08-30 14:38:39.000000000 +0200
> +++ gcc/testsuite/gcc.dg/unroll_3.c	2013-11-06 17:10:38.864800338 +0100
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll=foo" } */
> +/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fno-tree-vrp -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll=foo" } */
>  
>  unsigned a[100], b[100];
>  inline void bar()
> --- gcc/testsuite/gcc.dg/unroll_4.c.jj	2013-08-30 14:38:40.000000000 +0200
> +++ gcc/testsuite/gcc.dg/unroll_4.c	2013-11-06 17:11:03.816665603 +0100
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll=foo2" } */
> +/* { dg-options "-O2 -fdump-rtl-loop2_unroll -fno-peel-loops -fno-tree-vrp -fdisable-tree-cunroll -fdisable-tree-cunrolli -fenable-rtl-loop2_unroll=foo2" } */
>  
>  unsigned a[100], b[100];
>  inline void bar()
> --- gcc/testsuite/gcc.dg/tree-ssa/vrp90.c.jj	2013-11-07 11:01:55.137535477 +0100
> +++ gcc/testsuite/gcc.dg/tree-ssa/vrp90.c	2013-11-07 11:02:35.189335924 +0100
> @@ -0,0 +1,36 @@
> +/* { dg-do link } */
> +/* { dg-options "-O2 -fdump-tree-vrp1" } */
> +/* { dg-final { scan-tree-dump-not "link_error" "vrp1"} } */
> +/* { dg-final { cleanup-tree-dump "vrp1" } } */
> +
> +extern void link_error (void);
> +
> +__attribute__((noinline, noclone)) int
> +foo (unsigned int n, int r)
> +{
> +  int i;
> +  if (n > 0)
> +    {
> +      asm ("");
> +      if (n < 10)
> +	{
> +	  asm ("");
> +	  do
> +	    {
> +	      --n;
> +	      r *= 2;
> +	      if (n >= 9)
> +		link_error ();
> +	    }
> +	  while (n > 0);
> +	}
> +    }
> +  return r + n;
> +}
> +
> +int
> +main ()
> +{
> +  foo (7, 2);
> +  return 0;
> +}
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]