Patchwork Change sel_print from vararg macro to out of line function

login
register
mail settings
Submitter Jakub Jelinek
Date June 14, 2010, 9:39 a.m.
Message ID <20100614093916.GF7811@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/55514/
State New
Headers show

Comments

Jakub Jelinek - June 14, 2010, 9:39 a.m.
Hi!

This patch saves almost 19KB of .text (roughly 21% of sel-sched{,-dump}.c
.text) and removes varargs macro while doing that (which don't have to
be supported by pre-ISOC99 bootstrap compilers).

Not that I care much about such bootstrap compilers, but having large
inline expansion of this macro in almost 150 places when it is in
code not enabled by default is strange (and sched_dump_to_dot_p
is only true when invoking sel_debug_cfg{,_1} from the debugger).

Invoking sel_debug_cfg{,_1} from the debugger ICEs for me though even
before the patch, I probably don't know where is the right spot to
invoke this function, so I haven't tested sel_print when creating the
*.dot file, can one of the sel-sched authors please test that?

2010-06-14  Jakub Jelinek  <jakub@redhat.com>

	PR bootstrap/44426
	* sel-sched-dump.h (sel_prepare_string_for_dot_label): Remove
	prototype.
	(sel_print_to_dot): Remove macro.
	(sel_print): Likewise.  New prototype.
	* sel-sched-dump.c (sel_prepare_string_for_dot_label): Make static.
	(sel_print): New function.


	Jakub
Andrey Belevantsev - June 15, 2010, 8:32 a.m.
Hi Jakub,

On 14.06.2010 13:39, Jakub Jelinek wrote:
> Hi!
>
> This patch saves almost 19KB of .text (roughly 21% of sel-sched{,-dump}.c
> .text) and removes varargs macro while doing that (which don't have to
> be supported by pre-ISOC99 bootstrap compilers).
>
> Not that I care much about such bootstrap compilers, but having large
> inline expansion of this macro in almost 150 places when it is in
> code not enabled by default is strange (and sched_dump_to_dot_p
> is only true when invoking sel_debug_cfg{,_1} from the debugger).
>
> Invoking sel_debug_cfg{,_1} from the debugger ICEs for me though even
> before the patch, I probably don't know where is the right spot to
> invoke this function, so I haven't tested sel_print when creating the
> *.dot file, can one of the sel-sched authors please test that?

Sorry for delay, I was traveling until yesterday.  It's my oversight to 
make sel_print a macro, and I think it is perfectly fine to make it a 
function.  Thanks for doing this!

I did verify that the generated dot file looks the same before and after 
your patch.  The dumping code itself is supposed to work after all major 
scheduler data structures are initialized.  I will add a comment about this 
in the sel-sched-dump.h after your patch will go in.

Yours, Andrey

Patch

--- gcc/sel-sched-dump.h.jj	2010-06-14 11:08:04.000000000 +0200
+++ gcc/sel-sched-dump.h	2010-06-14 11:15:38.000000000 +0200
@@ -177,34 +177,13 @@  extern void dump_insn_1 (insn_t, int);
 extern void dump_insn (insn_t);
 extern void debug_insn (insn_t);
 
-extern void sel_prepare_string_for_dot_label (char *);
-
 /* When this flag is on, we are dumping to the .dot file.
    When it is off, we are dumping to log.  */
 extern bool sched_dump_to_dot_p;
-
-/* This macro acts like printf but dumps information to the .dot file.
-   Used when dumping control flow.  */
-#define sel_print_to_dot(...)                           \
-  do {                                                  \
-    int __j = 1 + 2 * snprintf (NULL, 0, __VA_ARGS__);  \
-    char *__s = XALLOCAVEC (char, __j);                 \
-    snprintf (__s, __j, __VA_ARGS__);                   \
-    sel_prepare_string_for_dot_label (__s);             \
-    fprintf (sched_dump, "%s", __s);                    \
-  } while (0)
-
-/* This macro acts like printf but dumps to the sched_dump file.  */
-#define sel_print(...)					\
-  do {							\
-    if (sched_dump_to_dot_p)                            \
-      sel_print_to_dot (__VA_ARGS__);                   \
-    else                                                \
-      fprintf (sched_dump, __VA_ARGS__);                \
-  } while (0)
 
 
 /* Functions from sel-sched-dump.c.  */
+extern void sel_print (const char *fmt, ...) ATTRIBUTE_PRINTF_1;
 extern const char * sel_print_insn (const_rtx, int);
 extern void free_sel_dump_data (void);
 
--- gcc/sel-sched-dump.c.jj	2010-06-14 11:08:04.000000000 +0200
+++ gcc/sel-sched-dump.c	2010-06-14 11:14:56.000000000 +0200
@@ -566,7 +566,7 @@  replace_str_in_buf (char *buf, const cha
 }
 
 /* Replace characters in BUF that have special meaning in .dot file.  */
-void
+static void
 sel_prepare_string_for_dot_label (char *buf)
 {
   static char specials_from[7][2] = { "<", ">", "{", "|", "}", "\"",
@@ -579,6 +579,28 @@  sel_prepare_string_for_dot_label (char *
     replace_str_in_buf (buf, specials_from[i], specials_to[i]);
 }
 
+/* This function acts like printf but dumps to the sched_dump file.  */
+void
+sel_print (const char *fmt, ...)
+{
+  va_list ap;
+  va_start (ap, fmt);
+  if (sched_dump_to_dot_p)
+    {
+      char *message;
+      if (vasprintf (&message, fmt, ap) >= 0 && message != NULL)
+	{
+	  message = (char *) xrealloc (message, 2 * strlen (message) + 1);
+	  sel_prepare_string_for_dot_label (message);
+	  fprintf (sched_dump, "%s", message);
+	  free (message);
+	}
+    }
+  else
+    vfprintf (sched_dump, fmt, ap);
+  va_end (ap);
+}
+
 /* Dump INSN with FLAGS.  */
 static void
 sel_dump_cfg_insn (insn_t insn, int flags)