Patchwork Avoid doing DCE in substitute_and_fold from VRP (PR tree-optimization/44508)

login
register
mail settings
Submitter Jakub Jelinek
Date June 14, 2010, 1:07 p.m.
Message ID <20100614130718.GJ7811@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/55529/
State New
Headers show

Comments

Jakub Jelinek - June 14, 2010, 1:07 p.m.
Hi!

libgomp.fortran/vla*.f90 fails on s390 on 4.4 branch, because
substitute_and_fold DCEs some trivially dead statements after earlier
substitutions/folding), which releases the LHS SSA_NAMEs.  Those
SSA_NAMEs are still referenced from some VRP ranges though and can result in
crashes, either still in substitute_and_fold in other folding using VRP,
or in subsequent identify_jump_threads call.
The problem is just latent on the trunk.

Bootstrapped/regtested on x86_64-linux and i686-linux on the trunk and on
x86_64-linux, i686-linux and s390-linux so far on 4.4 branch.

	Jakub
2010-06-14  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/44508
	* tree-ssa-propagate.h (substitute_and_fold): Add DO_DCE
	argument.
	* tree-ssa-propagate.c (substitute_and_fold): If !DO_DCE,
	don't eliminate trivially dead stmts.
	* tree-vrp.c (vrp_finalize): Pass false as last argument
	to substitute_and_fold.
	* tree-ssa-copy.c (fini_copy_prop): Pass true as last argument
	to substitute_and_fold.
	* tree-ssa-ccp.c (ccp_finalize): Likewise.

	* gcc.dg/tree-ssa/pr21086.c: Adjust.
2010-06-14  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/44508
	* tree-ssa-propagate.c (substitute_and_fold): Don't do DCE
	when called from tree-vrp.c.

	* gcc.dg/tree-ssa/pr21086.c: Adjust.

--- gcc/tree-ssa-propagate.c.jj	2009-04-24 21:41:29.000000000 +0200
+++ gcc/tree-ssa-propagate.c	2010-06-14 09:39:54.000000000 +0200
@@ -1113,8 +1113,12 @@ substitute_and_fold (prop_value_t *prop_
 	    }
 
 	  /* No point propagating into a stmt whose result is not used,
-	     but instead we might be able to remove a trivially dead stmt.  */
-	  if (gimple_get_lhs (stmt)
+	     but instead we might be able to remove a trivially dead stmt.
+	     Don't do this when called from VRP, since the SSA_NAME which
+	     is going to be released could be still referenced in VRP
+	     ranges.  */
+	  if (!use_ranges_p
+	      && gimple_get_lhs (stmt)
 	      && TREE_CODE (gimple_get_lhs (stmt)) == SSA_NAME
 	      && has_zero_uses (gimple_get_lhs (stmt))
 	      && !stmt_could_throw_p (stmt)
--- gcc/testsuite/gcc.dg/tree-ssa/pr21086.c.jj	2010-06-11 11:00:43.000000000 +0200
+++ gcc/testsuite/gcc.dg/tree-ssa/pr21086.c	2010-06-14 13:14:39.354353715 +0200
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-vrp1" } */
+/* { dg-options "-O2 -fdump-tree-vrp1 -fdump-tree-dce1" } */
 
 int
 foo (int *p)
@@ -16,8 +16,9 @@ foo (int *p)
 }
 
 /* Target with fno-delete-null-pointer-checks should not fold checks */
-/* { dg-final { scan-tree-dump-times "Folding predicate " 1 "vrp1" { target { ! keeps_null_pointer_checks } } } } */
+/* { dg-final { scan-tree-dump "Folding predicate " "vrp1" { target { ! keeps_null_pointer_checks } } } } */
 /* { dg-final { scan-tree-dump-times "Folding predicate " 0 "vrp1" { target {   keeps_null_pointer_checks } } } } */
-/* { dg-final { scan-tree-dump-not "b_. =" "vrp1" { target { ! avr-*-* } } } } */
-/* { dg-final { scan-tree-dump "b_. =" "vrp1" { target { avr-*-* } } } } */
+/* { dg-final { scan-tree-dump-not "b_. =" "dce1" { target { ! avr-*-* } } } } */
+/* { dg-final { scan-tree-dump "b_. =" "dce1" { target { avr-*-* } } } } */
 /* { dg-final { cleanup-tree-dump "vrp1" } } */
+/* { dg-final { cleanup-tree-dump "dce1" } } */
Richard Guenther - June 14, 2010, 1:11 p.m.
On Mon, Jun 14, 2010 at 3:07 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> libgomp.fortran/vla*.f90 fails on s390 on 4.4 branch, because
> substitute_and_fold DCEs some trivially dead statements after earlier
> substitutions/folding), which releases the LHS SSA_NAMEs.  Those
> SSA_NAMEs are still referenced from some VRP ranges though and can result in
> crashes, either still in substitute_and_fold in other folding using VRP,
> or in subsequent identify_jump_threads call.
> The problem is just latent on the trunk.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux on the trunk and on
> x86_64-linux, i686-linux and s390-linux so far on 4.4 branch.

Ok.

Thanks,
Richard.

>        Jakub
>

Patch

--- gcc/tree-ssa-propagate.h.jj	2010-06-11 11:06:00.000000000 +0200
+++ gcc/tree-ssa-propagate.h	2010-06-14 10:25:45.161210781 +0200
@@ -120,6 +120,6 @@  bool valid_gimple_call_p (tree);
 void move_ssa_defining_stmt_for_defs (gimple, gimple);
 bool update_call_from_tree (gimple_stmt_iterator *, tree);
 bool stmt_makes_single_store (gimple);
-bool substitute_and_fold (prop_value_t *, ssa_prop_fold_stmt_fn);
+bool substitute_and_fold (prop_value_t *, ssa_prop_fold_stmt_fn, bool);
 
 #endif /* _TREE_SSA_PROPAGATE_H  */
--- gcc/tree-ssa-propagate.c.jj	2010-06-11 11:06:00.858283899 +0200
+++ gcc/tree-ssa-propagate.c	2010-06-14 10:26:30.399304971 +0200
@@ -964,10 +964,13 @@  replace_phi_args_in (gimple phi, prop_va
    If FOLD_FN is non-NULL the function will be invoked on all statements
    before propagating values for pass specific simplification.
 
+   DO_DCE is true if trivially dead stmts can be removed.
+
    Return TRUE when something changed.  */
 
 bool
-substitute_and_fold (prop_value_t *prop_value, ssa_prop_fold_stmt_fn fold_fn)
+substitute_and_fold (prop_value_t *prop_value, ssa_prop_fold_stmt_fn fold_fn,
+		     bool do_dce)
 {
   basic_block bb;
   bool something_changed = false;
@@ -1012,8 +1015,12 @@  substitute_and_fold (prop_value_t *prop_
 	    continue;
 
 	  /* No point propagating into a stmt whose result is not used,
-	     but instead we might be able to remove a trivially dead stmt.  */
-	  if (gimple_get_lhs (stmt)
+	     but instead we might be able to remove a trivially dead stmt.
+	     Don't do this when called from VRP, since the SSA_NAME which
+	     is going to be released could be still referenced in VRP
+	     ranges.  */
+	  if (do_dce
+	      && gimple_get_lhs (stmt)
 	      && TREE_CODE (gimple_get_lhs (stmt)) == SSA_NAME
 	      && has_zero_uses (gimple_get_lhs (stmt))
 	      && !stmt_could_throw_p (stmt)
--- gcc/tree-ssa-copy.c.jj	2010-06-11 11:06:00.000000000 +0200
+++ gcc/tree-ssa-copy.c	2010-06-14 10:27:32.306210726 +0200
@@ -854,7 +854,7 @@  fini_copy_prop (void)
 	duplicate_ssa_name_ptr_info (tmp[i].value, SSA_NAME_PTR_INFO (var));
     }
 
-  substitute_and_fold (tmp, NULL);
+  substitute_and_fold (tmp, NULL, true);
 
   free (cached_last_copy_of);
   free (copy_of);
--- gcc/tree-ssa-ccp.c.jj	2010-06-11 11:06:00.000000000 +0200
+++ gcc/tree-ssa-ccp.c	2010-06-14 10:27:17.336210560 +0200
@@ -682,7 +682,7 @@  ccp_finalize (void)
 
   do_dbg_cnt ();
   /* Perform substitutions based on the known constant values.  */
-  something_changed = substitute_and_fold (const_val, ccp_fold_stmt);
+  something_changed = substitute_and_fold (const_val, ccp_fold_stmt, true);
 
   free (const_val);
   const_val = NULL;
--- gcc/tree-vrp.c.jj	2010-06-11 11:06:00.000000000 +0200
+++ gcc/tree-vrp.c	2010-06-14 10:27:00.002354677 +0200
@@ -7343,7 +7343,7 @@  vrp_finalize (void)
       single_val_range = NULL;
     }
 
-  substitute_and_fold (single_val_range, vrp_fold_stmt);
+  substitute_and_fold (single_val_range, vrp_fold_stmt, false);
 
   if (warn_array_bounds)
     check_all_array_refs ();
--- gcc/testsuite/gcc.dg/tree-ssa/pr21086.c.jj	2010-06-11 11:00:43.000000000 +0200
+++ gcc/testsuite/gcc.dg/tree-ssa/pr21086.c	2010-06-14 13:14:39.354353715 +0200
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-vrp1" } */
+/* { dg-options "-O2 -fdump-tree-vrp1 -fdump-tree-dce1" } */
 
 int
 foo (int *p)
@@ -16,8 +16,9 @@  foo (int *p)
 }
 
 /* Target with fno-delete-null-pointer-checks should not fold checks */
-/* { dg-final { scan-tree-dump-times "Folding predicate " 1 "vrp1" { target { ! keeps_null_pointer_checks } } } } */
+/* { dg-final { scan-tree-dump "Folding predicate " "vrp1" { target { ! keeps_null_pointer_checks } } } } */
 /* { dg-final { scan-tree-dump-times "Folding predicate " 0 "vrp1" { target {   keeps_null_pointer_checks } } } } */
-/* { dg-final { scan-tree-dump-not "b_. =" "vrp1" { target { ! avr-*-* } } } } */
-/* { dg-final { scan-tree-dump "b_. =" "vrp1" { target { avr-*-* } } } } */
+/* { dg-final { scan-tree-dump-not "b_. =" "dce1" { target { ! avr-*-* } } } } */
+/* { dg-final { scan-tree-dump "b_. =" "dce1" { target { avr-*-* } } } } */
 /* { dg-final { cleanup-tree-dump "vrp1" } } */
+/* { dg-final { cleanup-tree-dump "dce1" } } */