public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] tree-optimization/107323 - loop distribution partition ordering issue
@ 2022-10-21  9:16 Richard Biener
  0 siblings, 0 replies; only message in thread
From: Richard Biener @ 2022-10-21  9:16 UTC (permalink / raw)
  To: gcc-patches

The following reverts part of the PR94125 fix which causes us to
use a bogus partition ordering after applying versioning for
alias to the testcase in PR107323.  Instead PR94125 is fixed by
appropriately considering to be merged SCCs when skipping edges
we want to ignore because of the alias versioning.

Bootstrapped and tested on x86_64-unknown-linux-gnu,
on the 10 branch where reverting the part of PR94125 reproduces
the original issue and that's fixed by the adjustment,
on the 12 branch where the PR107323 bug can be reproduced, and
on trunk.

Pushed to trunk and gcc-12 sofar.

	PR tree-optimization/107323
	* tree-loop-distribution.cc (pg_unmark_merged_alias_ddrs):
	New function.
	(loop_distribution::break_alias_scc_partitions): Revert
	postorder save/restore from the PR94125 fix.  Instead
	make sure to not ignore edges from SCCs we are going to
	merge.

	* gcc.dg/tree-ssa/pr107323.c: New testcase.
---
 gcc/testsuite/gcc.dg/tree-ssa/pr107323.c | 28 +++++++++++++
 gcc/tree-loop-distribution.cc            | 50 +++++++++++++++++-------
 2 files changed, 64 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr107323.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr107323.c b/gcc/testsuite/gcc.dg/tree-ssa/pr107323.c
new file mode 100644
index 00000000000..1204b6e36d5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr107323.c
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-vectorize" } */
+
+int A[4];
+int B[4];
+
+static const char *__attribute__((noipa)) foo()
+{
+  return "1";
+}
+
+int main()
+{
+  const char *s = foo();
+
+  A[0] = 1000;
+  for(int i = 1; i < 4; ++i) {
+      B[i] = 0;
+      A[i] = 0;
+      if(s[0])
+	B[i] = 1;
+      A[i] = A[i - 1];
+  }
+
+  if (A[3] != 1000)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/tree-loop-distribution.cc b/gcc/tree-loop-distribution.cc
index e1948fb452a..ed3dd73e1a9 100644
--- a/gcc/tree-loop-distribution.cc
+++ b/gcc/tree-loop-distribution.cc
@@ -2201,8 +2201,6 @@ struct pg_edge_callback_data
   bitmap sccs_to_merge;
   /* Array constains component information for all vertices.  */
   int *vertices_component;
-  /* Array constains postorder information for all vertices.  */
-  int *vertices_post;
   /* Vector to record all data dependence relations which are needed
      to break strong connected components by runtime alias checks.  */
   vec<ddr_p> *alias_ddrs;
@@ -2452,6 +2450,33 @@ pg_collect_alias_ddrs (struct graph *g, struct graph_edge *e, void *data)
     cbdata->alias_ddrs->safe_splice (edata->alias_ddrs);
 }
 
+/* Callback function for traversing edge E.  DATA is private
+   callback data.  */
+
+static void
+pg_unmark_merged_alias_ddrs (struct graph *, struct graph_edge *e, void *data)
+{
+  int i, j, component;
+  struct pg_edge_callback_data *cbdata;
+  struct pg_edata *edata = (struct pg_edata *) e->data;
+
+  if (edata == NULL || edata->alias_ddrs.length () == 0)
+    return;
+
+  cbdata = (struct pg_edge_callback_data *) data;
+  i = e->src;
+  j = e->dest;
+  component = cbdata->vertices_component[i];
+  /* Make sure to not skip vertices inside SCCs we are going to merge.  */
+  if (component == cbdata->vertices_component[j]
+      && bitmap_bit_p (cbdata->sccs_to_merge, component))
+    {
+      edata->alias_ddrs.release ();
+      delete edata;
+      e->data = NULL;
+    }
+}
+
 /* This is the main function breaking strong conected components in
    PARTITIONS giving reduced depdendence graph RDG.  Store data dependence
    relations for runtime alias check in ALIAS_DDRS.  */
@@ -2511,7 +2536,6 @@ loop_distribution::break_alias_scc_partitions (struct graph *rdg,
       cbdata.sccs_to_merge = sccs_to_merge;
       cbdata.alias_ddrs = alias_ddrs;
       cbdata.vertices_component = XNEWVEC (int, pg->n_vertices);
-      cbdata.vertices_post = XNEWVEC (int, pg->n_vertices);
       /* Record the component information which will be corrupted by next
 	 graph scc finding call.  */
       for (i = 0; i < pg->n_vertices; ++i)
@@ -2520,17 +2544,18 @@ loop_distribution::break_alias_scc_partitions (struct graph *rdg,
       /* Collect data dependences for runtime alias checks to break SCCs.  */
       if (bitmap_count_bits (sccs_to_merge) != (unsigned) num_sccs)
 	{
-	  /* Record the postorder information which will be corrupted by next
-	     graph SCC finding call.  */
-	  for (i = 0; i < pg->n_vertices; ++i)
-	    cbdata.vertices_post[i] = pg->vertices[i].post;
+	  /* For SCCs we want to merge clear all alias_ddrs for edges
+	     inside the component.  */
+	  for_each_edge (pg, pg_unmark_merged_alias_ddrs, &cbdata);
 
 	  /* Run SCC finding algorithm again, with alias dependence edges
 	     skipped.  This is to topologically sort partitions according to
 	     compilation time known dependence.  Note the topological order
 	     is stored in the form of pg's post order number.  */
 	  num_sccs_no_alias = graphds_scc (pg, NULL, pg_skip_alias_edge);
-	  gcc_assert (partitions->length () == (unsigned) num_sccs_no_alias);
+	  /* We cannot assert partitions->length () == num_sccs_no_alias
+	     since we are not ignoring alias edges in cycles we are
+	     going to merge.  That's required to compute correct postorder.  */
 	  /* With topological order, we can construct two subgraphs L and R.
 	     L contains edge <x, y> where x < y in terms of post order, while
 	     R contains edge <x, y> where x > y.  Edges for compilation time
@@ -2565,16 +2590,14 @@ loop_distribution::break_alias_scc_partitions (struct graph *rdg,
 	      first->type = PTYPE_SEQUENTIAL;
 	    }
 	}
-      /* Restore the postorder information if it's corrupted in finding SCC
-	 with alias dependence edges skipped.  If reduction partition's SCC is
-	 broken by runtime alias checks, we force a negative post order to it
-	 making sure it will be scheduled in the last.  */
+      /* If reduction partition's SCC is broken by runtime alias checks,
+	 we force a negative post order to it making sure it will be scheduled
+	 in the last.  */
       if (num_sccs_no_alias > 0)
 	{
 	  j = -1;
 	  for (i = 0; i < pg->n_vertices; ++i)
 	    {
-	      pg->vertices[i].post = cbdata.vertices_post[i];
 	      struct pg_vdata *data = (struct pg_vdata *)pg->vertices[i].data;
 	      if (data->partition && partition_reduction_p (data->partition))
 		{
@@ -2587,7 +2610,6 @@ loop_distribution::break_alias_scc_partitions (struct graph *rdg,
 	}
 
       free (cbdata.vertices_component);
-      free (cbdata.vertices_post);
     }
 
   sort_partitions_by_post_order (pg, partitions);
-- 
2.35.3

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-10-21  9:16 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-21  9:16 [PATCH] tree-optimization/107323 - loop distribution partition ordering issue Richard Biener

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).