diff mbox series

[jakub@redhat.com:,Re:,[PATCH] Fix -fcompare-debug due to DEBUG_BEGIN_STMTs (PR debug/83419)]

Message ID 20171218193533.GF2320@tucnak
State New
Headers show
Series [jakub@redhat.com:,Re:,[PATCH] Fix -fcompare-debug due to DEBUG_BEGIN_STMTs (PR debug/83419)] | expand

Commit Message

Jakub Jelinek Dec. 18, 2017, 7:35 p.m. UTC 
On Mon, Dec 18, 2017 at 02:35:07PM +0100, Jakub Jelinek wrote:
> On Mon, Dec 18, 2017 at 02:17:18PM +0100, Jakub Jelinek wrote:
> > One option would be to deal with that in pop_stmt_list and the C++ caller
> > thereof, where we right now have:
> > 
> >       /* If the statement list contained exactly one statement, then
> >          extract it immediately.  */
> >       if (tsi_one_before_end_p (i))
> >         {
> >           u = tsi_stmt (i);
> >           tsi_delink (&i);
> >           free_stmt_list (t);
> >           t = u;
> >         }
> > 
> > This part would need a MAY_HAVE_DEBUG_MARKER_STMTS variant that would
> > check for the case where there are just DEBUG_BEGIN_STMT markers + one other
> > stmt (+ maybe some further DEBUG_BEGIN_STMT markers) and nothing else.
> > If that one other stmt doesn't have TREE_SIDE_EFFECTS, clear
> > TREE_SIDE_EFFECTS on the whole statement list to get the same
> > TREE_SIDE_EFFECTS from the returned expression as without -g.
> 
> Like this (the cp_parser_omp_for_loop_init function would need similar
> changes).  Except while it works for this new testcase and some of the
> testcases I've listed, it still doesn't work for others.
> 
> Alex, I'm giving up here.

Forgot to attach patch for reference:



	Jakub
diff mbox series

Patch

--- gcc/testsuite/gcc.dg/pr83419.c.jj	2017-12-18 10:08:24.214911480 +0100
+++ gcc/testsuite/gcc.dg/pr83419.c	2017-12-18 10:08:24.214911480 +0100
@@ -0,0 +1,16 @@ 
+/* PR debug/83419 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcompare-debug" } */
+
+int a, b;
+void foo (int, ...);
+
+void
+bar (void)
+{
+  if (a || 1 == b)
+    foo (1);
+  else
+    0;
+  foo (1, 0);
+}
--- gcc/c-family/c-semantics.c.jj	2017-12-12 09:48:11.000000000 +0100
+++ gcc/c-family/c-semantics.c	2017-12-18 14:30:06.389129165 +0100
@@ -23,6 +23,7 @@  along with GCC; see the file COPYING3.
 #include "coretypes.h"
 #include "c-common.h"
 #include "tree-iterator.h"
+#include "options.h"
 
 /* Create an empty statement tree rooted at T.  */
 
@@ -76,25 +77,49 @@  pop_stmt_list (tree t)
 	  free_stmt_list (t);
 	  t = u;
 	}
-      /* If the statement list contained a debug begin stmt and a
-	 statement list, move the debug begin stmt into the statement
-	 list and return it.  */
-      else if (!tsi_end_p (i)
-	       && TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
+      else if (MAY_HAVE_DEBUG_MARKER_STMTS)
 	{
-	  u = tsi_stmt (i);
-	  tsi_next (&i);
-	  if (tsi_one_before_end_p (i)
-	      && TREE_CODE (tsi_stmt (i)) == STATEMENT_LIST)
+	  while (!tsi_end_p (i)
+		 && TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
+	    tsi_next (&i);
+	  if (!tsi_end_p (i) && !TREE_SIDE_EFFECTS (tsi_stmt (i)))
+	    {
+	      tsi_next (&i);
+	      while (!tsi_end_p (i)
+		     && TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
+		tsi_next (&i);
+	      /* With -g0, if the above tsi_one_before_end_p case would
+		 hit, we'd return the single stmt without TREE_SIDE_EFFECTS
+		 set.  We need to return a STATEMENT_LIST instead, ensure it
+		 has a TREE_SIDE_EFFECTS flag clear.  */
+	      if (tsi_end_p (i))
+		{
+		  TREE_SIDE_EFFECTS (t) = 0;
+		  return t;
+		}
+	    }
+
+	  i = tsi_start (t);
+	  /* As an optimization, if the statement list contained a debug begin
+	     stmt and a statement list, move the debug begin stmt into the
+	     statement list and return it.  */
+	  if (!tsi_end_p (i)
+	      && TREE_CODE (tsi_stmt (i)) == DEBUG_BEGIN_STMT)
 	    {
-	      tree l = tsi_stmt (i);
-	      tsi_prev (&i);
-	      tsi_delink (&i);
-	      tsi_delink (&i);
-	      i = tsi_start (l);
-	      free_stmt_list (t);
-	      t = l;
-	      tsi_link_before (&i, u, TSI_SAME_STMT);
+	      u = tsi_stmt (i);
+	      tsi_next (&i);
+	      if (tsi_one_before_end_p (i)
+		  && TREE_CODE (tsi_stmt (i)) == STATEMENT_LIST)
+		{
+		  tree l = tsi_stmt (i);
+		  tsi_prev (&i);
+		  tsi_delink (&i);
+		  tsi_delink (&i);
+		  i = tsi_start (l);
+		  free_stmt_list (t);
+		  t = l;
+		  tsi_link_before (&i, u, TSI_SAME_STMT);
+		}
 	    }
 	}
     }
--- gcc/gimplify.c.jj	2017-12-14 21:11:40.000000000 +0100
+++ gcc/gimplify.c	2017-12-18 10:17:07.922556324 +0100
@@ -1763,6 +1763,9 @@  gimplify_statement_list (tree *expr_p, g
 
   tree_stmt_iterator i = tsi_start (*expr_p);
 
+  /* No need to recompute TREE_SIDE_EFFECTS on removal, we are going to
+     delink everything.  */
+  TREE_SIDE_EFFECTS (*expr_p) = 0;
   while (!tsi_end_p (i))
     {
       gimplify_stmt (tsi_stmt_ptr (i), pre_p);
--- gcc/tree-iterator.c.jj	2017-12-12 09:48:26.000000000 +0100
+++ gcc/tree-iterator.c	2017-12-18 13:44:02.330719440 +0100
@@ -23,6 +23,7 @@  along with GCC; see the file COPYING3.
 #include "coretypes.h"
 #include "tree.h"
 #include "tree-iterator.h"
+#include "options.h"
 
 
 /* This is a cache of STATEMENT_LIST nodes.  We create and destroy them
@@ -41,7 +42,10 @@  alloc_stmt_list (void)
       TREE_SET_CODE (list, STATEMENT_LIST);
     }
   else
-    list = make_node (STATEMENT_LIST);
+    {
+      list = make_node (STATEMENT_LIST);
+      TREE_SIDE_EFFECTS (list) = 0;
+    }
   TREE_TYPE (list) = void_type_node;
   return list;
 }
@@ -127,6 +131,8 @@  tsi_link_before (tree_stmt_iterator *i,
 	  gcc_assert (head == tail);
 	  return;
 	}
+      if (TREE_SIDE_EFFECTS (t))
+	TREE_SIDE_EFFECTS (i->container) = 1;
     }
   else
     {
@@ -135,11 +141,10 @@  tsi_link_before (tree_stmt_iterator *i,
       head->next = NULL;
       head->stmt = t;
       tail = head;
+      if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
+	TREE_SIDE_EFFECTS (i->container) = 1;
     }
 
-  if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
-    TREE_SIDE_EFFECTS (i->container) = 1;
-
   cur = i->ptr;
 
   /* Link it into the list.  */
@@ -157,9 +162,9 @@  tsi_link_before (tree_stmt_iterator *i,
     {
       head->prev = STATEMENT_LIST_TAIL (i->container);
       if (head->prev)
-       head->prev->next = head;
+	head->prev->next = head;
       else
-       STATEMENT_LIST_HEAD (i->container) = head;
+	STATEMENT_LIST_HEAD (i->container) = head;
       STATEMENT_LIST_TAIL (i->container) = tail;
     }
 
@@ -204,6 +209,9 @@  tsi_link_after (tree_stmt_iterator *i, t
 	  gcc_assert (head == tail);
 	  return;
 	}
+
+      if (TREE_SIDE_EFFECTS (t))
+	TREE_SIDE_EFFECTS (i->container) = 1;
     }
   else
     {
@@ -212,10 +220,10 @@  tsi_link_after (tree_stmt_iterator *i, t
       head->next = NULL;
       head->stmt = t;
       tail = head;
-    }
 
-  if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
-    TREE_SIDE_EFFECTS (i->container) = 1;
+      if (TREE_CODE (t) != DEBUG_BEGIN_STMT)
+	TREE_SIDE_EFFECTS (i->container) = 1;
+    }
 
   cur = i->ptr;
 
@@ -275,10 +283,30 @@  tsi_delink (tree_stmt_iterator *i)
   else
     STATEMENT_LIST_TAIL (i->container) = prev;
 
-  if (!next && !prev)
-    TREE_SIDE_EFFECTS (i->container) = 0;
-
   i->ptr = next;
+
+  if (!MAY_HAVE_DEBUG_MARKER_STMTS)
+    TREE_SIDE_EFFECTS (i->container) = 0;
+  else if (TREE_SIDE_EFFECTS (i->container))
+    {
+      while (next || prev)
+	{
+	  if (next)
+	    {
+	      if (TREE_CODE (next->stmt) != DEBUG_BEGIN_STMT)
+		break;
+	      next = next->next;
+	    }
+	  if (prev)
+	    {
+	      if (TREE_CODE (prev->stmt) != DEBUG_BEGIN_STMT)
+		break;
+	      prev = prev->prev;
+	    }
+	}
+      if (next == NULL && prev == NULL)
+	TREE_SIDE_EFFECTS (i->container) = 0;
+    }
 }
 
 /* Return the first expression in a sequence of COMPOUND_EXPRs, or in