This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][1/2] Fix PR84670


The following fixes a thinko in the assert I added.  It's true that
the value-set of ANTIC_IN shouldn't grow during iteration but there's
an exception for when the successors we intersect are not all visited
(recursively).  For example when the latch wasn't visited and thus
its ANTIC_IN is first computed without the finally antic i + 1
expression it won't get a new value for i + 1 in the next iteration
when translating over the backedge.  That only happens the second time.
It's enough if the loop exit block computes i + 1 with a different
expression (and VN was smart enough to discover the equivalence) that
the value won't be pruned by the intersection and thus its expression
reaches the backedge (once in untranslatable form and once in translatable
form).

So instead of nuking the assert completely I'm adjusting it as follows.

Bootstrap and regtest on x86_64-unknown-linux-gnu in progress.

Sorry for the inconvenience.

Richard.

2018-03-05  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/84670
	* tree-ssa-pre.c (struct bb_bitmap_sets): Add visited_with_visited_succs
	member.
	(BB_VISITED_WITH_VISITED_SUCCS): New define.
	(compute_antic): Initialize BB_VISITED_WITH_VISITED_SUCCS.
	(compute_antic_aux): Only assert the number of values in ANTIC_IN
	doesn't grow if all successors (recursively) were visited at least
	once.

	* gcc.dg/pr84670-3.c: New testcase.
	* gcc.dg/pr84670-4.c: Likewise.

Index: gcc/tree-ssa-pre.c
===================================================================
--- gcc/tree-ssa-pre.c	(revision 258124)
+++ gcc/tree-ssa-pre.c	(working copy)
@@ -484,6 +484,10 @@ typedef struct bb_bitmap_sets
   /* True if we have visited this block during ANTIC calculation.  */
   unsigned int visited : 1;
 
+  /* True if we have visited this block after all successors have been
+     visited this way.  */
+  unsigned int visited_with_visited_succs : 1;
+
   /* True when the block contains a call that might not return.  */
   unsigned int contains_may_not_return_call : 1;
 } *bb_value_sets_t;
@@ -497,6 +501,8 @@ typedef struct bb_bitmap_sets
 #define NEW_SETS(BB)	((bb_value_sets_t) ((BB)->aux))->new_sets
 #define EXPR_DIES(BB)	((bb_value_sets_t) ((BB)->aux))->expr_dies
 #define BB_VISITED(BB)	((bb_value_sets_t) ((BB)->aux))->visited
+#define BB_VISITED_WITH_VISITED_SUCCS(BB) \
+    ((bb_value_sets_t) ((BB)->aux))->visited_with_visited_succs
 #define BB_MAY_NOTRETURN(BB) ((bb_value_sets_t) ((BB)->aux))->contains_may_not_return_call
 #define BB_LIVE_VOP_ON_EXIT(BB) ((bb_value_sets_t) ((BB)->aux))->vop_on_exit
 
@@ -2032,6 +2047,8 @@ compute_antic_aux (basic_block block, bo
     {
       e = single_succ_edge (block);
       gcc_assert (BB_VISITED (e->dest));
+      BB_VISITED_WITH_VISITED_SUCCS (block)
+	= BB_VISITED_WITH_VISITED_SUCCS (e->dest);
       phi_translate_set (ANTIC_OUT, ANTIC_IN (e->dest), e);
     }
   /* If we have multiple successors, we take the intersection of all of
@@ -2042,6 +2059,7 @@ compute_antic_aux (basic_block block, bo
       size_t i;
       edge first = NULL;
 
+      BB_VISITED_WITH_VISITED_SUCCS (block) = true;
       auto_vec<edge> worklist (EDGE_COUNT (block->succs));
       FOR_EACH_EDGE (e, ei, block->succs)
 	{
@@ -2060,6 +2078,8 @@ compute_antic_aux (basic_block block, bo
 		fprintf (dump_file, "ANTIC_IN is MAX on %d->%d\n",
 			 e->src->index, e->dest->index);
 	    }
+	  BB_VISITED_WITH_VISITED_SUCCS (block)
+	    &= BB_VISITED_WITH_VISITED_SUCCS (e->dest);
 	}
 
       /* Of multiple successors we have to have visited one already
@@ -2139,7 +2159,7 @@ compute_antic_aux (basic_block block, bo
       changed = true;
       /* After the initial value set computation the value set may
          only shrink during the iteration.  */
-      if (was_visited && flag_checking)
+      if (was_visited && BB_VISITED_WITH_VISITED_SUCCS (block) && flag_checking)
 	{
 	  bitmap_iterator bi;
 	  unsigned int i;
@@ -2318,6 +2342,7 @@ compute_antic (void)
   FOR_ALL_BB_FN (block, cfun)
     {
       BB_VISITED (block) = 0;
+      BB_VISITED_WITH_VISITED_SUCCS (block) = 0;
 
       FOR_EACH_EDGE (e, ei, block->preds)
 	if (e->flags & EDGE_ABNORMAL)
@@ -2334,6 +2359,7 @@ compute_antic (void)
 
   /* At the exit block we anticipate nothing.  */
   BB_VISITED (EXIT_BLOCK_PTR_FOR_FN (cfun)) = 1;
+  BB_VISITED_WITH_VISITED_SUCCS (EXIT_BLOCK_PTR_FOR_FN (cfun)) = 1;
 
   /* For ANTIC computation we need a postorder that also guarantees that
      a block with a single successor is visited after its successor.

Index: gcc/testsuite/gcc.dg/pr84670-3.c
===================================================================
--- gcc/testsuite/gcc.dg/pr84670-3.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr84670-3.c	(working copy)
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -fno-strict-overflow" } */
+
+typedef unsigned char u8;
+typedef unsigned short u16;
+typedef unsigned int u32;
+typedef u16 acpi_rs_length;
+typedef u32 acpi_rsdesc_size;
+struct acpi_resource_source
+{
+  u16 string_length;
+  char *string_ptr;
+};
+static u16
+acpi_rs_strcpy (char *destination, char *source)
+{
+  u16 i;
+  for (i = 0; source[i]; i++)
+    {
+    }
+  return ((u16) (i + 1));
+}
+union aml_resource;
+acpi_rs_length
+acpi_rs_get_resource_source (acpi_rs_length resource_length,
+			     acpi_rs_length minimum_length,
+			     struct acpi_resource_source * resource_source,
+			     union aml_resource * aml, char *string_ptr)
+{
+  acpi_rsdesc_size total_length;
+  u8 *aml_resource_source;
+  if (total_length > (acpi_rsdesc_size) (minimum_length + 1))
+    {
+      resource_source->string_length =
+	acpi_rs_strcpy (resource_source->string_ptr,
+			((char *) (void *) (&aml_resource_source[1])));
+    }
+}
Index: gcc/testsuite/gcc.dg/pr84670-4.c
===================================================================
--- gcc/testsuite/gcc.dg/pr84670-4.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr84670-4.c	(working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fwrapv" } */
+
+char *a;
+int b(void)
+{
+  long d;
+  if (a) {
+      char c;
+      while ((c = *a) && !((unsigned)c - '0' <= 9) && c != ',' && c != '-'
+	     && c != '+')
+	++a;
+      d = (long)a;
+  }
+  if (*a)
+    return d;
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]