diff mbox

[debug-early] reuse variable DIEs and fix their context

Message ID 54185C1B.2020200@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Sept. 16, 2014, 3:49 p.m. UTC 
On 09/12/14 10:56, Jason Merrill wrote:
> On 09/12/2014 01:48 PM, Aldy Hernandez wrote:
>> Unless I'm misunderstanding something, validate_phases() verifies that
>> the numbers add up by looking at the actual string name of the phase,
>> irregardless of if you timevar_push/pop'ed it:
>
> Yes, but why wouldn't the numbers add up?  The comment for
> timevar_push_1 says "No further elapsed time is attributed to the
> previous topmost timing variable on the stack; subsequent elapsed time
> is attributed to TIMEVAR, until it is popped or another element is
> pushed on top."

As discussed on IRC, because even though a push will stop any timer on 
the stack, the timers throttled with timevar_{start,stop} do not live on 
the stack and will continue counting even if a nested timevar_{push,pop} 
happens, thus counting time twice.

You've suggested stopping the parsing and starting the deferred timer, 
which I've implemented here.

A few things: I didn't do anything with non C/C++ languages, which 
either don't care about TV_* timers, or don't implement TV_PHASE_DEFERRED.

Also, see ?? note in c_write_global_declarations_1.  I'm not sure how 
fine grained you want things.  If I were to keep track of 
TV_PHASE_DBGINFO here properly, it would start becoming a mess of nested 
phase timers that we'd have to manually keep track of.  This smells of 
overusing global variables.  I would prefer to use a 
timevar_push(*DBGINFO*) variant for dbginfo (non "phase" timer), but I'm 
also fine leaving things with the patch as is (modulo the comment).

Up to you sir.

OK for branch?

Aldy
commit c349dc8af009ac41ba1f86c3fde9052fb5282629
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Sep 16 08:38:10 2014 -0700

    Resurrect TV_PHASE_DEFERRED.
diff mbox

Patch

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index a0a047f..89e3193 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -10306,11 +10306,16 @@  c_write_global_declarations_1 (tree globals)
     }
   while (reconsider);
 
+  /* ?? For completeness, we could stop the TV_PHASE_DEFERRED timer
+     here, and start the TV_PHASE_DBGINFO timer.  Is it worth it, or
+     would it convolute things?  */
   for (decl = globals; decl; decl = DECL_CHAIN (decl))
     {
       check_global_declaration_1 (decl);
       debug_hooks->early_global_decl (decl);
     }
+  /* ?? Similarly here. Stop TV_PHASE_DBGINFO and start
+     TV_PHASE_DEFERRED again.  */
 }
 
 /* Callback to collect a source_ref from a DECL.  */
@@ -10373,6 +10378,9 @@  c_parse_final_cleanups (void)
   if (pch_file)
     return;
 
+  timevar_stop (TV_PHASE_PARSING);
+  timevar_start (TV_PHASE_DEFERRED);
+
   /* Do the Objective-C stuff.  This is where all the Objective-C
      module stuff gets generated (symtab, class/protocol/selector
      lists etc).  */
@@ -10414,6 +10422,9 @@  c_parse_final_cleanups (void)
     c_write_global_declarations_1 (BLOCK_VARS (DECL_INITIAL (t)));
   c_write_global_declarations_1 (BLOCK_VARS (ext_block));
 
+  timevar_stop (TV_PHASE_DEFERRED);
+  timevar_start (TV_PHASE_PARSING);
+
   ext_block = NULL;
 }
 
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 28bf6e4..64cd968 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -4354,6 +4354,9 @@  c_parse_final_cleanups (void)
 
   /* FIXME - huh?  was  input_line -= 1;*/
 
+  timevar_stop (TV_PHASE_PARSING);
+  timevar_start (TV_PHASE_DEFERRED);
+
   /* We now have to write out all the stuff we put off writing out.
      These include:
 
@@ -4671,6 +4674,9 @@  c_parse_final_cleanups (void)
   /* Collect candidates for Java hidden aliases.  */
   java_hidden_aliases = collect_candidates_for_java_method_aliases ();
 
+  timevar_stop (TV_PHASE_DEFERRED);
+  timevar_start (TV_PHASE_PARSING);
+
   if (flag_vtable_verify)
     {
       vtv_recover_class_info ();
@@ -4680,6 +4686,8 @@  c_parse_final_cleanups (void)
 
   /* Issue warnings about static, but not defined, functions, etc, and
      generate initial debug information.  */
+  timevar_stop (TV_PHASE_PARSING);
+  timevar_start (TV_PHASE_DBGINFO);
   walk_namespaces (emit_debug_for_namespace, 0);
   if (vec_safe_length (pending_statics) != 0)
     {
@@ -4689,7 +4697,8 @@  c_parse_final_cleanups (void)
 				      pending_statics->length (),
 				      EMIT_DEBUG_EARLY);
     }
-
+  timevar_stop (TV_PHASE_DBGINFO);
+  timevar_start (TV_PHASE_PARSING);
 }
 
 /* Perform any post compilation-proper cleanups for the C++ front-end.
diff --git a/gcc/langhooks.c b/gcc/langhooks.c
index dfc3ab6..49658d7 100644
--- a/gcc/langhooks.c
+++ b/gcc/langhooks.c
@@ -300,6 +300,8 @@  global_decl_processing_and_early_debug (void)
   tree globals, decl, *vec;
   int len, i;
 
+  timevar_stop (TV_PHASE_PARSING);
+  timevar_start (TV_PHASE_DEFERRED);
   /* Really define vars that have had only a tentative definition.
      Really output inline functions that must actually be callable
      and have not been output so far.  */
@@ -316,9 +318,13 @@  global_decl_processing_and_early_debug (void)
 
   wrapup_global_declarations (vec, len);
   check_global_declarations (vec, len);
+  timevar_stop (TV_PHASE_DEFERRED);
 
+  timevar_start (TV_PHASE_DBGINFO);
   emit_debug_global_declarations (vec, len, EMIT_DEBUG_EARLY);
+  timevar_stop (TV_PHASE_DBGINFO);
 
+  timevar_start (TV_PHASE_PARSING);
   free (vec);
 }
 
diff --git a/gcc/timevar.def b/gcc/timevar.def
index d9d95de..15fe7f3 100644
--- a/gcc/timevar.def
+++ b/gcc/timevar.def
@@ -40,6 +40,7 @@  DEFTIMEVAR (TV_TOTAL                 , "total time")
    validate_phases).  */
 DEFTIMEVAR (TV_PHASE_SETUP           , "phase setup")
 DEFTIMEVAR (TV_PHASE_PARSING         , "phase parsing")
+DEFTIMEVAR (TV_PHASE_DEFERRED        , "phase lang. deferred")
 DEFTIMEVAR (TV_PHASE_LATE_PARSING_CLEANUPS, "phase late parsing cleanups")
 DEFTIMEVAR (TV_PHASE_OPT_GEN         , "phase opt and generate")
 DEFTIMEVAR (TV_PHASE_DBGINFO         , "phase debug info")