public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Jambor <mjambor@suse.cz>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Richard Biener <rguenther@suse.de>
Subject: [PR 80898] Propagate grp_write from disqualified SRA candidates
Date: Thu, 01 Jun 2017 11:39:00 -0000	[thread overview]
Message-ID: <20170601113923.rx3sdkm4xjik5zzt@virgil.suse.cz> (raw)

Hi,

when I wrote the lazy setting of grp_write flag early next year, I
made a mistake when thinking about what to do about SRA candidates
that were disqualified but form a RHS of an assignment link which was
to be used to set grp_write of the LHS when appropriate.  The code
expects that the RHS accesses form an access tree, but given that some
are rejected exactly because such a tree cannot be built, it does not
work.

The solution is to move dealing with disqualified RHSs to the
assignment link processing.  The patch below checks RHS and if it is
disqualified, marks the corresponding LHS as containing data.  As the
second testcase shows, that information must be then also propagated
downwards (this is not necessary in the normal propagation case
because there propagate_subaccesses_across_link will already do that
more elaborately) as well as upwards.

Bootstrapped and tested on x86_64-linux without any issues. OK for
trunk?

Thanks,

Martin


2017-06-01  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/80898
	* tree-sra.c (process_subtree_disqualification): Removed.
	(disqualify_candidate): Do not call
	process_subtree_disqualification.
	(subtree_mark_written_and_enqueue): New function.
	(propagate_all_subaccesses): Set grp_write of LHS subtree if the
	RHS has been disqualified and re-queue LHS if necessary.  Apart
	from that, ignore disqualified RHS.

testsuite/
	* gcc.dg/tree-ssa/pr80898.c: New test.
	* gcc.dg/tree-ssa/pr80898-2.c: Likewise.
---
 gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c | 71 +++++++++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/pr80898.c   | 20 +++++++++
 gcc/tree-sra.c                            | 56 +++++++++++++++---------
 3 files changed, 126 insertions(+), 21 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr80898.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c
new file mode 100644
index 00000000000..cb4799c5ced
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80898-2.c
@@ -0,0 +1,71 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+struct S0
+{
+  unsigned a : 15;
+  int b;
+  int c;
+};
+
+struct S1
+{
+  struct S0 s0;
+  int e;
+};
+
+struct Z
+{
+  char c;
+  int z;
+} __attribute__((packed));
+
+union U
+{
+  struct S1 s1;
+  struct Z z;
+};
+
+
+int __attribute__((noinline, noclone))
+return_zero (void)
+{
+  return 0;
+}
+
+volatile union U gu;
+struct S0 gs;
+
+int __attribute__((noinline, noclone))
+check_outcome ()
+{
+  if (gs.a != 6
+      || gs.b != 80000)
+    __builtin_abort ();
+}
+
+int
+main (int argc, char *argv[])
+{
+  union U u;
+  struct S1 m;
+  struct S0 l;
+
+  if (return_zero ())
+    u.z.z = 20000;
+  else
+    {
+      u.s1.s0.a = 6;
+      u.s1.s0.b = 80000;
+      u.s1.e = 2;
+
+      m = u.s1;
+      m.s0.c = 0;
+      l = m.s0;
+      gs = l;
+    }
+
+  gu = u;
+  check_outcome ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c b/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c
new file mode 100644
index 00000000000..ed88f2cbd1a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr80898.c
@@ -0,0 +1,20 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+struct S0 {
+  int f0 : 24;
+  int f1;
+  int f74;
+} a, *c = &a;
+struct S0 fn1() {
+  struct S0 b = {4, 3};
+  return b;
+}
+
+int main() {
+  *c = fn1();
+
+  if (a.f1 != 3)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 6a8a0a4a427..f25818f4481 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -694,21 +694,9 @@ static bool constant_decl_p (tree decl)
   return VAR_P (decl) && DECL_IN_CONSTANT_POOL (decl);
 }
 
-
-/* Mark LHS of assign links out of ACCESS and its children as written to.  */
-
-static void
-process_subtree_disqualification (struct access *access)
-{
-  struct access *child;
-  for (struct assign_link *link = access->first_link; link; link = link->next)
-    link->lacc->grp_write = true;
-  for (child = access->first_child; child; child = child->next_sibling)
-    process_subtree_disqualification (child);
-}
-
 /* Remove DECL from candidates for SRA and write REASON to the dump file if
    there is one.  */
+
 static void
 disqualify_candidate (tree decl, const char *reason)
 {
@@ -723,13 +711,6 @@ disqualify_candidate (tree decl, const char *reason)
       print_generic_expr (dump_file, decl);
       fprintf (dump_file, " - %s\n", reason);
     }
-
-  struct access *access = get_first_repr_for_decl (decl);
-  while (access)
-    {
-      process_subtree_disqualification (access);
-      access = access->next_grp;
-    }
 }
 
 /* Return true iff the type contains a field or an element which does not allow
@@ -2679,6 +2660,26 @@ propagate_subaccesses_across_link (struct access *lacc, struct access *racc)
   return ret;
 }
 
+/* Beginning with ACCESS, traverse its whole access subtree and mark all
+   sub-trees as written to.  If any of them has not been marked so previously
+   and has assignment links leading from it, re-enqueue it.  */
+
+static void
+subtree_mark_written_and_enqueue (struct access *access)
+{
+  if (access->grp_write)
+    return;
+  access->grp_write = true;
+  if (access->first_link)
+    add_access_to_work_queue (access);
+
+  struct access *child;
+  for (child = access->first_child; child; child = child->next_sibling)
+    subtree_mark_written_and_enqueue (child);
+}
+
+
+
 /* Propagate all subaccesses across assignment links.  */
 
 static void
@@ -2698,7 +2699,20 @@ propagate_all_subaccesses (void)
 	  if (!bitmap_bit_p (candidate_bitmap, DECL_UID (lacc->base)))
 	    continue;
 	  lacc = lacc->group_representative;
-	  if (propagate_subaccesses_across_link (lacc, racc))
+
+	  bool reque_parents = false;
+	  if (!bitmap_bit_p (candidate_bitmap, DECL_UID (racc->base)))
+	    {
+	      if (!lacc->grp_write)
+		{
+		  subtree_mark_written_and_enqueue (lacc);
+		  reque_parents = true;
+		}
+	    }
+	  else if (propagate_subaccesses_across_link (lacc, racc))
+	    reque_parents = true;
+
+	  if (reque_parents)
 	    do
 	      {
 		if (lacc->first_link)
-- 
2.12.2

             reply	other threads:[~2017-06-01 11:39 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-01 11:39 Martin Jambor [this message]
2017-06-01 11:52 ` Richard Biener

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=20170601113923.rx3sdkm4xjik5zzt@virgil.suse.cz \
    --to=mjambor@suse.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --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).