diff mbox

[0/n] Merge from match-and-simplify

Message ID alpine.LSU.2.11.1410171009390.9891@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Oct. 17, 2014, 8:17 a.m. UTC
On Fri, 17 Oct 2014, Ramana Radhakrishnan wrote:

> On Wed, Oct 15, 2014 at 5:29 PM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> >
> > On 15/10/14 14:00, Richard Biener wrote:
> >>
> >>
> >> Any comments and reviews welcome (I don't think that
> >> my maintainership covers enough to simply check this in
> >> without approval).
> >>
> > Hi Richard,
> >
> > The match-and-simplify branch bootstrapped successfully on
> > aarch64-none-linux-gnu FWIW.
> >
> 
> What about regression tests ?

Note the branch isn't regression free on x86_64 either.  The branch
does more than I want to merge to trunk (and it also retains all
folding code I added patterns for).  I've gone farther there to
explore whether it will end up working in the end and what kind
of features the IL and the APIs need.

I've pasted testsuite results on x86_64 below for rev. 216324
which is based on trunk rev. 216315 which unfortunately has
lots of regressions on its own.

This is why I want to restrict the effect of the machinery to
fold (), fold_stmt () and tree-ssa-forwprop.c for the moment
and merge individual patterns (well, maybe in small groups)
separately to allow for easy bi-section.

I suppose I should push the most visible change to trunk first,
namely tree-ssa-forwprop.c folding all statements via fold_stmt
after the merge.  I suspect this alone can have some odd effects
like the sub + cmp fusing.  That would be sth like the patch
attached below.

Richard.

Comments

Richard Biener Oct. 17, 2014, 11:51 a.m. UTC | #1
On Fri, 17 Oct 2014, Richard Biener wrote:

> On Fri, 17 Oct 2014, Ramana Radhakrishnan wrote:
> 
> > On Wed, Oct 15, 2014 at 5:29 PM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> > >
> > > On 15/10/14 14:00, Richard Biener wrote:
> > >>
> > >>
> > >> Any comments and reviews welcome (I don't think that
> > >> my maintainership covers enough to simply check this in
> > >> without approval).
> > >>
> > > Hi Richard,
> > >
> > > The match-and-simplify branch bootstrapped successfully on
> > > aarch64-none-linux-gnu FWIW.
> > >
> > 
> > What about regression tests ?
> 
> Note the branch isn't regression free on x86_64 either.  The branch
> does more than I want to merge to trunk (and it also retains all
> folding code I added patterns for).  I've gone farther there to
> explore whether it will end up working in the end and what kind
> of features the IL and the APIs need.
> 
> I've pasted testsuite results on x86_64 below for rev. 216324
> which is based on trunk rev. 216315 which unfortunately has
> lots of regressions on its own.
> 
> This is why I want to restrict the effect of the machinery to
> fold (), fold_stmt () and tree-ssa-forwprop.c for the moment
> and merge individual patterns (well, maybe in small groups)
> separately to allow for easy bi-section.
> 
> I suppose I should push the most visible change to trunk first,
> namely tree-ssa-forwprop.c folding all statements via fold_stmt
> after the merge.  I suspect this alone can have some odd effects
> like the sub + cmp fusing.  That would be sth like the patch
> attached below.

Just finished testing this (with -m32 on x86_64), showing regressions
in the testsuite like

FAIL: gcc.dg/tree-ssa/slsr-19.c scan-tree-dump-times optimized " \\\\* y" 
1
FAIL: gcc.dg/vect/bb-slp-27.c -flto -ffat-lto-objects  
scan-tree-dump-times slp2
 "basic block vectorized" 1
FAIL: gcc.dg/vect/bb-slp-27.c scan-tree-dump-times slp2 "basic block 
vectorized"
 1
FAIL: gcc.dg/vect/bb-slp-8b.c -flto -ffat-lto-objects  
scan-tree-dump-times slp2
 "basic block vectorized" 1
FAIL: gcc.dg/vect/bb-slp-8b.c scan-tree-dump-times slp2 "basic block 
vectorized"
 1
FAIL: gcc.dg/vect/slp-cond-3.c -flto -ffat-lto-objects  
scan-tree-dump-times vec
t "vectorizing stmts using SLP" 1
FAIL: gcc.dg/vect/slp-cond-3.c scan-tree-dump-times vect "vectorizing 
stmts usin
g SLP" 1

Bah.

I suppose I need to investigate this (simply folding a stmt shouldn't
cause any of the above... - with SLP it is probably operand
canonicalization, but not sure).

Richard.

> Richard.
> 
> Index: gcc/tree-ssa-forwprop.c
> ===================================================================
> --- gcc/tree-ssa-forwprop.c	(revision 216258)
> +++ gcc/tree-ssa-forwprop.c	(working copy)
> @@ -54,6 +54,8 @@ along with GCC; see the file COPYING3.
>  #include "tree-ssa-propagate.h"
>  #include "tree-ssa-dom.h"
>  #include "builtins.h"
> +#include "tree-cfgcleanup.h"
> +#include "tree-into-ssa.h"
>  
>  /* This pass propagates the RHS of assignment statements into use
>     sites of the LHS of the assignment.  It's basically a specialized
> @@ -3586,6 +3588,8 @@ simplify_mult (gimple_stmt_iterator *gsi
>  
>    return false;
>  }
> +
> +
>  /* Main entry point for the forward propagation and statement combine
>     optimizer.  */
>  
> @@ -3626,6 +3630,40 @@ pass_forwprop::execute (function *fun)
>  
>    cfg_changed = false;
>  
> +  /* Combine stmts with the stmts defining their operands.  Do that
> +     in an order that guarantees visiting SSA defs before SSA uses.  */
> +  int *postorder = XNEWVEC (int, n_basic_blocks_for_fn (fun));
> +  int postorder_num = inverted_post_order_compute (postorder);
> +  for (int i = 0; i < postorder_num; ++i)
> +    {
> +      bb = BASIC_BLOCK_FOR_FN (fun, postorder[i]);
> +      for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
> +	   !gsi_end_p (gsi); gsi_next (&gsi))
> +	{
> +	  gimple stmt = gsi_stmt (gsi);
> +	  gimple orig_stmt = stmt;
> +
> +	  if (fold_stmt (&gsi))
> +	    {
> +	      stmt = gsi_stmt (gsi);
> +	      if (maybe_clean_or_replace_eh_stmt (orig_stmt, stmt)
> +		  && gimple_purge_dead_eh_edges (bb))
> +		cfg_changed = true;
> +	      update_stmt (stmt);
> +	    }
> +	}
> +    }
> +  free (postorder);
> +
> +  /* ???  Code below doesn't expect non-renamed VOPs and the above
> +     doesn't keep virtual operand form up-to-date.  */
> +  if (cfg_changed)
> +    {
> +      cleanup_tree_cfg ();
> +      cfg_changed = false;
> +    }
> +  update_ssa (TODO_update_ssa_only_virtuals);
> +
>    FOR_EACH_BB_FN (bb, fun)
>      {
>        gimple_stmt_iterator gsi;
>
diff mbox

Patch

Index: gcc/tree-ssa-forwprop.c
===================================================================
--- gcc/tree-ssa-forwprop.c	(revision 216258)
+++ gcc/tree-ssa-forwprop.c	(working copy)
@@ -54,6 +54,8 @@  along with GCC; see the file COPYING3.
 #include "tree-ssa-propagate.h"
 #include "tree-ssa-dom.h"
 #include "builtins.h"
+#include "tree-cfgcleanup.h"
+#include "tree-into-ssa.h"
 
 /* This pass propagates the RHS of assignment statements into use
    sites of the LHS of the assignment.  It's basically a specialized
@@ -3586,6 +3588,8 @@  simplify_mult (gimple_stmt_iterator *gsi
 
   return false;
 }
+
+
 /* Main entry point for the forward propagation and statement combine
    optimizer.  */
 
@@ -3626,6 +3630,40 @@  pass_forwprop::execute (function *fun)
 
   cfg_changed = false;
 
+  /* Combine stmts with the stmts defining their operands.  Do that
+     in an order that guarantees visiting SSA defs before SSA uses.  */
+  int *postorder = XNEWVEC (int, n_basic_blocks_for_fn (fun));
+  int postorder_num = inverted_post_order_compute (postorder);
+  for (int i = 0; i < postorder_num; ++i)
+    {
+      bb = BASIC_BLOCK_FOR_FN (fun, postorder[i]);
+      for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
+	   !gsi_end_p (gsi); gsi_next (&gsi))
+	{
+	  gimple stmt = gsi_stmt (gsi);
+	  gimple orig_stmt = stmt;
+
+	  if (fold_stmt (&gsi))
+	    {
+	      stmt = gsi_stmt (gsi);
+	      if (maybe_clean_or_replace_eh_stmt (orig_stmt, stmt)
+		  && gimple_purge_dead_eh_edges (bb))
+		cfg_changed = true;
+	      update_stmt (stmt);
+	    }
+	}
+    }
+  free (postorder);
+
+  /* ???  Code below doesn't expect non-renamed VOPs and the above
+     doesn't keep virtual operand form up-to-date.  */
+  if (cfg_changed)
+    {
+      cleanup_tree_cfg ();
+      cfg_changed = false;
+    }
+  update_ssa (TODO_update_ssa_only_virtuals);
+
   FOR_EACH_BB_FN (bb, fun)
     {
       gimple_stmt_iterator gsi;