Patchwork Call estimate_numbers_of_iterations after insert_range_assertions (PR tree-optimization/49419)

login
register
mail settings
Submitter Jakub Jelinek
Date June 15, 2011, 6:57 p.m.
Message ID <20110615185701.GL17079@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/100563/
State New
Headers show

Comments

Jakub Jelinek - June 15, 2011, 6:57 p.m.
Hi!

The following testcase is miscompiled e.g. on powerpc (not on x86_64/i686
due to different ivopts choices), because estimate_numbers_of_iterations
was called before assert_exprs are added to the IL and gets info cached
until adjust_range_with_scev, where using the original SSA_NAMEs leads
to problems where VR changes might not be propagated properly.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk/4.6?  I'll work on followup improvements for the trunk
afterwards.

2011-06-15  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/49419
	* tree-vrp.c (execute_vrp): Call init_range_assertions
	before estimate_numbers_of_iterations, call
	free_number_of_iterations_estimates before calling
	remove_range_assertions.

	* gcc.c-torture/execute/pr49419.c: New test.


	Jakub
Richard Guenther - June 16, 2011, 7:36 a.m.
On Wed, Jun 15, 2011 at 8:57 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The following testcase is miscompiled e.g. on powerpc (not on x86_64/i686
> due to different ivopts choices), because estimate_numbers_of_iterations
> was called before assert_exprs are added to the IL and gets info cached
> until adjust_range_with_scev, where using the original SSA_NAMEs leads
> to problems where VR changes might not be propagated properly.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk/4.6?  I'll work on followup improvements for the trunk
> afterwards.

Ok.

Thanks,
Richard.

> 2011-06-15  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/49419
>        * tree-vrp.c (execute_vrp): Call init_range_assertions
>        before estimate_numbers_of_iterations, call
>        free_number_of_iterations_estimates before calling
>        remove_range_assertions.
>
>        * gcc.c-torture/execute/pr49419.c: New test.
>
> --- gcc/tree-vrp.c.jj   2011-05-31 08:03:10.000000000 +0200
> +++ gcc/tree-vrp.c      2011-06-15 17:25:32.000000000 +0200
> @@ -7730,14 +7730,14 @@ execute_vrp (void)
>   rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
>   scev_initialize ();
>
> +  insert_range_assertions ();
> +
>   /* Estimate number of iterations - but do not use undefined behavior
>      for this.  We can't do this lazily as other functions may compute
>      this using undefined behavior.  */
>   free_numbers_of_iterations_estimates ();
>   estimate_numbers_of_iterations (false);
>
> -  insert_range_assertions ();
> -
>   to_remove_edges = VEC_alloc (edge, heap, 10);
>   to_update_switch_stmts = VEC_alloc (switch_update, heap, 5);
>   threadedge_initialize_values ();
> @@ -7746,6 +7746,8 @@ execute_vrp (void)
>   ssa_propagate (vrp_visit_stmt, vrp_visit_phi_node);
>   vrp_finalize ();
>
> +  free_numbers_of_iterations_estimates ();
> +
>   /* ASSERT_EXPRs must be removed before finalizing jump threads
>      as finalizing jump threads calls the CFG cleanup code which
>      does not properly handle ASSERT_EXPRs.  */
> --- gcc/testsuite/gcc.c-torture/execute/pr49419.c.jj    2011-06-15 18:06:43.000000000 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr49419.c       2011-06-15 18:05:18.000000000 +0200
> @@ -0,0 +1,38 @@
> +/* PR tree-optimization/49419 */
> +
> +extern void abort (void);
> +
> +struct S { int w, x, y; } *t;
> +
> +int
> +foo (int n, int f, int *s, int m)
> +{
> +  int x, i, a;
> +  if (n == -1)
> +    return 0;
> +  for (x = n, i = 0; t[x].w == f && i < m; i++)
> +    x = t[x].x;
> +  if (i == m)
> +    abort ();
> +  a = i + 1;
> +  for (x = n; i > 0; i--)
> +    {
> +      s[i] = t[x].y;
> +      x = t[x].x;
> +    }
> +  s[0] = x;
> +  return a;
> +}
> +
> +int
> +main (void)
> +{
> +  int s[3], i;
> +  struct S buf[3] = { { 1, 1, 2 }, { 0, 0, 0 }, { 0, 0, 0 } };
> +  t = buf;
> +  if (foo (0, 1, s, 3) != 2)
> +    abort ();
> +  if (s[0] != 1 || s[1] != 2)
> +    abort ();
> +  return 0;
> +}
>
>        Jakub
>

Patch

--- gcc/tree-vrp.c.jj	2011-05-31 08:03:10.000000000 +0200
+++ gcc/tree-vrp.c	2011-06-15 17:25:32.000000000 +0200
@@ -7730,14 +7730,14 @@  execute_vrp (void)
   rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
   scev_initialize ();
 
+  insert_range_assertions ();
+
   /* Estimate number of iterations - but do not use undefined behavior
      for this.  We can't do this lazily as other functions may compute
      this using undefined behavior.  */
   free_numbers_of_iterations_estimates ();
   estimate_numbers_of_iterations (false);
 
-  insert_range_assertions ();
-
   to_remove_edges = VEC_alloc (edge, heap, 10);
   to_update_switch_stmts = VEC_alloc (switch_update, heap, 5);
   threadedge_initialize_values ();
@@ -7746,6 +7746,8 @@  execute_vrp (void)
   ssa_propagate (vrp_visit_stmt, vrp_visit_phi_node);
   vrp_finalize ();
 
+  free_numbers_of_iterations_estimates ();
+
   /* ASSERT_EXPRs must be removed before finalizing jump threads
      as finalizing jump threads calls the CFG cleanup code which
      does not properly handle ASSERT_EXPRs.  */
--- gcc/testsuite/gcc.c-torture/execute/pr49419.c.jj	2011-06-15 18:06:43.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr49419.c	2011-06-15 18:05:18.000000000 +0200
@@ -0,0 +1,38 @@ 
+/* PR tree-optimization/49419 */
+
+extern void abort (void);
+
+struct S { int w, x, y; } *t;
+
+int
+foo (int n, int f, int *s, int m)
+{
+  int x, i, a;
+  if (n == -1)
+    return 0;
+  for (x = n, i = 0; t[x].w == f && i < m; i++)
+    x = t[x].x;
+  if (i == m)
+    abort ();
+  a = i + 1;
+  for (x = n; i > 0; i--)
+    {
+      s[i] = t[x].y;
+      x = t[x].x;
+    }
+  s[0] = x;
+  return a;
+}
+
+int
+main (void)
+{
+  int s[3], i;
+  struct S buf[3] = { { 1, 1, 2 }, { 0, 0, 0 }, { 0, 0, 0 } };
+  t = buf;
+  if (foo (0, 1, s, 3) != 2)
+    abort ();
+  if (s[0] != 1 || s[1] != 2)
+    abort ();
+  return 0;
+}