diff mbox

RFC: LRA for x86/x86-64 [2/9]

Message ID 5064D9DD.8060507@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov Sept. 27, 2012, 10:57 p.m. UTC
LRA outputs a lot debug information about insns.  I found that using 
slim insn/rtl presentation helps a lot for LRA debuging. The following 
patch makes slim presentation printing functions visible to LRA.  It 
also implements one more such function.

2012-09-27  Vladimir Makarov  <vmakarov@redhat.com>

     * rtl.h (debug_bb_n_slim, debug_bb_slim, print_value_slim): New
     prototypes.
     (debug_rtl_slim, debug_insn_slim): Ditto.
     * sched-vis.c (print_value_slim): New.

Comments

Steven Bosscher Sept. 28, 2012, 8:43 a.m. UTC | #1
On Fri, Sep 28, 2012 at 12:57 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
> LRA outputs a lot debug information about insns.  I found that using slim
> insn/rtl presentation helps a lot for LRA debuging. The following patch
> makes slim presentation printing functions visible to LRA.  It also
> implements one more such function.
>
> 2012-09-27  Vladimir Makarov  <vmakarov@redhat.com>
>
>     * rtl.h (debug_bb_n_slim, debug_bb_slim, print_value_slim): New
>     prototypes.
>     (debug_rtl_slim, debug_insn_slim): Ditto.
>     * sched-vis.c (print_value_slim): New.

I have patches in the works to use the slim RTL dumping format more,
too, and to use the pretty-printer code so that printing strings with
escaped characters can be made more transparent (e.g. for use in
GraphViz dumps).

Perhaps it's time to rename sched-vis.c to print-rtl-slim.c? :-)

Ciao!
Steven
Vladimir Makarov Sept. 28, 2012, 3:21 p.m. UTC | #2
On 12-09-28 4:43 AM, Steven Bosscher wrote:
> On Fri, Sep 28, 2012 at 12:57 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>> LRA outputs a lot debug information about insns.  I found that using slim
>> insn/rtl presentation helps a lot for LRA debuging. The following patch
>> makes slim presentation printing functions visible to LRA.  It also
>> implements one more such function.
>>
>> 2012-09-27  Vladimir Makarov  <vmakarov@redhat.com>
>>
>>      * rtl.h (debug_bb_n_slim, debug_bb_slim, print_value_slim): New
>>      prototypes.
>>      (debug_rtl_slim, debug_insn_slim): Ditto.
>>      * sched-vis.c (print_value_slim): New.
> I have patches in the works to use the slim RTL dumping format more,
> too, and to use the pretty-printer code so that printing strings with
> escaped characters can be made more transparent (e.g. for use in
> GraphViz dumps).
That would be nice.  Slim printing is very useful for LRA which prints a 
lot of changes in RTL code during all its work.  Regular printing is 
unreadable because of its volume.  For LRA debugging I usually find a 
suspicious place in slim dump and if I need more info I use regular dump 
of the suspicious insn.
> Perhaps it's time to rename sched-vis.c to print-rtl-slim.c? :-)
>
Yes, the name sched-vis.c is very misleading.
Jeff Law Sept. 28, 2012, 3:36 p.m. UTC | #3
On 09/28/2012 09:21 AM, Vladimir Makarov wrote:
> On 12-09-28 4:43 AM, Steven Bosscher wrote:
>> On Fri, Sep 28, 2012 at 12:57 AM, Vladimir Makarov
>> <vmakarov@redhat.com> wrote:
>>> LRA outputs a lot debug information about insns.  I found that using
>>> slim
>>> insn/rtl presentation helps a lot for LRA debuging. The following patch
>>> makes slim presentation printing functions visible to LRA.  It also
>>> implements one more such function.
>>>
>>> 2012-09-27  Vladimir Makarov  <vmakarov@redhat.com>
>>>
>>>      * rtl.h (debug_bb_n_slim, debug_bb_slim, print_value_slim): New
>>>      prototypes.
>>>      (debug_rtl_slim, debug_insn_slim): Ditto.
>>>      * sched-vis.c (print_value_slim): New.
>> I have patches in the works to use the slim RTL dumping format more,
>> too, and to use the pretty-printer code so that printing strings with
>> escaped characters can be made more transparent (e.g. for use in
>> GraphViz dumps).
> That would be nice.  Slim printing is very useful for LRA which prints a
> lot of changes in RTL code during all its work.  Regular printing is
> unreadable because of its volume.  For LRA debugging I usually find a
> suspicious place in slim dump and if I need more info I use regular dump
> of the suspicious insn.
>> Perhaps it's time to rename sched-vis.c to print-rtl-slim.c? :-)
>>
> Yes, the name sched-vis.c is very misleading.
It seems to me this change ought to go forward now (rename to 
print-rtl-slim.c and add print_value_slim.

WRT print_value_slim we have this block comment:




+/* Prints rtxes, I customarily classified as values.  They're
+   constants, registers, labels, symbols and memory accesses.  Print
+   them to file F.  */


That block comment just doesn't make sense when I read it.  It seems like.

/* Print X, an RTL value node, to file F in slim format.  Include
    additional information if VERBOSE is nonzero.

    Value nodes are constants, registers, labels, symbols and
    memory.  */


With that change I think you could make the obvious changes necessary to 
rename to print-rtl-slim and check this patch in.

jeff
Vladimir Makarov Sept. 28, 2012, 11:13 p.m. UTC | #4
On 12-09-28 11:36 AM, Jeff Law wrote:
> On 09/28/2012 09:21 AM, Vladimir Makarov wrote:
>> On 12-09-28 4:43 AM, Steven Bosscher wrote:
>>> I have patches in the works to use the slim RTL dumping format more,
>>> too, and to use the pretty-printer code so that printing strings with
>>> escaped characters can be made more transparent (e.g. for use in
>>> GraphViz dumps).
>> That would be nice.  Slim printing is very useful for LRA which prints a
>> lot of changes in RTL code during all its work.  Regular printing is
>> unreadable because of its volume.  For LRA debugging I usually find a
>> suspicious place in slim dump and if I need more info I use regular dump
>> of the suspicious insn.
>>> Perhaps it's time to rename sched-vis.c to print-rtl-slim.c? :-)
>>>
>> Yes, the name sched-vis.c is very misleading.
> It seems to me this change ought to go forward now (rename to 
> print-rtl-slim.c and add print_value_slim.
>
> WRT print_value_slim we have this block comment:
>
>
>
>
> +/* Prints rtxes, I customarily classified as values.  They're
> +   constants, registers, labels, symbols and memory accesses. Print
> +   them to file F.  */
>
>
> That block comment just doesn't make sense when I read it.  It seems 
> like.
>
> /* Print X, an RTL value node, to file F in slim format.  Include
>    additional information if VERBOSE is nonzero.
>
>    Value nodes are constants, registers, labels, symbols and
>    memory.  */
>
>
> With that change I think you could make the obvious changes necessary 
> to rename to print-rtl-slim and check this patch in.
>
Thanks, Jeff.  I will do it on Monday.
Vladimir Makarov Oct. 3, 2012, 9:05 p.m. UTC | #5
On 09/28/2012 11:36 AM, Jeff Law wrote:
> On 09/28/2012 09:21 AM, Vladimir Makarov wrote:
>> On 12-09-28 4:43 AM, Steven Bosscher wrote:
>>> On Fri, Sep 28, 2012 at 12:57 AM, Vladimir Makarov
>>> <vmakarov@redhat.com> wrote:
>>>> LRA outputs a lot debug information about insns.  I found that using
>>>> slim
>>>> insn/rtl presentation helps a lot for LRA debuging. The following 
>>>> patch
>>>> makes slim presentation printing functions visible to LRA. It also
>>>> implements one more such function.
>>>>
>>>> 2012-09-27  Vladimir Makarov  <vmakarov@redhat.com>
>>>>
>>>>      * rtl.h (debug_bb_n_slim, debug_bb_slim, print_value_slim): New
>>>>      prototypes.
>>>>      (debug_rtl_slim, debug_insn_slim): Ditto.
>>>>      * sched-vis.c (print_value_slim): New.
>>> I have patches in the works to use the slim RTL dumping format more,
>>> too, and to use the pretty-printer code so that printing strings with
>>> escaped characters can be made more transparent (e.g. for use in
>>> GraphViz dumps).
>> That would be nice.  Slim printing is very useful for LRA which prints a
>> lot of changes in RTL code during all its work.  Regular printing is
>> unreadable because of its volume.  For LRA debugging I usually find a
>> suspicious place in slim dump and if I need more info I use regular dump
>> of the suspicious insn.
>>> Perhaps it's time to rename sched-vis.c to print-rtl-slim.c? :-)
>>>
>> Yes, the name sched-vis.c is very misleading.
> It seems to me this change ought to go forward now (rename to 
> print-rtl-slim.c and add print_value_slim.
>
> WRT print_value_slim we have this block comment:
>
>
>
>
> +/* Prints rtxes, I customarily classified as values.  They're
> +   constants, registers, labels, symbols and memory accesses. Print
> +   them to file F.  */
>
>
> That block comment just doesn't make sense when I read it.  It seems 
> like.
>
> /* Print X, an RTL value node, to file F in slim format.  Include
>    additional information if VERBOSE is nonzero.
>
>    Value nodes are constants, registers, labels, symbols and
>    memory.  */
>
>
> With that change I think you could make the obvious changes necessary 
> to rename to print-rtl-slim and check this patch in.
>
    Unfortunately, it is not easy.  The right thing to do is to make it 
independent from scheduler. There are dependecies on insn scheduler in 
this file.  Hooks print_insn and current_sched_info are used in the code.

     So I just fixed the comment and will work on removing dependencies 
later if I have time.
diff mbox

Patch

Index: rtl.h
===================================================================
--- rtl.h	(revision 191771)
+++ rtl.h	(working copy)
@@ -2487,7 +2487,12 @@  extern rtx make_compound_operation (rtx,
 extern void delete_dead_jumptables (void);
 
 /* In sched-vis.c.  */
-extern void dump_insn_slim (FILE *, const_rtx x);
+extern void debug_bb_n_slim (int);
+extern void debug_bb_slim (struct basic_block_def *);
+extern void print_value_slim (FILE *, const_rtx, int);
+extern void debug_rtl_slim (FILE *, const_rtx, const_rtx, int, int);
+extern void dump_insn_slim (FILE *f, const_rtx x);
+extern void debug_insn_slim (const_rtx x);
 
 /* In sched-rgn.c.  */
 extern void schedule_insns (void);
Index: sched-vis.c
===================================================================
--- sched-vis.c	(revision 191771)
+++ sched-vis.c	(working copy)
@@ -546,6 +546,19 @@  print_value (char *buf, const_rtx x, int
     }
 }				/* print_value */
 
+/* Prints rtxes, I customarily classified as values.  They're
+   constants, registers, labels, symbols and memory accesses.  Print
+   them to file F.  */
+
+void
+print_value_slim (FILE *f, const_rtx x, int verbose)
+{
+  char buf[BUF_LEN];
+
+  print_value (buf, x, verbose);
+  fprintf (f, "%s", buf);
+}
+
 /* The next step in insn detalization, its pattern recognition.  */
 
 void