On Wed, Jul 18, 2012 at 7:18 PM, Steven Bosscher <stevenb....@gmail.com> wrote: > On Wed, Jul 18, 2012 at 10:08 AM, Steven Bosscher <stevenb....@gmail.com> > wrote: >> On Wed, Jul 18, 2012 at 2:24 AM, H.J. Lu <hjl.to...@gmail.com> wrote: >>> This caused: >>> >>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54008 >> >> Yes, they failed in my testing, too. I must have been blind to overlook >> them... >> >> I received some comments in private about the "new look" of the dumps, >> that I'll be addressing with a patch later today. I'll fix these two >> test cases with that patch also. > > Hello, > > The attached patch further consolidates RTL and GIMPLE CFG dumping. It > also fixes a couple of formatting issues, and it fixes the two broken > test case patterns from yesterday's patch. > > I also ran into a bug in the GIMPLE CFG dumping hooks while working on > value profiling cleanups (that have been oh so long on my TODO > list...). It is necessary to flush the pretty-printer buffer to the > dump file before printing directly to the pretty-printer stream with > dump_histograms_for_stmt. For good measure, I replaced pp_newfile with > pp_flush in places where that seemed to be more appropriate, and added > comments to the functions that do _not_ flush the buffer (this can > happen e.g. because a user of those functions wants to print > additional information before the newline that pp_flush adds).
Hmm, pp_flush looks like a lot more expensive compared to pp_newline for example in @@ -74,7 +74,7 @@ maybe_init_pretty_print (FILE *file) static void newline_and_indent (pretty_printer *buffer, int spc) { - pp_newline (buffer); + pp_flush (buffer); INDENT (spc); } And I'm pretty sure that newline_and_indent callers that after it directly dump to the stream should be fixed instead. In fact, constant flushing will just make things slow (yes, it's only dumping ...). Of course @@ -2256,7 +2261,7 @@ gimple_dump_bb_buff (pretty_printer *buf INDENT (curr_indent); dump_gimple_stmt (buffer, stmt, curr_indent, flags); - pp_newline (buffer); + pp_flush (buffer); dump_histograms_for_stmt (cfun, buffer->buffer->stream, stmt); } shows one of this "bogus" usages - indeed a pp_flush is required before dumping directly to the stream. So, can you please leave out the newline_and_indent change and the following ones? The caller that constructed the pretty-printer is responsible for flushing before destroying it again. @@ -128,7 +130,7 @@ dump_gimple_seq (pretty_printer *buffer, INDENT (spc); dump_gimple_stmt (buffer, gs, spc, flags); if (!gsi_one_before_end_p (i)) - pp_newline (buffer); + pp_flush (buffer); } } @@ -895,6 +897,7 @@ dump_gimple_bind (pretty_printer *buffer pp_character (buffer, '>'); else pp_character (buffer, '}'); + pp_flush (buffer); } @@ -2134,7 +2139,7 @@ dump_phi_nodes (pretty_printer *buffer, INDENT (indent); pp_string (buffer, "# "); dump_gimple_phi (buffer, phi, indent, flags); - pp_newline (buffer); + pp_flush (buffer); } } } @@ -2194,7 +2199,7 @@ dump_implicit_edges (pretty_printer *buf pp_string (buffer, "else"); newline_and_indent (buffer, indent + 2); pp_cfg_jump (buffer, false_edge->dest); - pp_newline (buffer); + pp_flush (buffer); return; } @@ -2225,7 +2230,7 @@ dump_implicit_edges (pretty_printer *buf } pp_cfg_jump (buffer, e->dest); - pp_newline (buffer); + pp_flush (buffer); } } Ok with that changes. Thanks, Richard. > > Bootstrapped&tested on powerpc64-unknown-linux-gnu. This patch doesn't > affect Graphite, so perhaps it won't break this time :-) > OK for trunk? > > Ciao! > Steven