On 04/12/2011 11:49 AM, Lawrence Crowl wrote:
This patch is available for review at http://codereview.appspot.com/4378056

I tried to comment there, but it didn't seem to be working; looking at the side-by-side diffs didn't show any changes, and double-clicking on a line in the patch form didn't let me add a comment.

+  timevar_start (TV_RESOLVE_OVERLOAD);

Putting this in perform_overload_resolution isn't enough; only a couple of cases of overload resolution actually use it. Any function that calls tourney will also need this.

+lookup_template_class (tree d1, tree arglist, tree in_decl, tree context,
+                      int entering_scope, tsubst_flags_t complain)
+{
+  tree ret;
+  bool subtime = timevar_cond_start (TV_NAME_LOOKUP);

Let's count this as TV_INSTANTIATE_TEMPLATE instead.

@@ -17194,7 +17225,7 @@ instantiate_decl (tree d, int defer_ok,
-  timevar_push (TV_PARSE);
+  timevar_push (TV_PARSE_GLOBAL);

This too.

@@ -1911,7 +1911,7 @@ ggc_collect (void)
-  timevar_push (TV_GC);
+  timevar_start (TV_GC);

Why this change? GC time shouldn't be counted against whatever we happen to be parsing when it happens.

+DEFTIMEVAR (TV_PHASE_C_WRAPUP_CHECK  , "phase C wrapup & check")
+DEFTIMEVAR (TV_PHASE_CP_DEFERRED     , "phase C++ deferred")

Why do these need to be different timevars?

+DEFTIMEVAR (TV_PARSE_INMETH          , "parser inl. meth. body")

Is it really important to distinguish this from other functions?

-DEFTIMEVAR (TV_NAME_LOOKUP           , "name lookup")
-DEFTIMEVAR (TV_OVERLOAD              , "overload resolution")
-DEFTIMEVAR (TV_TEMPLATE_INSTANTIATION, "template instantiation")
+DEFTIMEVAR (TV_INSTANTIATE_TEMPLATE  , "instantiate template")
+DEFTIMEVAR (TV_NAME_LOOKUP           , "|name lookup")
+DEFTIMEVAR (TV_RESOLVE_OVERLOAD      , "|overload resolution")

Why these changes?

@@ -564,6 +564,8 @@ compile_file (void)
+  timevar_start (TV_PHASE_PARSING);

Why does this happen before...

+  timevar_push (TV_PARSE_GLOBAL);

...this?  I would think the bits in there should be part of _SETUP.

@@ -16760,6 +16770,7 @@ cp_parser_class_specifier (cp_parser* parser)
+      timevar_pop (TV_PARSE_STRUCT);
+      timevar_pop (TV_PARSE_STRUCT);
+      timevar_pop (TV_PARSE_STRUCT);
+  timevar_pop (TV_PARSE_STRUCT);

Why not factor this out like you did with so many functions outside the parser?

Jason

Reply via email to