Another DSE improvement and thinko fix

Message ID alpine.LSU.2.20.1805171413110.24704@zhemvz.fhfr.qr
State New
Headers show
Series
  • Another DSE improvement and thinko fix
Related show

Commit Message

Richard Biener May 17, 2018, 12:15 p.m.
The previous DSE improvements left us with skipping elements we could
have possibly removed because I messed up the iterator increment
upon removal.  The following fixes this and also adds another pruning
opportunity in case the only stmt feeded by the def is an already
visited PHI.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

2018-05-17  Richard Biener  <rguenther@suse.de>

        * tree-ssa-dse.c (dse_classify_store): Fix iterator increment
        for pruning loop and prune defs feeding only already visited PHIs.

Comments

Jeff Law May 21, 2018, 8:27 p.m. | #1
On 05/17/2018 06:15 AM, Richard Biener wrote:
> 
> The previous DSE improvements left us with skipping elements we could
> have possibly removed because I messed up the iterator increment
> upon removal.  The following fixes this and also adds another pruning
> opportunity in case the only stmt feeded by the def is an already
> visited PHI.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> Richard.
> 
> 2018-05-17  Richard Biener  <rguenther@suse.de>
> 
>         * tree-ssa-dse.c (dse_classify_store): Fix iterator increment
>         for pruning loop and prune defs feeding only already visited PHIs.
LGTM.
jeff

Patch

diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 589cfef5df5..28dc95f1740 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -662,7 +669,7 @@  dse_classify_store (ao_ref *ref, gimple *stmt,
 	}
 
       /* Process defs and remove those we need not process further.  */
-      for (unsigned i = 0; i < defs.length (); ++i)
+      for (unsigned i = 0; i < defs.length ();)
 	{
 	  gimple *def = defs[i];
 	  gimple *use_stmt;
@@ -680,11 +687,18 @@  dse_classify_store (ao_ref *ref, gimple *stmt,
 	  /* In addition to kills we can remove defs whose only use
 	     is another def in defs.  That can only ever be PHIs of which
 	     we track a single for simplicity reasons (we fail for multiple
-	     PHIs anyways).  */
+	     PHIs anyways).  We can also ignore defs that feed only into
+	     already visited PHIs.  */
 	  else if (gimple_code (def) != GIMPLE_PHI
 		   && single_imm_use (gimple_vdef (def), &use_p, &use_stmt)
-		   && use_stmt == phi_def)
+		   && (use_stmt == phi_def
+		       || (gimple_code (use_stmt) == GIMPLE_PHI
+			   && bitmap_bit_p (visited,
+					    SSA_NAME_VERSION
+					      (PHI_RESULT (use_stmt))))))
 	    defs.unordered_remove (i);
+	  else
+	    ++i;
 	}
 
       /* If all defs kill the ref we are done.  */