public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] More cleanups for CFG dumping
@ 2012-07-18 17:18 Steven Bosscher
  2012-07-19  9:10 ` Richard Guenther
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Bosscher @ 2012-07-18 17:18 UTC (permalink / raw)
  To: GCC Patches; +Cc: Richard Guenther, H.J. Lu

[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]

On Wed, Jul 18, 2012 at 10:08 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Wed, Jul 18, 2012 at 2:24 AM, H.J. Lu <hjl.tools@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).

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

[-- Attachment #2: cleanup_cfg_dump_2.diff --]
[-- Type: application/octet-stream, Size: 27041 bytes --]

gcc/
	* basic-block.h (BB_FLAGS_TO_PRESERVE): New define.
	(brief_dump_cfg): Update prototype to take flags argument.
	(check_bb_profile): Remove prototype.
	* tracer.c (tracer): Update brief_dump_cfg calls.
	* cfghooks.c (dump_bb): Do not pass TDF_COMMENT to dump_bb_info.
	Call dump_bb_info before and after the cfghook dump_bb.  Terminate
	the dump with a newline.
	(dump_flow_info): Do not call check_bb_profile.
	* cfg.c (clear_bb_flags): Update using BB_FLAGS_TO_PRESERVE.
	(check_bb_profile): Make static.  Take indent and flags arguments.
	(dump_bb_info): Always dump loop depth.  With TDF_DETAILS, call
	check_bb_profile.  Print one edge per line.
	(brief_dump_cfg): Take a flags argument, and filter out
	TDF_COMMENT and TDF_DETAILS.
	* gimple-pretty-print.c (newline_and_indent): Use pp_flush
	instead of pp_newline.
	(dump_gimple_seq): Likewise for all but the last stmt.
	(dump_gimple_bind): Flush after pretty printing.
	(dump_gimple_phi): Document that the user must call pp_flush.
	(dump_gimple_stmt): Likewise.
	(dump_gimple_bb_header): Do not use dump_bb_info here, it is
	already called from dump_bb.  Idem for check_bb_profile.
	(dump_gimple_bb_footer): Likewise.
	(dump_phi_nodes): Call pp_flush after dump_gimple_phi.
	(dump_implicit_edges): Call pp_flush after pp_cfg_jump.
	(gimple_dump_bb_buff): Call pp_flush after dump_gimple_stmt to
	avoid broken dumps for statement histograms.
	(gimple_dump_bb): Handle ENTRY_BLOCK and EXIT_BLOCK.  Do
	not call pp_flush here, the buffer should be empty.
	* sched-rgn.c (debug_region): Pass TDF_BLOCKS to dump_bb.
	* sched-vis.c (debug_bb_slim): Likewise.
	* tree-cfg.c (remove_bb): Pass dump_flags to dump_bb.
	(gimple_debug_bb): Pass TDF_BLOCKS to dump_bb.
	(gimple_dump_cfg): Do brief_dump_cfg with TDF_COMMENT.
	(dump_function_to_file): Do not call check_bb_profile on
	ENTRY_BLOCK and EXIT_BLOCK, check_bb_profile doesn't handle them.
	Use dump_bb instead of gimple_dump_bb.
	(print_loops_bb): Use dump_bb instead of gimple_dump_bb.
	* passes.c (execute_function_dump): Always call print_rtl_with_bb
	for RTL dumps.
	* cfgrtl.c (print_rtl_with_bb): Handle printing without an up-to-date
	CFG.  With TDF_BLOCKS and TDF_DETAILS, do DF dumps at the top and bottom
	of each basic block.

testsuite/
	* gcc.dg/tree-prof/update-loopch.c: Look for counts on the dumps of
	the basic block and check loop depth.
	* gcc.dg/tree-ssa/pr18133-1.c: Dump details, not blocks.  Update
	matching patterns and comments.
	* gcc.dg/tree-ssa/20031021-1.c: Fix check patterns.
	* gcc.dg/tree-ssa/vector-2.c: Likewise.

Index: basic-block.h
===================================================================
--- basic-block.h	(revision 189600)
+++ basic-block.h	(working copy)
@@ -203,9 +203,15 @@ enum cfg_bb_flags
 };
 #undef DEF_BASIC_BLOCK_FLAG
 
-/* Bit mask for all edge flags.  */
+/* Bit mask for all basic block flags.  */
 #define BB_ALL_FLAGS		((LAST_CFG_BB_FLAG - 1) * 2 - 1)
 
+/* Bit mask for all basic block flags that must be preserved.  These are
+   the bit masks that are *not* cleared by clear_bb_flags.  */
+#define BB_FLAGS_TO_PRESERVE					\
+  (BB_DISABLE_SCHEDULE | BB_RTL | BB_NON_LOCAL_GOTO_TARGET	\
+   | BB_HOT_PARTITION | BB_COLD_PARTITION)
+
 /* Dummy bitmask for convenience in the hot/cold partitioning code.  */
 #define BB_UNPARTITIONED	0
 
@@ -395,7 +401,7 @@ extern basic_block create_basic_block_st
 extern void clear_bb_flags (void);
 extern void dump_bb_info (FILE *, basic_block, int, int, bool, bool);
 extern void dump_edge_info (FILE *, edge, int, int);
-extern void brief_dump_cfg (FILE *);
+extern void brief_dump_cfg (FILE *, int);
 extern void clear_edges (void);
 extern void scale_bbs_frequencies_int (basic_block *, int, int, int);
 extern void scale_bbs_frequencies_gcov_type (basic_block *, int, gcov_type,
@@ -816,7 +822,6 @@ unsigned bb_dom_dfs_out (enum cdi_direct
 extern edge try_redirect_by_replacing_jump (edge, basic_block, bool);
 extern void break_superblocks (void);
 extern void relink_block_chain (bool);
-extern void check_bb_profile (basic_block, FILE *);
 extern void update_bb_profile_for_threading (basic_block, int, gcov_type, edge);
 extern void init_rtl_bb_info (basic_block);
 
Index: tracer.c
===================================================================
--- tracer.c	(revision 189600)
+++ tracer.c	(working copy)
@@ -374,7 +374,7 @@ tracer (void)
 
   mark_dfs_back_edges ();
   if (dump_file)
-    brief_dump_cfg (dump_file);
+    brief_dump_cfg (dump_file, dump_flags);
 
   /* Trace formation is done on the fly inside tail_duplicate */
   changed = tail_duplicate ();
@@ -382,7 +382,7 @@ tracer (void)
     free_dominance_info (CDI_DOMINATORS);
 
   if (dump_file)
-    brief_dump_cfg (dump_file);
+    brief_dump_cfg (dump_file, dump_flags);
 
   return changed ? TODO_cleanup_cfg : 0;
 }
Index: cfghooks.c
===================================================================
--- cfghooks.c	(revision 189600)
+++ cfghooks.c	(working copy)
@@ -271,9 +271,11 @@ verify_flow_info (void)
 void
 dump_bb (FILE *outf, basic_block bb, int indent, int flags)
 {
-  dump_bb_info (outf, bb, indent, flags | TDF_COMMENT, true, true);
+  dump_bb_info (outf, bb, indent, flags, true, false);
   if (cfg_hooks->dump_bb)
     cfg_hooks->dump_bb (outf, bb, indent, flags);
+  dump_bb_info (outf, bb, indent, flags, false, true);
+  fputc ('\n', outf);
 }
 
 /* Dump the complete CFG to FILE.  FLAGS are the TDF_* flags in dumpfile.h.  */
@@ -284,10 +286,7 @@ dump_flow_info (FILE *file, int flags)
 
   fprintf (file, "\n%d basic blocks, %d edges.\n", n_basic_blocks, n_edges);
   FOR_ALL_BB (bb)
-    {
-      dump_bb (file, bb, 0, flags);
-      check_bb_profile (bb, file);
-    }
+    dump_bb (file, bb, 0, flags);
 
   putc ('\n', file);
 }
Index: cfg.c
===================================================================
--- cfg.c	(revision 189600)
+++ cfg.c	(working copy)
@@ -382,16 +382,14 @@ redirect_edge_pred (edge e, basic_block 
   connect_src (e);
 }
 
-/* Clear all basic block flags, with the exception of partitioning and
-   setjmp_target.  */
+/* Clear all basic block flags that do not have to be preserved.  */
 void
 clear_bb_flags (void)
 {
   basic_block bb;
 
   FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR, NULL, next_bb)
-    bb->flags = (BB_PARTITION (bb)
-		 | (bb->flags & (BB_DISABLE_SCHEDULE + BB_RTL + BB_NON_LOCAL_GOTO_TARGET)));
+    bb->flags &= BB_FLAGS_TO_PRESERVE;
 }
 \f
 /* Check the consistency of profile information.  We can't do that
@@ -399,13 +397,16 @@ clear_bb_flags (void)
    solved graphs, later eliminating of conditionals or roundoff errors.
    It is still practical to have them reported for debugging of simple
    testcases.  */
-void
-check_bb_profile (basic_block bb, FILE * file)
+static void
+check_bb_profile (basic_block bb, FILE * file, int indent, int flags)
 {
   edge e;
   int sum = 0;
   gcov_type lsum;
   edge_iterator ei;
+  char *s_indent = (char *) alloca ((size_t) indent + 1);
+  memset ((void *) s_indent, ' ', (size_t) indent);
+  s_indent[indent] = '\0';
 
   if (profile_status == PROFILE_ABSENT)
     return;
@@ -415,14 +416,16 @@ check_bb_profile (basic_block bb, FILE *
       FOR_EACH_EDGE (e, ei, bb->succs)
 	sum += e->probability;
       if (EDGE_COUNT (bb->succs) && abs (sum - REG_BR_PROB_BASE) > 100)
-	fprintf (file, "Invalid sum of outgoing probabilities %.1f%%\n",
+	fprintf (file, "%s%sInvalid sum of outgoing probabilities %.1f%%\n",
+		 (flags & TDF_COMMENT) ? ";; " : "", s_indent,
 		 sum * 100.0 / REG_BR_PROB_BASE);
       lsum = 0;
       FOR_EACH_EDGE (e, ei, bb->succs)
 	lsum += e->count;
       if (EDGE_COUNT (bb->succs)
 	  && (lsum - bb->count > 100 || lsum - bb->count < -100))
-	fprintf (file, "Invalid sum of outgoing counts %i, should be %i\n",
+	fprintf (file, "%s%sInvalid sum of outgoing counts %i, should be %i\n",
+		 (flags & TDF_COMMENT) ? ";; " : "", s_indent,
 		 (int) lsum, (int) bb->count);
     }
   if (bb != ENTRY_BLOCK_PTR)
@@ -432,13 +435,15 @@ check_bb_profile (basic_block bb, FILE *
 	sum += EDGE_FREQUENCY (e);
       if (abs (sum - bb->frequency) > 100)
 	fprintf (file,
-		 "Invalid sum of incoming frequencies %i, should be %i\n",
+		 "%s%sInvalid sum of incoming frequencies %i, should be %i\n",
+		 (flags & TDF_COMMENT) ? ";; " : "", s_indent,
 		 sum, bb->frequency);
       lsum = 0;
       FOR_EACH_EDGE (e, ei, bb->preds)
 	lsum += e->count;
       if (lsum - bb->count > 100 || lsum - bb->count < -100)
-	fprintf (file, "Invalid sum of incoming counts %i, should be %i\n",
+	fprintf (file, "%s%sInvalid sum of incoming counts %i, should be %i\n",
+		 (flags & TDF_COMMENT) ? ";; " : "", s_indent,
 		 (int) lsum, (int) bb->count);
     }
 }
@@ -679,6 +684,7 @@ dump_bb_info (FILE *outf, basic_block bb
 #undef DEF_BASIC_BLOCK_FLAG
     };
   const unsigned n_bitnames = sizeof (bb_bitnames) / sizeof (char *);
+  bool first;
   char *s_indent = (char *) alloca ((size_t) indent + 1);
   memset ((void *) s_indent, ' ', (size_t) indent);
   s_indent[indent] = '\0';
@@ -691,11 +697,12 @@ dump_bb_info (FILE *outf, basic_block bb
 
       if (flags & TDF_COMMENT)
 	fputs (";; ", outf);
-      fprintf (outf, "%sbasic block %d", s_indent, bb->index);
+      fprintf (outf, "%sbasic block %d, loop depth %d",
+	       s_indent, bb->index, bb->loop_depth);
       if (flags & TDF_DETAILS)
 	{
-	  fprintf (outf, ", loop depth %d, count " HOST_WIDEST_INT_PRINT_DEC,
-		   bb->loop_depth, (HOST_WIDEST_INT) bb->count);
+	  fprintf (outf, ", count " HOST_WIDEST_INT_PRINT_DEC,
+		   (HOST_WIDEST_INT) bb->count);
 	  fprintf (outf, ", freq %i", bb->frequency);
 	  if (maybe_hot_bb_p (bb))
 	    fputs (", maybe hot", outf);
@@ -703,11 +710,11 @@ dump_bb_info (FILE *outf, basic_block bb
 	    fputs (", probably never executed", outf);
 	}
       fputc ('\n', outf);
+      if (TDF_DETAILS)
+	check_bb_profile (bb, outf, indent, flags);
 
       if (flags & TDF_DETAILS)
 	{
-	  bool first = true;
-
 	  if (flags & TDF_COMMENT)
 	    fputs (";; ", outf);
 	  fprintf (outf, "%s prev block ", s_indent);
@@ -722,6 +729,7 @@ dump_bb_info (FILE *outf, basic_block bb
 	    fprintf (outf, "(nil)");
 
 	  fputs (", flags:", outf);
+	  first = true;
 	  for (i = 0; i < n_bitnames; i++)
 	    if (bb->flags & (1 << i))
 	      {
@@ -734,15 +742,25 @@ dump_bb_info (FILE *outf, basic_block bb
 	      }
 	  if (!first)
 	    fputc (')', outf);
+	  fputc ('\n', outf);
 	}
-      fputc ('\n', outf);
 
       if (flags & TDF_COMMENT)
 	fputs (";; ", outf);
       fprintf (outf, "%s pred:      ", s_indent);
+      first = true;
       FOR_EACH_EDGE (e, ei, bb->preds)
-	dump_edge_info (outf, e, flags, 0);
-      fputc ('\n', outf);
+	{
+	  if (! first)
+	    {
+	      if (flags & TDF_COMMENT)
+		fputs (";; ", outf);
+	      fprintf (outf, "%s            ", s_indent);
+	    }
+	  first = false;
+	  dump_edge_info (outf, e, flags, 0);
+	  fputc ('\n', outf);
+	}
     }
 
   if (do_footer)
@@ -750,22 +768,34 @@ dump_bb_info (FILE *outf, basic_block bb
       if (flags & TDF_COMMENT)
 	fputs (";; ", outf);
       fprintf (outf, "%s succ:      ", s_indent);
+      first = true;
       FOR_EACH_EDGE (e, ei, bb->succs)
-	dump_edge_info (outf, e, flags, 1);
-      fputs ("\n\n", outf);
+        {
+	  if (! first)
+	    {
+	      if (flags & TDF_COMMENT)
+		fputs (";; ", outf);
+	      fprintf (outf, "%s            ", s_indent);
+	    }
+	  first = false;
+	  dump_edge_info (outf, e, flags, 1);
+	  fputc ('\n', outf);
+	}
     }
 }
 
 /* Dumps a brief description of cfg to FILE.  */
 
 void
-brief_dump_cfg (FILE *file)
+brief_dump_cfg (FILE *file, int flags)
 {
   basic_block bb;
 
   FOR_EACH_BB (bb)
     {
-      dump_bb_info (file, bb, 0, 0, true, true);
+      dump_bb_info (file, bb, 0,
+		    flags & (TDF_COMMENT | TDF_DETAILS),
+		    true, true);
     }
 }
 
Index: gimple-pretty-print.c
===================================================================
--- gimple-pretty-print.c	(revision 189600)
+++ gimple-pretty-print.c	(working copy)
@@ -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);
 }
 
@@ -115,7 +115,9 @@ print_gimple_expr (FILE *file, gimple g,
 
 
 /* Print the GIMPLE sequence SEQ on BUFFER using SPC indentantion
-   spaces and FLAGS as in dump_gimple_stmt.  */
+   spaces and FLAGS as in dump_gimple_stmt.
+   The caller is responsible for calling pp_flush on BUFFER to finalize
+   the pretty printer.  */
 
 static void
 dump_gimple_seq (pretty_printer *buffer, gimple_seq seq, int spc, int flags)
@@ -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);
 }
 
 
@@ -1578,8 +1581,9 @@ dump_gimple_asm (pretty_printer *buffer,
 }
 
 
-/* Dump a PHI node PHI.  BUFFER, SPC and FLAGS are as in
-   dump_gimple_stmt.  */
+/* Dump a PHI node PHI.  BUFFER, SPC and FLAGS are as in dump_gimple_stmt.
+   The caller is responsible for calling pp_flush on BUFFER to finalize
+   pretty printer.  */
 
 static void
 dump_gimple_phi (pretty_printer *buffer, gimple phi, int spc, int flags)
@@ -1843,7 +1847,8 @@ dump_gimple_mem_ops (pretty_printer *buf
 
 /* Dump the gimple statement GS on the pretty printer BUFFER, SPC
    spaces of indent.  FLAGS specifies details to show in the dump (see
-   TDF_* in dumpfile.h).  */
+   TDF_* in dumpfile.h).  The caller is responsible for calling
+   pp_flush on BUFFER to finalize the pretty printer.  */
 
 void
 dump_gimple_stmt (pretty_printer *buffer, gimple gs, int spc, int flags)
@@ -2067,8 +2072,6 @@ dump_gimple_bb_header (FILE *outf, basic
 {
   if (flags & TDF_BLOCKS)
     {
-      dump_bb_info (outf, bb, indent, flags, true, false);
-
       if (flags & TDF_LINENO)
 	{
 	  gimple_stmt_iterator gsi;
@@ -2103,8 +2106,6 @@ dump_gimple_bb_header (FILE *outf, basic
 	  fprintf (outf, "%s<bb %d>:\n", s_indent, bb->index);
 	}
     }
-  if (cfun)
-    check_bb_profile (bb, outf);
 }
 
 
@@ -2112,9 +2113,13 @@ dump_gimple_bb_header (FILE *outf, basic
    spaces.  */
 
 static void
-dump_gimple_bb_footer (FILE *outf, basic_block bb, int indent, int flags)
+dump_gimple_bb_footer (FILE *outf ATTRIBUTE_UNUSED,
+		       basic_block bb ATTRIBUTE_UNUSED,
+		       int indent ATTRIBUTE_UNUSED,
+		       int flags ATTRIBUTE_UNUSED)
 {
-  dump_bb_info (outf, bb, indent, flags, false, true);
+  /* There is currently no GIMPLE-specific basic block info to dump.  */
+  return;
 }
 
 
@@ -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);
     }
 }
 
@@ -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);
     }
 
@@ -2271,8 +2276,10 @@ void
 gimple_dump_bb (FILE *file, basic_block bb, int indent, int flags)
 {
   dump_gimple_bb_header (file, bb, indent, flags);
-  maybe_init_pretty_print (file);
-  gimple_dump_bb_buff (&buffer, bb, indent, flags);
-  pp_flush (&buffer);
+  if (bb->index >= NUM_FIXED_BLOCKS)
+    {
+      maybe_init_pretty_print (file);
+      gimple_dump_bb_buff (&buffer, bb, indent, flags);
+    }
   dump_gimple_bb_footer (file, bb, indent, flags);
 }
Index: sched-rgn.c
===================================================================
--- sched-rgn.c	(revision 189600)
+++ sched-rgn.c	(working copy)
@@ -397,7 +397,7 @@ debug_region (int rgn)
   for (bb = 0; bb < rgn_table[rgn].rgn_nr_blocks; bb++)
     {
       dump_bb (stderr, BASIC_BLOCK (rgn_bb_table[current_blocks + bb]),
-	       0, TDF_SLIM);
+	       0, TDF_SLIM | TDF_BLOCKS);
       fprintf (stderr, "\n");
     }
 
Index: sched-vis.c
===================================================================
--- sched-vis.c	(revision 189600)
+++ sched-vis.c	(working copy)
@@ -810,7 +810,7 @@ extern void debug_bb_slim (basic_block);
 DEBUG_FUNCTION void
 debug_bb_slim (basic_block bb)
 {
-  dump_bb (stderr, bb, 0, TDF_SLIM);
+  dump_bb (stderr, bb, 0, TDF_SLIM | TDF_BLOCKS);
 }
 
 extern void debug_bb_n_slim (int);
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 189600)
+++ tree-cfg.c	(working copy)
@@ -1844,7 +1844,7 @@ remove_bb (basic_block bb)
       fprintf (dump_file, "Removing basic block %d\n", bb->index);
       if (dump_flags & TDF_DETAILS)
 	{
-	  dump_bb (dump_file, bb, 0, 0);
+	  dump_bb (dump_file, bb, 0, dump_flags);
 	  fprintf (dump_file, "\n");
 	}
     }
@@ -2063,7 +2063,7 @@ find_case_label_for_value (gimple switch
 void
 gimple_debug_bb (basic_block bb)
 {
-  dump_bb (stderr, bb, 0, TDF_VOPS|TDF_MEMSYMS);
+  dump_bb (stderr, bb, 0, TDF_VOPS|TDF_MEMSYMS|TDF_BLOCKS);
 }
 
 
@@ -2103,7 +2103,7 @@ gimple_dump_cfg (FILE *file, int flags)
       fprintf (file, ";; \n%d basic blocks, %d edges, last basic block %d.\n\n",
 	       n_basic_blocks, n_edges, last_basic_block);
 
-      brief_dump_cfg (file);
+      brief_dump_cfg (file, flags | TDF_COMMENT);
       fprintf (file, "\n");
     }
 
@@ -6687,7 +6687,6 @@ dump_function_to_file (tree fn, FILE *fi
   if (cfun && cfun->decl == fn && cfun->cfg && basic_block_info)
     {
       /* If the CFG has been built, emit a CFG-based dump.  */
-      check_bb_profile (ENTRY_BLOCK_PTR, file);
       if (!ignore_topmost_bind)
 	fprintf (file, "{\n");
 
@@ -6695,10 +6694,9 @@ dump_function_to_file (tree fn, FILE *fi
 	fprintf (file, "\n");
 
       FOR_EACH_BB (bb)
-	gimple_dump_bb (file, bb, 2, flags);
+	dump_bb (file, bb, 2, flags | TDF_COMMENT);
 
       fprintf (file, "}\n");
-      check_bb_profile (EXIT_BLOCK_PTR, file);
     }
   else if (DECL_SAVED_TREE (fn) == NULL)
     {
@@ -6821,7 +6819,7 @@ print_loops_bb (FILE *file, basic_block 
   if (verbosity >= 3)
     {
       fprintf (file, "%s  {\n", s_indent);
-      gimple_dump_bb (file, bb, indent + 4, TDF_VOPS|TDF_MEMSYMS);
+      dump_bb (file, bb, indent + 4, TDF_VOPS|TDF_MEMSYMS);
       fprintf (file, "%s  }\n", s_indent);
     }
 }
Index: passes.c
===================================================================
--- passes.c	(revision 189600)
+++ passes.c	(working copy)
@@ -1725,11 +1725,7 @@ execute_function_dump (void *data ATTRIB
         dump_function_to_file (current_function_decl, dump_file, dump_flags);
       else
 	{
-	  if ((cfun->curr_properties & PROP_cfg)
-	      && (dump_flags & TDF_BLOCKS))
-	    print_rtl_with_bb (dump_file, get_insns (), dump_flags);
-          else
-	    print_rtl (dump_file, get_insns ());
+	  print_rtl_with_bb (dump_file, get_insns (), dump_flags);
 
 	  if ((cfun->curr_properties & PROP_cfg)
 	      && graph_dump_format != no_graph
Index: cfgrtl.c
===================================================================
--- cfgrtl.c	(revision 189600)
+++ cfgrtl.c	(working copy)
@@ -1875,8 +1875,9 @@ rtl_dump_bb (FILE *outf, basic_block bb,
 
 }
 \f
-/* Like print_rtl, but also print out live information for the start of each
-   basic block.  FLAGS are the flags documented in dumpfile.h.  */
+/* Like dump_function_to_file, but for RTL.  Print out dataflow information
+   for the start of each basic block.  FLAGS are the TDF_* masks documented
+   in dumpfile.h.  */
 
 void
 print_rtl_with_bb (FILE *outf, const_rtx rtx_first, int flags)
@@ -1891,52 +1892,74 @@ print_rtl_with_bb (FILE *outf, const_rtx
       basic_block *start = XCNEWVEC (basic_block, max_uid);
       basic_block *end = XCNEWVEC (basic_block, max_uid);
       enum bb_state *in_bb_p = XCNEWVEC (enum bb_state, max_uid);
-
       basic_block bb;
 
+      /* After freeing the CFG, we still have BLOCK_FOR_INSN set on most
+	 insns, but the CFG is not maintained so the basic block info
+	 is not reliable.  Therefore it's omitted from the dumps.  */
+      if (! (cfun->curr_properties & PROP_cfg))
+        flags &= ~TDF_BLOCKS;
+
       if (df)
 	df_dump_start (outf);
 
-      FOR_EACH_BB_REVERSE (bb)
+      if (flags & TDF_BLOCKS)
 	{
-	  rtx x;
-
-	  start[INSN_UID (BB_HEAD (bb))] = bb;
-	  end[INSN_UID (BB_END (bb))] = bb;
-	  for (x = BB_HEAD (bb); x != NULL_RTX; x = NEXT_INSN (x))
+	  FOR_EACH_BB_REVERSE (bb)
 	    {
-	      enum bb_state state = IN_MULTIPLE_BB;
+	      rtx x;
+
+	      start[INSN_UID (BB_HEAD (bb))] = bb;
+	      end[INSN_UID (BB_END (bb))] = bb;
+	      for (x = BB_HEAD (bb); x != NULL_RTX; x = NEXT_INSN (x))
+		{
+		  enum bb_state state = IN_MULTIPLE_BB;
 
-	      if (in_bb_p[INSN_UID (x)] == NOT_IN_BB)
-		state = IN_ONE_BB;
-	      in_bb_p[INSN_UID (x)] = state;
+		  if (in_bb_p[INSN_UID (x)] == NOT_IN_BB)
+		    state = IN_ONE_BB;
+		  in_bb_p[INSN_UID (x)] = state;
 
-	      if (x == BB_END (bb))
-		break;
+		  if (x == BB_END (bb))
+		    break;
+		}
 	    }
 	}
 
       for (tmp_rtx = rtx_first; NULL != tmp_rtx; tmp_rtx = NEXT_INSN (tmp_rtx))
 	{
-	  bb = start[INSN_UID (tmp_rtx)];
-	  if (bb != NULL)
-	    dump_bb_info (outf, bb, 0, dump_flags | TDF_COMMENT, true, false);
-
-	  if (in_bb_p[INSN_UID (tmp_rtx)] == NOT_IN_BB
-	      && !NOTE_P (tmp_rtx)
-	      && !BARRIER_P (tmp_rtx))
-	    fprintf (outf, ";; Insn is not within a basic block\n");
-	  else if (in_bb_p[INSN_UID (tmp_rtx)] == IN_MULTIPLE_BB)
-	    fprintf (outf, ";; Insn is in multiple basic blocks\n");
+	  if (flags & TDF_BLOCKS)
+	    {
+	      bb = start[INSN_UID (tmp_rtx)];
+	      if (bb != NULL)
+		{
+		  dump_bb_info (outf, bb, 0, dump_flags | TDF_COMMENT, true, false);
+		  if (df && (flags & TDF_DETAILS))
+		    df_dump_top (bb, outf);
+		}
+
+	      if (in_bb_p[INSN_UID (tmp_rtx)] == NOT_IN_BB
+		  && !NOTE_P (tmp_rtx)
+		  && !BARRIER_P (tmp_rtx))
+		fprintf (outf, ";; Insn is not within a basic block\n");
+	      else if (in_bb_p[INSN_UID (tmp_rtx)] == IN_MULTIPLE_BB)
+		fprintf (outf, ";; Insn is in multiple basic blocks\n");
+	    }
 
 	  if (! (flags & TDF_SLIM))
 	    print_rtl_single (outf, tmp_rtx);
 	  else
 	    dump_insn_slim (outf, tmp_rtx);
 
-	  bb = end[INSN_UID (tmp_rtx)];
-	  if (bb != NULL)
-	    dump_bb_info (outf, bb, 0, dump_flags | TDF_COMMENT, false, true);
+	  if (flags & TDF_BLOCKS)
+	    {
+	      bb = end[INSN_UID (tmp_rtx)];
+	      if (bb != NULL)
+		{
+		  dump_bb_info (outf, bb, 0, dump_flags | TDF_COMMENT, false, true);
+		  if (df && (flags & TDF_DETAILS))
+		    df_dump_bottom (bb, outf);
+		}
+	    }
 
 	  putc ('\n', outf);
 	}
Index: testsuite/gcc.dg/tree-prof/update-loopch.c
===================================================================
--- testsuite/gcc.dg/tree-prof/update-loopch.c	(revision 189600)
+++ testsuite/gcc.dg/tree-prof/update-loopch.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-options "-O2 -fdump-ipa-profile-blocks -fdump-tree-optimized-blocks" } */
+/* { dg-options "-O2 -fdump-ipa-profile-details -fdump-tree-optimized-details" } */
 int max = 33333;
 int a[8];
 int
@@ -14,8 +14,8 @@ main ()
 /* Loop header copying will peel away the initial conditional, so the loop body
    is once reached directly from entry point of function, rest via loopback
    edge.  */
-/* { dg-final-use { scan-ipa-dump "count:33333" "profile"} } */
-/* { dg-final-use { scan-tree-dump "count:33332" "optimized"} } */
+/* { dg-final-use { scan-ipa-dump "loop depth 1, count 33334" "profile"} } */
+/* { dg-final-use { scan-tree-dump "loop depth 1, count 33332" "optimized"} } */
 /* { dg-final-use { scan-tree-dump-not "Invalid sum" "optimized"} } */
 /* { dg-final-use { cleanup-ipa-dump "profile" } } */
 /* { dg-final-use { cleanup-tree-dump "optimized" } } */
Index: testsuite/gcc.dg/tree-ssa/pr18133-1.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/pr18133-1.c	(revision 189600)
+++ testsuite/gcc.dg/tree-ssa/pr18133-1.c	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O1 -fdump-tree-optimized-blocks" } */
+/* { dg-options "-O1 -fdump-tree-optimized-details" } */
 
 void foo (void)
 {
@@ -12,17 +12,17 @@ return;
 
 /* The goto &L0 should have been optimized away during CFG
    cleanups.  */
-/* { dg-final { scan-tree-dump-times "goto &L0" 0 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "goto &L0" "optimized" } } */
 
 /* There should not be any abnormal edges as DOM removed the
    computed goto.  */
 
-/* { dg-final { scan-tree-dump-times "ab" 0 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "ABNORMAL" "optimized" } } */
 
 /* And verify that we have fixed the fallthru flag as well. 
    After DOM we will have two fallthru edges (e->0, 0->1),
-   but in the dump files we mention the 0->1 two times.  So
-   scan for 3 instances of "fallthru".  */
+   but in the dump files we mention the 2->3 two times.  So
+   scan for 3 instances of "FALLTHRU".  */
 
-/* { dg-final { scan-tree-dump-times "fallthru" 3 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "FALLTHRU" 3 "optimized" } } */
 /* { dg-final { cleanup-tree-dump "optimized" } } */
Index: testsuite/gcc.dg/tree-ssa/20031021-1.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/20031021-1.c	(revision 189600)
+++ testsuite/gcc.dg/tree-ssa/20031021-1.c	(working copy)
@@ -17,5 +17,5 @@ int main()
 }
 
 /* We should only store to a.i, not load from it.  */
-/* { dg-final { scan-tree-dump-times "a.i" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "= a.i" "optimized" } } */
 /* { dg-final { cleanup-tree-dump "optimized" } } */
Index: testsuite/gcc.dg/tree-ssa/vector-2.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/vector-2.c	(revision 189600)
+++ testsuite/gcc.dg/tree-ssa/vector-2.c	(working copy)
@@ -17,7 +17,7 @@ float f(vector float a, int b, vector fl
 }
 
 /* We should be able to optimize this to just "return 0.0;" */
-/* { dg-final { scan-tree-dump-times "BIT_FIELD_REF" 0 "optimized"} } */
-/* { dg-final { scan-tree-dump-times "0.0" 1 "optimized"} } */
+/* { dg-final { scan-tree-dump-not "BIT_FIELD_REF" "optimized"} } */
+/* { dg-final { scan-tree-dump-times "return 0.0" 1 "optimized"} } */
 
 /* { dg-final { cleanup-tree-dump "optimized" } } */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] More cleanups for CFG dumping
  2012-07-18 17:18 [patch] More cleanups for CFG dumping Steven Bosscher
@ 2012-07-19  9:10 ` Richard Guenther
  2012-07-20  9:03   ` Steven Bosscher
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Guenther @ 2012-07-19  9:10 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches, H.J. Lu

On Wed, Jul 18, 2012 at 7:18 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Wed, Jul 18, 2012 at 10:08 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>> On Wed, Jul 18, 2012 at 2:24 AM, H.J. Lu <hjl.tools@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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] More cleanups for CFG dumping
  2012-07-19  9:10 ` Richard Guenther
@ 2012-07-20  9:03   ` Steven Bosscher
  2012-07-20  9:29     ` Richard Guenther
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Bosscher @ 2012-07-20  9:03 UTC (permalink / raw)
  To: Richard Guenther; +Cc: GCC Patches, H.J. Lu

On Thu, Jul 19, 2012 at 11:09 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> 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 ...).

Right, it's only dumping. I'm surprised one would care about its
performance. And the patch actually helps in the debugger also: if for
some reason a piece of invalid gimple is encountered then at least the
part that was OK is still dumped. But oh well :-)

I will need this additional patch to avoid test suite failures:

Index: pretty-print.c
===================================================================
--- pretty-print.c      (revision 189705)
+++ pretty-print.c      (working copy)
@@ -759,6 +759,7 @@ void
 pp_base_newline (pretty_printer *pp)
 {
   obstack_1grow (pp->buffer->obstack, '\n');
+  pp_needs_newline (pp) = false;
   pp->buffer->line_length = 0;
 }

I suppose that's OK?

Ciao!
Steven

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] More cleanups for CFG dumping
  2012-07-20  9:03   ` Steven Bosscher
@ 2012-07-20  9:29     ` Richard Guenther
  2012-07-23 13:10       ` Steven Bosscher
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Guenther @ 2012-07-20  9:29 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches, H.J. Lu

On Fri, Jul 20, 2012 at 11:02 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Thu, Jul 19, 2012 at 11:09 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> 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 ...).
>
> Right, it's only dumping. I'm surprised one would care about its
> performance. And the patch actually helps in the debugger also: if for
> some reason a piece of invalid gimple is encountered then at least the
> part that was OK is still dumped. But oh well :-)
>
> I will need this additional patch to avoid test suite failures:
>
> Index: pretty-print.c
> ===================================================================
> --- pretty-print.c      (revision 189705)
> +++ pretty-print.c      (working copy)
> @@ -759,6 +759,7 @@ void
>  pp_base_newline (pretty_printer *pp)
>  {
>    obstack_1grow (pp->buffer->obstack, '\n');
> +  pp_needs_newline (pp) = false;
>    pp->buffer->line_length = 0;
>  }
>
> I suppose that's OK?

Yes.

Thanks,
Richard.

> Ciao!
> Steven

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] More cleanups for CFG dumping
  2012-07-20  9:29     ` Richard Guenther
@ 2012-07-23 13:10       ` Steven Bosscher
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Bosscher @ 2012-07-23 13:10 UTC (permalink / raw)
  To: Richard Guenther; +Cc: GCC Patches

On Fri, Jul 20, 2012 at 11:28 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Fri, Jul 20, 2012 at 11:02 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>> On Thu, Jul 19, 2012 at 11:09 AM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> 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 ...).
>>
>> Right, it's only dumping. I'm surprised one would care about its
>> performance. And the patch actually helps in the debugger also: if for
>> some reason a piece of invalid gimple is encountered then at least the
>> part that was OK is still dumped. But oh well :-)
>>
>> I will need this additional patch to avoid test suite failures:
>>
>> Index: pretty-print.c
>> ===================================================================
>> --- pretty-print.c      (revision 189705)
>> +++ pretty-print.c      (working copy)
>> @@ -759,6 +759,7 @@ void
>>  pp_base_newline (pretty_printer *pp)
>>  {
>>    obstack_1grow (pp->buffer->obstack, '\n');
>> +  pp_needs_newline (pp) = false;
>>    pp->buffer->line_length = 0;
>>  }
>>
>> I suppose that's OK?
>
> Yes.

I also need this one:

-------------- 8< -----------
Index: gimple-pretty-print.c
--- gimple-pretty-print.c       (revision 189778)
+++ gimple-pretty-print.c       (working copy)
@@ -2265,6 +2265,7 @@ gimple_dump_bb_buff (pretty_printer *buf
     }

   dump_implicit_edges (buffer, bb, indent, flags);
+  pp_flush (buffer);
 }


-------------- 8< -----------

or you get nonsense dumps like:
...
;;   basic block 4, loop depth 0
;;    pred:       2
  <bb 4>:
  if (node_3(D) != node_20)
;;    succ:       6
;;                5

;;   basic block 5, loop depth 0
;;    pred:       6
;;                4
  <bb 5>:
    goto <bb 6>;
  else
    goto <bb 5>;

...

Will commit as obvious.

Ciao!
Steven

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-07-23 13:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-18 17:18 [patch] More cleanups for CFG dumping Steven Bosscher
2012-07-19  9:10 ` Richard Guenther
2012-07-20  9:03   ` Steven Bosscher
2012-07-20  9:29     ` Richard Guenther
2012-07-23 13:10       ` Steven Bosscher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).