Patchwork PATCH RFA: Speed up warnings about jumps over declarations

login
register
mail settings
Submitter Ian Taylor
Date Jan. 25, 2011, 9:27 p.m.
Message ID <mcrd3nk1xor.fsf@google.com>
Download mbox | patch
Permalink /patch/80408/
State New
Headers show

Comments

Ian Taylor - Jan. 25, 2011, 9:27 p.m.
This patch speeds up warnings about jumps over declarations, by keeping
track of whether there are any decls that we might need to warn about.
If there are no decls that might require warnings, we can skip the loops
checking all the decls.

For the test case in PR 26854, when compiling without optimization, this
speeds up overall compilation about 5.1% wall time and 5.5% user time.
According to -ftime-report, parsing drops from 50% of total compilation
time to 45%.

This is a small improvement for an unusual test case.  However, even
more ordinary code the change should typically be a miniscule
improvement in compilation time, as warnings about a goto statement
crossing an initializer are much rarer than declarations.

Bootstrapped and ran C testsuite on x86_64-unknown-linux-gnu.

OK for mainline?

Ian


2011-01-25  Ian Lance Taylor  <iant@google.com>

	PR tree-optimization/26854
	* c-decl.c (struct c_scope): Add field has_jump_unsafe_decl.
	(decl_jump_unsafe): Move higher in file, with no other change.
	(bind): Set has_jump_unsafe_decl if appropriate.
	(update_label_decls): Test has_jump_unsafe_decl to avoid loop.
	(check_earlier_gotos): Likewise.
	(c_check_switch_jump_warnings): Likewise.
Joseph S. Myers - Jan. 26, 2011, 1:13 a.m.
On Tue, 25 Jan 2011, Ian Lance Taylor wrote:

> 2011-01-25  Ian Lance Taylor  <iant@google.com>
> 
> 	PR tree-optimization/26854
> 	* c-decl.c (struct c_scope): Add field has_jump_unsafe_decl.
> 	(decl_jump_unsafe): Move higher in file, with no other change.
> 	(bind): Set has_jump_unsafe_decl if appropriate.
> 	(update_label_decls): Test has_jump_unsafe_decl to avoid loop.
> 	(check_earlier_gotos): Likewise.
> 	(c_check_switch_jump_warnings): Likewise.

OK.

Patch

Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(revision 169248)
+++ gcc/c-decl.c	(working copy)
@@ -404,6 +404,13 @@  struct GTY((chain_next ("%h.outer"))) c_
      up searching for labels when popping scopes, particularly since
      labels are normally only found at function scope.  */
   BOOL_BITFIELD has_label_bindings : 1;
+
+  /* True if we should issue a warning if a goto statement crosses any
+     of the bindings.  We still need to check the list of bindings to
+     find the specific ones we need to warn about.  This is true if
+     decl_jump_unsafe would return true for any of the bindings.  This
+     is used to avoid looping over all the bindings unnecessarily.  */
+  BOOL_BITFIELD has_jump_unsafe_decl : 1;
 };
 
 /* The scope currently in effect.  */
@@ -554,6 +561,31 @@  add_stmt (tree t)
   return t;
 }
 
+/* Return true if we will want to say something if a goto statement
+   crosses DECL.  */
+
+static bool
+decl_jump_unsafe (tree decl)
+{
+  if (decl == error_mark_node || TREE_TYPE (decl) == error_mark_node)
+    return false;
+
+  /* Always warn about crossing variably modified types.  */
+  if ((TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == TYPE_DECL)
+      && variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
+    return true;
+
+  /* Otherwise, only warn if -Wgoto-misses-init and this is an
+     initialized automatic decl.  */
+  if (warn_jump_misses_init
+      && TREE_CODE (decl) == VAR_DECL
+      && !TREE_STATIC (decl)
+      && DECL_INITIAL (decl) != NULL_TREE)
+    return true;
+
+  return false;
+}
+
 
 void
 c_print_identifier (FILE *file, tree node, int indent)
@@ -602,6 +634,9 @@  bind (tree name, tree decl, struct c_sco
   b->prev = scope->bindings;
   scope->bindings = b;
 
+  if (decl_jump_unsafe (decl))
+    scope->has_jump_unsafe_decl = 1;
+
   if (!name)
     return;
 
@@ -758,31 +793,6 @@  set_spot_bindings (struct c_spot_binding
   p->left_stmt_expr = false;
 }
 
-/* Return true if we will want to say something if a goto statement
-   crosses DECL.  */
-
-static bool
-decl_jump_unsafe (tree decl)
-{
-  if (decl == error_mark_node || TREE_TYPE (decl) == error_mark_node)
-    return false;
-
-  /* Always warn about crossing variably modified types.  */
-  if ((TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == TYPE_DECL)
-      && variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
-    return true;
-
-  /* Otherwise, only warn if -Wgoto-misses-init and this is an
-     initialized automatic decl.  */
-  if (warn_jump_misses_init
-      && TREE_CODE (decl) == VAR_DECL
-      && !TREE_STATIC (decl)
-      && DECL_INITIAL (decl) != NULL_TREE)
-    return true;
-
-  return false;
-}
-
 /* Update spot bindings P as we pop out of SCOPE.  Return true if we
    should push decls for a label.  */
 
@@ -969,6 +979,7 @@  update_label_decls (struct c_scope *scop
 	    {
 	      struct c_label_vars *label_vars;
 	      struct c_binding *b1;
+	      bool hjud;
 	      unsigned int ix;
 	      struct c_goto_bindings *g;
 
@@ -977,18 +988,26 @@  update_label_decls (struct c_scope *scop
 	      label_vars = b->u.label;
 
 	      b1 = label_vars->label_bindings.bindings_in_scope;
+	      if (label_vars->label_bindings.scope == NULL)
+		hjud = false;
+	      else
+		hjud = label_vars->label_bindings.scope->has_jump_unsafe_decl;
 	      if (update_spot_bindings (scope, &label_vars->label_bindings))
 		{
 		  /* This label is defined in this scope.  */
-		  for (; b1 != NULL;  b1 = b1->prev)
+		  if (hjud)
 		    {
-		      /* A goto from later in the function to this
-			 label will never see the initialization of
-			 B1, if any.  Save it to issue a warning if
-			 needed.  */
-		      if (decl_jump_unsafe (b1->decl))
-			VEC_safe_push (tree, gc, label_vars->decls_in_scope,
-				       b1->decl);
+		      for (; b1 != NULL; b1 = b1->prev)
+			{
+			  /* A goto from later in the function to this
+			     label will never see the initialization
+			     of B1, if any.  Save it to issue a
+			     warning if needed.  */
+			  if (decl_jump_unsafe (b1->decl))
+			    VEC_safe_push (tree, gc,
+					   label_vars->decls_in_scope,
+					   b1->decl);
+			}
 		    }
 		}
 
@@ -3165,12 +3184,15 @@  check_earlier_gotos (tree label, struct 
       /* We have a goto to this label.  The goto is going forward.  In
 	 g->scope, the goto is going to skip any binding which was
 	 defined after g->bindings_in_scope.  */
-      for (b = g->goto_bindings.scope->bindings;
-	   b != g->goto_bindings.bindings_in_scope;
-	   b = b->prev)
+      if (g->goto_bindings.scope->has_jump_unsafe_decl)
 	{
-	  if (decl_jump_unsafe (b->decl))
-	    warn_about_goto (g->loc, label, b->decl);
+	  for (b = g->goto_bindings.scope->bindings;
+	       b != g->goto_bindings.bindings_in_scope;
+	       b = b->prev)
+	    {
+	      if (decl_jump_unsafe (b->decl))
+		warn_about_goto (g->loc, label, b->decl);
+	    }
 	}
 
       /* We also need to warn about decls defined in any scopes
@@ -3180,14 +3202,17 @@  check_earlier_gotos (tree label, struct 
 	   scope = scope->outer)
 	{
 	  gcc_assert (scope != NULL);
-	  if (scope == label_vars->label_bindings.scope)
-	    b = label_vars->label_bindings.bindings_in_scope;
-	  else
-	    b = scope->bindings;
-	  for (; b != NULL; b = b->prev)
+	  if (scope->has_jump_unsafe_decl)
 	    {
-	      if (decl_jump_unsafe (b->decl))
-		warn_about_goto (g->loc, label, b->decl);
+	      if (scope == label_vars->label_bindings.scope)
+		b = label_vars->label_bindings.bindings_in_scope;
+	      else
+		b = scope->bindings;
+	      for (; b != NULL; b = b->prev)
+		{
+		  if (decl_jump_unsafe (b->decl))
+		    warn_about_goto (g->loc, label, b->decl);
+		}
 	    }
 	}
 
@@ -3303,6 +3328,10 @@  c_check_switch_jump_warnings (struct c_s
       struct c_binding *b;
 
       gcc_assert (scope != NULL);
+
+      if (!scope->has_jump_unsafe_decl)
+	continue;
+
       for (b = scope->bindings; b != NULL; b = b->prev)
 	{
 	  if (decl_jump_unsafe (b->decl))