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]

Re: [pph] Namespaces, step 1. Trace formatting. (issue4433054)



http://codereview.appspot.com/4433054/diff/1/gcc/cp/pph-streamer.c File gcc/cp/pph-streamer.c (right):

http://codereview.appspot.com/4433054/diff/1/gcc/cp/pph-streamer.c#newcode144
gcc/cp/pph-streamer.c:144: return;
+  if ((type == PPH_TRACE_TREE || type == PPH_TRACE_CHAIN)
+      && !data && flag_pph_tracer <= 3)
+    return;

Line up the predicates vertically.

http://codereview.appspot.com/4433054/diff/1/gcc/cp/pph-streamer.c#newcode172
gcc/cp/pph-streamer.c:172: fprintf (pph_logfile, ", code=%s",
tree_code_name[TREE_CODE (t)]);
   case PPH_TRACE_REF:
+      {
+	const_tree t = (const_tree) data;
+	if (t)
+	  {
+	    print_generic_expr (pph_logfile, CONST_CAST (union tree_node *,
t),
+				0);
+	    fprintf (pph_logfile, ", code=%s", tree_code_name[TREE_CODE (t)]);


But how are we going to tell if this is a REF instead of a tree? The output seems identical to the PPH_TRACE_TREE case.

http://codereview.appspot.com/4433054/diff/1/gcc/cp/pph-streamer.h
File gcc/cp/pph-streamer.h (right):

http://codereview.appspot.com/4433054/diff/1/gcc/cp/pph-streamer.h#newcode149
gcc/cp/pph-streamer.h:149: }
pph_output_tree_lst (pph_stream *stream, tree t, bool ref_p)
+{
+  if (flag_pph_tracer >= 2)
+    pph_stream_trace_tree (stream, t, ref_p);
+  lto_output_tree (stream->ob, t, ref_p);
+}

I don't really like all this code duplication.  Wouldn't it be better if
instead of having pph_output_tree_aux and pph_output_tree_lst, we added
another argument to pph_output_tree?  The argument would be an enum and
we could have a default 'DONT_CARE' value.

http://codereview.appspot.com/4433054/diff/1/gcc/cp/pph-streamer.h#newcode298
gcc/cp/pph-streamer.h:298: pph_stream_trace_tree (stream, t, false); /*
FIXME pph: always false? */
@@ -285,7 +295,7 @@ pph_input_tree (pph_stream *stream)
 {
   tree t = lto_input_tree (stream->ib, stream->data_in);
   if (flag_pph_tracer >= 4)
-    pph_stream_trace_tree (stream, t);
+    pph_stream_trace_tree (stream, t, false); /* FIXME pph: always
false?

Yes, on input we can't tell if we read a reference or a real tree.  We
could, but not at this level.  That's inside the actual LTO streaming
code.

http://codereview.appspot.com/4433054/


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