Re: [PATCH] Make EVRP propagate into PHIs and remove dead stmts

2016-10-19 Thread Richard Biener
On Tue, 18 Oct 2016, Richard Biener wrote:

> On Tue, 18 Oct 2016, Trevor Saunders wrote:
> 
> > On Tue, Oct 18, 2016 at 02:34:58PM +0200, Richard Biener wrote:
> > > 
> > > The following patch makes EVRP remove stmts that will become dead
> > > after propagation.  For this to work we have to propagate into PHIs
> > > (sth we missed as well).
> > > 
> > > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> > > 
> > > Richard.
> > > 
> > > 2016-10-18  Richard Biener  
> > > 
> > >   * tree-vrp.c (evrp_dom_walker::evrp_dom_walker): Initialize
> > >   stmts_to_remove.
> > >   (evrp_dom_walker::~evrp_dom_walker): Free it.
> > >   (evrp_dom_walker::stmts_to_remove): Add.
> > >   (evrp_dom_walker::before_dom_children): Mark PHIs and stmts
> > >   whose output we fully propagate for removal.  Propagate
> > >   into BB destination PHI arguments.
> > >   (execute_early_vrp): Remove queued stmts.  Dump value ranges
> > >   before stmt removal.
> > > 
> > > Index: gcc/tree-vrp.c
> > > ===
> > > --- gcc/tree-vrp.c(revision 241302)
> > > +++ gcc/tree-vrp.c(working copy)
> > > @@ -10643,11 +10643,13 @@ public:
> > >  : dom_walker (CDI_DOMINATORS), stack (10)
> > >  {
> > >stmts_to_fixup.create (0);
> > > +  stmts_to_remove.create (0);
> > >need_eh_cleanup = BITMAP_ALLOC (NULL);
> > >  }
> > >~evrp_dom_walker ()
> > >  {
> > >stmts_to_fixup.release ();
> > > +  stmts_to_remove.release ();
> > >BITMAP_FREE (need_eh_cleanup);
> > >  }
> > >virtual edge before_dom_children (basic_block);
> > > @@ -10660,6 +10662,7 @@ public:
> > >auto_vec > stack;
> > >bitmap need_eh_cleanup;
> > >vec stmts_to_fixup;
> > > +  vec stmts_to_remove;
> > 
> > That might as well be an auto_vec right?
> > 
> > >  };
> > >  
> > >  
> > > @@ -10769,6 +10772,15 @@ evrp_dom_walker::before_dom_children (ba
> > >else
> > >   set_value_range_to_varying (_result);
> > >update_value_range (lhs, _result);
> > > +
> > > +  /* Mark PHIs whose lhs we fully propagate for removal.  */
> > > +  tree val;
> > > +  if ((val = op_with_constant_singleton_value_range (lhs))
> > > +   && may_propagate_copy (lhs, val))
> > 
> > wouldn't it be clearer to write that as
> > 
> > tree val = op_with_constant_singleton_value_range (lhs);
> > if (val && may_propagate_copy (lhs, val))
> > 
> > > + {
> > > +   stmts_to_remove.safe_push (phi);
> > > +   continue;
> > > + }
> > >  }
> > >  
> > >edge taken_edge = NULL;
> > > @@ -10806,7 +10818,6 @@ evrp_dom_walker::before_dom_children (ba
> > > update_value_range (output, );
> > > vr = *get_value_range (output);
> > >  
> > > -
> > > /* Set the SSA with the value range.  */
> > > if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
> > >   {
> > > @@ -10824,6 +10835,17 @@ evrp_dom_walker::before_dom_children (ba
> > >  && range_includes_zero_p (vr.min,
> > >vr.max) == 1)))
> > >   set_ptr_nonnull (output);
> > > +
> > > +   /* Mark stmts whose output we fully propagate for removal.  */
> > > +   tree val;
> > > +   if ((val = op_with_constant_singleton_value_range (output))
> > > +   && may_propagate_copy (output, val)
> > > +   && !stmt_could_throw_p (stmt)
> > > +   && !gimple_has_side_effects (stmt))
> > 
> > similar.
> 
> Fixed.  Testing the following.

Turns out I need to fix gcc.dg/tree-ssa/pr61839_2.c which didn't test
what it was supposed to anyway.

Committed to trunk.

Richard.


Re: [PATCH] Make EVRP propagate into PHIs and remove dead stmts

2016-10-18 Thread Richard Biener
On Tue, 18 Oct 2016, Trevor Saunders wrote:

> On Tue, Oct 18, 2016 at 02:34:58PM +0200, Richard Biener wrote:
> > 
> > The following patch makes EVRP remove stmts that will become dead
> > after propagation.  For this to work we have to propagate into PHIs
> > (sth we missed as well).
> > 
> > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> > 
> > Richard.
> > 
> > 2016-10-18  Richard Biener  
> > 
> > * tree-vrp.c (evrp_dom_walker::evrp_dom_walker): Initialize
> > stmts_to_remove.
> > (evrp_dom_walker::~evrp_dom_walker): Free it.
> > (evrp_dom_walker::stmts_to_remove): Add.
> > (evrp_dom_walker::before_dom_children): Mark PHIs and stmts
> > whose output we fully propagate for removal.  Propagate
> > into BB destination PHI arguments.
> > (execute_early_vrp): Remove queued stmts.  Dump value ranges
> > before stmt removal.
> > 
> > Index: gcc/tree-vrp.c
> > ===
> > --- gcc/tree-vrp.c  (revision 241302)
> > +++ gcc/tree-vrp.c  (working copy)
> > @@ -10643,11 +10643,13 @@ public:
> >  : dom_walker (CDI_DOMINATORS), stack (10)
> >  {
> >stmts_to_fixup.create (0);
> > +  stmts_to_remove.create (0);
> >need_eh_cleanup = BITMAP_ALLOC (NULL);
> >  }
> >~evrp_dom_walker ()
> >  {
> >stmts_to_fixup.release ();
> > +  stmts_to_remove.release ();
> >BITMAP_FREE (need_eh_cleanup);
> >  }
> >virtual edge before_dom_children (basic_block);
> > @@ -10660,6 +10662,7 @@ public:
> >auto_vec > stack;
> >bitmap need_eh_cleanup;
> >vec stmts_to_fixup;
> > +  vec stmts_to_remove;
> 
> That might as well be an auto_vec right?
> 
> >  };
> >  
> >  
> > @@ -10769,6 +10772,15 @@ evrp_dom_walker::before_dom_children (ba
> >else
> > set_value_range_to_varying (_result);
> >update_value_range (lhs, _result);
> > +
> > +  /* Mark PHIs whose lhs we fully propagate for removal.  */
> > +  tree val;
> > +  if ((val = op_with_constant_singleton_value_range (lhs))
> > + && may_propagate_copy (lhs, val))
> 
> wouldn't it be clearer to write that as
> 
> tree val = op_with_constant_singleton_value_range (lhs);
> if (val && may_propagate_copy (lhs, val))
> 
> > +   {
> > + stmts_to_remove.safe_push (phi);
> > + continue;
> > +   }
> >  }
> >  
> >edge taken_edge = NULL;
> > @@ -10806,7 +10818,6 @@ evrp_dom_walker::before_dom_children (ba
> >   update_value_range (output, );
> >   vr = *get_value_range (output);
> >  
> > -
> >   /* Set the SSA with the value range.  */
> >   if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
> > {
> > @@ -10824,6 +10835,17 @@ evrp_dom_walker::before_dom_children (ba
> >&& range_includes_zero_p (vr.min,
> >  vr.max) == 1)))
> > set_ptr_nonnull (output);
> > +
> > + /* Mark stmts whose output we fully propagate for removal.  */
> > + tree val;
> > + if ((val = op_with_constant_singleton_value_range (output))
> > + && may_propagate_copy (output, val)
> > + && !stmt_could_throw_p (stmt)
> > + && !gimple_has_side_effects (stmt))
> 
> similar.

Fixed.  Testing the following.

Richard.

2016-10-18  Richard Biener  

* tree-vrp.c (evrp_dom_walker::evrp_dom_walker): Initialize
stmts_to_remove.
(evrp_dom_walker::~evrp_dom_walker): Free it.
(evrp_dom_walker::stmts_to_remove): Add.
(evrp_dom_walker::before_dom_children): Mark PHIs and stmts
whose output we fully propagate for removal.  Propagate
into BB destination PHI arguments.
(execute_early_vrp): Remove queued stmts.  Dump value ranges
before stmt removal.

Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 241302)
+++ gcc/tree-vrp.c  (working copy)
@@ -10642,12 +10642,10 @@ public:
   evrp_dom_walker ()
 : dom_walker (CDI_DOMINATORS), stack (10)
 {
-  stmts_to_fixup.create (0);
   need_eh_cleanup = BITMAP_ALLOC (NULL);
 }
   ~evrp_dom_walker ()
 {
-  stmts_to_fixup.release ();
   BITMAP_FREE (need_eh_cleanup);
 }
   virtual edge before_dom_children (basic_block);
@@ -10659,7 +10657,8 @@ public:
   /* Cond_stack holds the old VR.  */
   auto_vec > stack;
   bitmap need_eh_cleanup;
-  vec stmts_to_fixup;
+  auto_vec stmts_to_fixup;
+  auto_vec stmts_to_remove;
 };
 
 
@@ -10769,6 +10768,11 @@ evrp_dom_walker::before_dom_children (ba
   else
set_value_range_to_varying (_result);
   update_value_range (lhs, _result);
+
+  /* Mark PHIs whose lhs we fully propagate for removal.  */
+  tree val = op_with_constant_singleton_value_range (lhs);
+  if (val && may_propagate_copy (lhs, val))
+   

Re: [PATCH] Make EVRP propagate into PHIs and remove dead stmts

2016-10-18 Thread Trevor Saunders
On Tue, Oct 18, 2016 at 02:34:58PM +0200, Richard Biener wrote:
> 
> The following patch makes EVRP remove stmts that will become dead
> after propagation.  For this to work we have to propagate into PHIs
> (sth we missed as well).
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> Richard.
> 
> 2016-10-18  Richard Biener  
> 
>   * tree-vrp.c (evrp_dom_walker::evrp_dom_walker): Initialize
>   stmts_to_remove.
>   (evrp_dom_walker::~evrp_dom_walker): Free it.
>   (evrp_dom_walker::stmts_to_remove): Add.
>   (evrp_dom_walker::before_dom_children): Mark PHIs and stmts
>   whose output we fully propagate for removal.  Propagate
>   into BB destination PHI arguments.
>   (execute_early_vrp): Remove queued stmts.  Dump value ranges
>   before stmt removal.
> 
> Index: gcc/tree-vrp.c
> ===
> --- gcc/tree-vrp.c(revision 241302)
> +++ gcc/tree-vrp.c(working copy)
> @@ -10643,11 +10643,13 @@ public:
>  : dom_walker (CDI_DOMINATORS), stack (10)
>  {
>stmts_to_fixup.create (0);
> +  stmts_to_remove.create (0);
>need_eh_cleanup = BITMAP_ALLOC (NULL);
>  }
>~evrp_dom_walker ()
>  {
>stmts_to_fixup.release ();
> +  stmts_to_remove.release ();
>BITMAP_FREE (need_eh_cleanup);
>  }
>virtual edge before_dom_children (basic_block);
> @@ -10660,6 +10662,7 @@ public:
>auto_vec > stack;
>bitmap need_eh_cleanup;
>vec stmts_to_fixup;
> +  vec stmts_to_remove;

That might as well be an auto_vec right?

>  };
>  
>  
> @@ -10769,6 +10772,15 @@ evrp_dom_walker::before_dom_children (ba
>else
>   set_value_range_to_varying (_result);
>update_value_range (lhs, _result);
> +
> +  /* Mark PHIs whose lhs we fully propagate for removal.  */
> +  tree val;
> +  if ((val = op_with_constant_singleton_value_range (lhs))
> +   && may_propagate_copy (lhs, val))

wouldn't it be clearer to write that as

tree val = op_with_constant_singleton_value_range (lhs);
if (val && may_propagate_copy (lhs, val))

> + {
> +   stmts_to_remove.safe_push (phi);
> +   continue;
> + }
>  }
>  
>edge taken_edge = NULL;
> @@ -10806,7 +10818,6 @@ evrp_dom_walker::before_dom_children (ba
> update_value_range (output, );
> vr = *get_value_range (output);
>  
> -
> /* Set the SSA with the value range.  */
> if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
>   {
> @@ -10824,6 +10835,17 @@ evrp_dom_walker::before_dom_children (ba
>  && range_includes_zero_p (vr.min,
>vr.max) == 1)))
>   set_ptr_nonnull (output);
> +
> +   /* Mark stmts whose output we fully propagate for removal.  */
> +   tree val;
> +   if ((val = op_with_constant_singleton_value_range (output))
> +   && may_propagate_copy (output, val)
> +   && !stmt_could_throw_p (stmt)
> +   && !gimple_has_side_effects (stmt))

similar.

Thanks!

Trev



[PATCH] Make EVRP propagate into PHIs and remove dead stmts

2016-10-18 Thread Richard Biener

The following patch makes EVRP remove stmts that will become dead
after propagation.  For this to work we have to propagate into PHIs
(sth we missed as well).

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

Richard.

2016-10-18  Richard Biener  

* tree-vrp.c (evrp_dom_walker::evrp_dom_walker): Initialize
stmts_to_remove.
(evrp_dom_walker::~evrp_dom_walker): Free it.
(evrp_dom_walker::stmts_to_remove): Add.
(evrp_dom_walker::before_dom_children): Mark PHIs and stmts
whose output we fully propagate for removal.  Propagate
into BB destination PHI arguments.
(execute_early_vrp): Remove queued stmts.  Dump value ranges
before stmt removal.

Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 241302)
+++ gcc/tree-vrp.c  (working copy)
@@ -10643,11 +10643,13 @@ public:
 : dom_walker (CDI_DOMINATORS), stack (10)
 {
   stmts_to_fixup.create (0);
+  stmts_to_remove.create (0);
   need_eh_cleanup = BITMAP_ALLOC (NULL);
 }
   ~evrp_dom_walker ()
 {
   stmts_to_fixup.release ();
+  stmts_to_remove.release ();
   BITMAP_FREE (need_eh_cleanup);
 }
   virtual edge before_dom_children (basic_block);
@@ -10660,6 +10662,7 @@ public:
   auto_vec > stack;
   bitmap need_eh_cleanup;
   vec stmts_to_fixup;
+  vec stmts_to_remove;
 };
 
 
@@ -10769,6 +10772,15 @@ evrp_dom_walker::before_dom_children (ba
   else
set_value_range_to_varying (_result);
   update_value_range (lhs, _result);
+
+  /* Mark PHIs whose lhs we fully propagate for removal.  */
+  tree val;
+  if ((val = op_with_constant_singleton_value_range (lhs))
+ && may_propagate_copy (lhs, val))
+   {
+ stmts_to_remove.safe_push (phi);
+ continue;
+   }
 }
 
   edge taken_edge = NULL;
@@ -10806,7 +10818,6 @@ evrp_dom_walker::before_dom_children (ba
  update_value_range (output, );
  vr = *get_value_range (output);
 
-
  /* Set the SSA with the value range.  */
  if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
{
@@ -10824,6 +10835,17 @@ evrp_dom_walker::before_dom_children (ba
   && range_includes_zero_p (vr.min,
 vr.max) == 1)))
set_ptr_nonnull (output);
+
+ /* Mark stmts whose output we fully propagate for removal.  */
+ tree val;
+ if ((val = op_with_constant_singleton_value_range (output))
+ && may_propagate_copy (output, val)
+ && !stmt_could_throw_p (stmt)
+ && !gimple_has_side_effects (stmt))
+   {
+ stmts_to_remove.safe_push (stmt);
+ continue;
+   }
}
  else
set_defs_to_varying (stmt);
@@ -10860,6 +10882,24 @@ evrp_dom_walker::before_dom_children (ba
}
}
 }
+
+  /* Visit BB successor PHI nodes and replace PHI args.  */
+  FOR_EACH_EDGE (e, ei, bb->succs)
+{
+  for (gphi_iterator gpi = gsi_start_phis (e->dest);
+  !gsi_end_p (gpi); gsi_next ())
+   {
+ gphi *phi = gpi.phi ();
+ use_operand_p use_p = PHI_ARG_DEF_PTR_FROM_EDGE (phi, e);
+ tree arg = USE_FROM_PTR (use_p);
+ if (TREE_CODE (arg) != SSA_NAME
+ || virtual_operand_p (arg))
+   continue;
+ if (tree val = op_with_constant_singleton_value_range (arg))
+   propagate_value (use_p, val);
+   }
+}
+ 
   bb->flags |= BB_VISITED;
 
   return taken_edge;
@@ -10941,6 +10981,34 @@ execute_early_vrp ()
   evrp_dom_walker walker;
   walker.walk (ENTRY_BLOCK_PTR_FOR_FN (cfun));
 
+  if (dump_file)
+{
+  fprintf (dump_file, "\nValue ranges after Early VRP:\n\n");
+  dump_all_value_ranges (dump_file);
+  fprintf (dump_file, "\n");
+}
+
+  /* Remove stmts in reverse order to make debug stmt creation possible.  */
+  while (! walker.stmts_to_remove.is_empty ())
+{
+  gimple *stmt = walker.stmts_to_remove.pop ();
+  if (dump_file && dump_flags & TDF_DETAILS)
+   {
+ fprintf (dump_file, "Removing dead stmt ");
+ print_gimple_stmt (dump_file, stmt, 0, 0);
+ fprintf (dump_file, "\n");
+   }
+  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+  if (gimple_code (stmt) == GIMPLE_PHI)
+   remove_phi_node (, true);
+  else
+   {
+ unlink_stmt_vdef (stmt);
+ gsi_remove (, true);
+ release_defs (stmt);
+   }
+}
+
   if (!bitmap_empty_p (walker.need_eh_cleanup))
 gimple_purge_all_dead_eh_edges (walker.need_eh_cleanup);
 
@@ -10954,12 +11022,6 @@ execute_early_vrp ()
   fixup_noreturn_call (stmt);
 }
 
-  if (dump_file)
-{
-  fprintf (dump_file, "\nValue ranges after Early