public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Marek Polacek <polacek@redhat.com>
To: Richard Biener <rguenther@suse.de>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, Jakub Jelinek <jakub@redhat.com>
Subject: Re: RFC: Add ADDR_EXPR lowering (PR tree-optimization/66718)
Date: Wed, 08 Jul 2015 17:37:00 -0000	[thread overview]
Message-ID: <20150708173740.GC16027@redhat.com> (raw)
In-Reply-To: <alpine.LSU.2.11.1507031536070.9923@zhemvz.fhfr.qr>

On Fri, Jul 03, 2015 at 03:41:29PM +0200, Richard Biener wrote:
> On Fri, 3 Jul 2015, Marek Polacek wrote:
> 
> > This patch implements a new pass, called laddress, which deals with
> > lowering ADDR_EXPR assignments.  Such lowering ought to help the
> > vectorizer, but it also could expose more CSE opportunities, maybe
> > help reassoc, etc.  It's only active when optimize != 0.
> > 
> > So e.g.
> >   _1 = (sizetype) i_9;
> >   _7 = _1 * 4;
> >   _4 = &b + _7;
> > instead of
> >   _4 = &b[i_9];
> > 
> > This triggered 14105 times during the regtest and 6392 times during
> > the bootstrap.
> > 
> > The fallout (at least on x86_64) is surprisingly small, i.e. none, just
> > gcc.dg/vect/pr59984.c test (using -fopenmp-simd) ICEs, but that is due
> > to a bug in the vectorizer.  Jakub has a patch and knows the details.
> > As the test shows, we're now able to vectorize ADDR_EXPR of non-invariants
> > (that was the motivation of this pass).
> > 
> > This doesn't introduce any kind of verification nor PROP_laddress.
> > Don't know if we want that, but hopefully it can be done as a follow-up
> > if we do.
> 
> Yes.  At the moment nothing requires lowered address form so this is
> merely an optimization (and not a bug for some later pass to
> re-introduce un-lowered non-invariant addresses).  I can imagine
> that for example IVOPTs could be simplified if we didn't have this
> kind of addresses in the IL.
> 
> > Do we want to move some optimizations into this new pass, e.g.
> > from fwprop?
> 
> I think we might want to re-try forwprop_into_addr_expr before lowering
> the address.  Well, but that's maybe just over-cautionous.
> 
> > Thoughts?
> 
> Please move the pass before crited, crited and pre are supposed to
> go together.

Done.
 
> Otherwise looks ok to me.

I renamed the file to gimple-laddress.c then and adjusted the timevar.
Another change is that for x86_64 we don't need -mavx at all, so I dropped
that.  The test is now restricted to x86_64/i?86; on aarch64/ppc64 we aren't
able to vectorize all the functions.

Bootstrapped/regtested on x86_64-linux + ppc64-linux, ok for trunk?

2015-07-08  Marek Polacek  <polacek@redhat.com>

	PR tree-optimization/66718
	* Makefile.in (OBJS): Add gimple-laddress.o. 
	* passes.def: Schedule pass_laddress.
	* timevar.def (DEFTIMEVAR): Add TV_GIMPLE_LADDRESS.
	* tree-pass.h (make_pass_laddress): Declare.
	* gimple-laddress.c: New file.

	* gcc.dg/vect/vect-126.c: New test.

diff --git gcc/Makefile.in gcc/Makefile.in
index 89eda96..1817025 100644
--- gcc/Makefile.in
+++ gcc/Makefile.in
@@ -1255,6 +1255,7 @@ OBJS = \
 	gimple-expr.o \
 	gimple-iterator.o \
 	gimple-fold.o \
+	gimple-laddress.o \
 	gimple-low.o \
 	gimple-match.o \
 	generic-match.o \
diff --git gcc/gimple-laddress.c gcc/gimple-laddress.c
index e69de29..c8036b9 100644
--- gcc/gimple-laddress.c
+++ gcc/gimple-laddress.c
@@ -0,0 +1,137 @@
+/* Lower and optimize address expressions.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   Contributed by Marek Polacek <polacek@redhat.com>
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "alias.h"
+#include "predict.h"
+#include "tm.h"
+#include "function.h"
+#include "dominance.h"
+#include "cfg.h"
+#include "basic-block.h"
+#include "tree-ssa-alias.h"
+#include "symtab.h"
+#include "tree.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
+#include "fold-const.h"
+#include "gimple-expr.h"
+#include "gimple.h"
+#include "gimplify.h"
+#include "gimple-iterator.h"
+#include "gimplify-me.h"
+#include "tree-pass.h"
+
+
+namespace {
+
+const pass_data pass_data_laddress =
+{
+  GIMPLE_PASS, /* type */
+  "laddress", /* name */
+  OPTGROUP_NONE, /* optinfo_flags */
+  TV_GIMPLE_LADDRESS, /* tv_id */
+  ( PROP_cfg | PROP_ssa ), /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  0, /* todo_flags_finish */
+};
+
+class pass_laddress : public gimple_opt_pass
+{
+public:
+  pass_laddress (gcc::context *ctxt)
+    : gimple_opt_pass (pass_data_laddress, ctxt)
+  {}
+
+  /* opt_pass methods: */
+  opt_pass * clone () { return new pass_laddress (m_ctxt); }
+  virtual bool gate (function *) { return optimize != 0; }
+  virtual unsigned int execute (function *);
+
+}; // class pass_laddress
+
+unsigned int
+pass_laddress::execute (function *fun)
+{
+  basic_block bb;
+
+  FOR_EACH_BB_FN (bb, fun)
+    {
+      for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
+	{
+	  gimple stmt = gsi_stmt (gsi);
+	  if (!is_gimple_assign (stmt)
+	      || gimple_assign_rhs_code (stmt) != ADDR_EXPR
+	      || is_gimple_invariant_address (gimple_assign_rhs1 (stmt)))
+	    {
+	      gsi_next (&gsi);
+	      continue;
+	    }
+
+	  /* Lower ADDR_EXPR assignments:
+	       _4 = &b[i_9];
+	     into
+	       _1 = (sizetype) i_9;
+	       _7 = _1 * 4;
+	       _4 = &b + _7;
+	     This ought to aid the vectorizer and expose CSE opportunities.
+	  */
+
+	  tree expr = gimple_assign_rhs1 (stmt);
+	  HOST_WIDE_INT bitsize, bitpos;
+	  tree base, offset;
+	  machine_mode mode;
+	  int volatilep = 0, unsignedp = 0;
+	  base = get_inner_reference (TREE_OPERAND (expr, 0), &bitsize,
+				      &bitpos, &offset, &mode, &unsignedp,
+				      &volatilep, false);
+	  gcc_assert (base != NULL_TREE && (bitpos % BITS_PER_UNIT) == 0);
+	  if (offset != NULL_TREE)
+	    {
+	      if (bitpos != 0)
+		offset = size_binop (PLUS_EXPR, offset,
+				     size_int (bitpos / BITS_PER_UNIT));
+	      offset = force_gimple_operand_gsi (&gsi, offset, true, NULL,
+						 true, GSI_SAME_STMT);
+	      base = build_fold_addr_expr (base);
+	      base = force_gimple_operand_gsi (&gsi, base, true, NULL,
+					       true, GSI_SAME_STMT);
+	      gimple g = gimple_build_assign (gimple_assign_lhs (stmt),
+					      POINTER_PLUS_EXPR, base, offset);
+	      gsi_replace (&gsi, g, false);
+	    }
+	  gsi_next (&gsi);
+	}
+    }
+
+  return 0;
+}
+
+} // anon namespace
+
+gimple_opt_pass *
+make_pass_laddress (gcc::context *ctxt)
+{
+  return new pass_laddress (ctxt);
+}
diff --git gcc/passes.def gcc/passes.def
index 0d8356b..5cd07ae 100644
--- gcc/passes.def
+++ gcc/passes.def
@@ -213,6 +213,7 @@ along with GCC; see the file COPYING3.  If not see
 	 form if possible.  */
       NEXT_PASS (pass_cse_sincos);
       NEXT_PASS (pass_optimize_bswap);
+      NEXT_PASS (pass_laddress);
       NEXT_PASS (pass_split_crit_edges);
       NEXT_PASS (pass_pre);
       NEXT_PASS (pass_sink_code);
diff --git gcc/testsuite/gcc.dg/vect/vect-126.c gcc/testsuite/gcc.dg/vect/vect-126.c
index e69de29..e269ad3 100644
--- gcc/testsuite/gcc.dg/vect/vect-126.c
+++ gcc/testsuite/gcc.dg/vect/vect-126.c
@@ -0,0 +1,63 @@
+/* PR tree-optimization/66718 */
+/* { dg-do compile } */
+
+int *a[1024], b[1024];
+struct S { int u, v, w, x; };
+struct S c[1024];
+int d[1024][10];
+
+void
+f0 (void)
+{
+  for (int i = 0; i < 1024; i++)
+    a[i] = &b[0];
+}
+
+void
+f1 (void)
+{
+  for (int i = 0; i < 1024; i++)
+    {
+      int *p = &b[0];
+      a[i] = p + i;
+    }
+}
+
+void
+f2 (int *p)
+{
+  for (int i = 0; i < 1024; i++)
+    a[i] = &p[i];
+}
+
+void
+f3 (void)
+{
+  for (int i = 0; i < 1024; i++)
+    a[i] = &b[i];
+}
+
+void
+f4 (void)
+{
+  int *p = &c[0].v;
+  for (int i = 0; i < 1024; i++)
+    a[i] = &p[4 * i];
+}
+
+void
+f5 (void)
+{
+  for (int i = 0; i < 1024; i++)
+    a[i] = &c[i].v;
+}
+
+void
+f6 (void)
+{
+  for (int i = 0; i < 1024; i++)
+    for (unsigned int j = 0; j < 10; j++)
+      a[i] = &d[i][j];
+}
+
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops in function" 7 "vect" { target { i?86-*-* x86_64-*-* } } } } */
diff --git gcc/timevar.def gcc/timevar.def
index efac4b7..ff22909 100644
--- gcc/timevar.def
+++ gcc/timevar.def
@@ -275,6 +275,7 @@ DEFTIMEVAR (TV_GIMPLE_SLSR           , "straight-line strength reduction")
 DEFTIMEVAR (TV_VTABLE_VERIFICATION   , "vtable verification")
 DEFTIMEVAR (TV_TREE_UBSAN            , "tree ubsan")
 DEFTIMEVAR (TV_INITIALIZE_RTL        , "initialize rtl")
+DEFTIMEVAR (TV_GIMPLE_LADDRESS         , "address lowering")
 
 /* Everything else in rest_of_compilation not included above.  */
 DEFTIMEVAR (TV_EARLY_LOCAL	     , "early local passes")
diff --git gcc/tree-pass.h gcc/tree-pass.h
index 2808dad..c47b22e 100644
--- gcc/tree-pass.h
+++ gcc/tree-pass.h
@@ -393,6 +393,7 @@ extern gimple_opt_pass *make_pass_cd_dce (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_call_cdce (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_merge_phi (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_split_crit_edges (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_laddress (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_pre (gcc::context *ctxt);
 extern unsigned int tail_merge_optimize (unsigned int);
 extern gimple_opt_pass *make_pass_profile (gcc::context *ctxt);

	Marek

  parent reply	other threads:[~2015-07-08 17:37 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-03 13:21 Marek Polacek
2015-07-03 13:41 ` Richard Biener
2015-07-03 13:43   ` Richard Biener
2015-07-03 14:06   ` Jakub Jelinek
2015-07-03 17:13     ` Richard Biener
2015-07-04 14:19     ` Jakub Jelinek
2015-07-09  9:14       ` Richard Biener
2015-07-09  9:21         ` Jakub Jelinek
2015-07-09 21:12           ` Jakub Jelinek
2015-07-08 17:37   ` Marek Polacek [this message]
2015-07-08 22:23     ` Marek Polacek
2015-07-09  8:53     ` Richard Biener
2015-07-09  9:04       ` Marek Polacek
2015-07-09  9:06         ` Jakub Jelinek
2015-07-04  7:20 ` Jakub Jelinek
2015-07-04 14:17   ` Jakub Jelinek
2015-07-04 14:57     ` Richard Biener
2015-07-05  3:10   ` Bin.Cheng
2015-07-08 17:33     ` Marek Polacek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150708173740.GC16027@redhat.com \
    --to=polacek@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).