[PR 80898] Propagate grp_write from disqualified SRA candidates

[Martin Jambor <mjambor at suse dot cz>]

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