[Bug tree-optimization/85757] tree optimizers fail to fully clean up fixed-size memcpy

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85757

Richard Biener  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #6 from Richard Biener  ---
Fixed on trunk.

[Bug tree-optimization/85757] tree optimizers fail to fully clean up fixed-size memcpy

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85757

--- Comment #5 from Richard Biener  ---
Author: rguenth
Date: Thu May 17 06:57:45 2018
New Revision: 260306

URL: https://gcc.gnu.org/viewcvs?rev=260306=gcc=rev
Log:
2018-05-17  Richard Biener  

PR tree-optimization/85757
* tree-ssa-dse.c (dse_classify_store): Record a PHI def and
remove defs that only feed that PHI from further processing.

* gcc.dg/tree-ssa/ssa-dse-34.c: New testcase.

Added:
trunk/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-34.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-ssa-dse.c

[Bug tree-optimization/85757] tree optimizers fail to fully clean up fixed-size memcpy

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85757

--- Comment #4 from Richard Biener  ---
Ontop of recent patches:

diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 32a25b9eb1e..1b672ad204a 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -577,6 +577,7 @@ dse_classify_store (ao_ref *ref, gimple *stmt,
   else
defvar = gimple_vdef (temp);
   auto_vec defs;
+  gimple *phi_def = NULL;
   FOR_EACH_IMM_USE_STMT (use_stmt, ui, defvar)
{
  /* Limit stmt walking.  */
@@ -600,7 +601,10 @@ dse_classify_store (ao_ref *ref, gimple *stmt,
 processing.  */
  if (!bitmap_bit_p (visited,
 SSA_NAME_VERSION (PHI_RESULT (use_stmt
-   defs.safe_push (use_stmt);
+   {
+ defs.safe_push (use_stmt);
+ phi_def = use_stmt;
+   }
}
  /* If the statement is a use the store is not dead.  */
  else if (ref_maybe_used_by_stmt_p (use_stmt, ref))
@@ -660,12 +671,25 @@ dse_classify_store (ao_ref *ref, gimple *stmt,
   /* Process defs and remove paths starting with a kill from further
  processing.  */
   for (unsigned i = 0; i < defs.length (); ++i)
-   if (stmt_kills_ref_p (defs[i], ref))
- {
-   if (by_clobber_p && !gimple_clobber_p (defs[i]))
- *by_clobber_p = false;
+   {
+ gimple *def = defs[i];
+ gimple *use_stmt;
+ use_operand_p use_p;
+ if (stmt_kills_ref_p (def, ref))
+   {
+ if (by_clobber_p && !gimple_clobber_p (def))
+   *by_clobber_p = false;
+ defs.unordered_remove (i);
+   }
+ /* 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).  */
+ else if (gimple_code (def) != GIMPLE_PHI
+  && single_imm_use (gimple_vdef (def), _p, _stmt)
+  && use_stmt == phi_def)
defs.unordered_remove (i);
- }
+   }

   /* If all defs kill the ref we are done.  */
   if (defs.is_empty ())

[Bug tree-optimization/85757] tree optimizers fail to fully clean up fixed-size memcpy

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85757

Richard Biener  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |rguenth at gcc dot 
gnu.org

--- Comment #3 from Richard Biener  ---
Mine.  Working on some refactoring to make this possible.

[Bug tree-optimization/85757] tree optimizers fail to fully clean up fixed-size memcpy

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85757

Richard Biener  changed:

   What|Removed |Added

 CC||kugan at gcc dot gnu.org,
   ||rguenth at gcc dot gnu.org

--- Comment #2 from Richard Biener  ---
This is the failure to handle non-aliasing stores.  We get it recorded here:

  /* If this is a store, remember it or bail out if we have
 multiple ones (the will be in different CFG parts then).  */
  else if (gimple_vdef (use_stmt))
{
  if (temp)
{
  fail = true;
  BREAK_FROM_IMM_USE_STMT (ui);
}
  temp = use_stmt;
}

but that's necessary for the walking algorithm.  Basically PHI handling was
added for a few ad-hoc cases.

diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 9220fea7f2e..ab5aa5bb314 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -569,7 +569,8 @@ dse_classify_store (ao_ref *ref, gimple *stmt, gimple
**use_stmt,
 See gcc.c-torture/execute/20051110-*.c.  */
  else if (gimple_code (use_stmt) == GIMPLE_PHI)
{
- if (temp
+ if ((temp
+  && stmt_may_clobber_ref_p_1 (temp, ref))
  /* Make sure we are not in a loop latch block.  */
  || gimple_bb (stmt) == gimple_bb (use_stmt)
  || dominated_by_p (CDI_DOMINATORS,

"solves" half of it.  But the above snipped needs similar treatment for
the case of temp being the PHI.

Basically a more programatical approach of walking the individual paths
into the PHI node is required which, for clarity, should require
refactoring of the walk.  walk_nonaliased_vuses and friends may or may not
be a good copying source.

[Bug tree-optimization/85757] tree optimizers fail to fully clean up fixed-size memcpy

[glisse at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85757

Marc Glisse  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2018-05-12
Version|unknown |9.0
 Ever confirmed|0   |1

--- Comment #1 from Marc Glisse  ---
Yes, DSE is known to be pretty bad at handling branches, there may be a dup
somewhere.