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: [patch] Unified debug dump function names.


On Wed, Mar 27, 2013 at 12:50 AM, Lawrence Crowl <crowl@googlers.com> wrote:
> On 3/26/13, Lawrence Crowl <crowl@googlers.com> wrote:
>> On 3/26/13, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Mar 25, 2013 Lawrence Crowl <crowl@googlers.com> wrote:
>>> > On 3/25/13, Richard Biener <richard.guenther@gmail.com> wrote:
>>> > > You add a not used new interface.  What for?
>>> >
>>> > So that people can use it.
>>> >
>>> > > For use from gdb only?
>>> >
>>> > No, for use from both gdb and internally.  It is often that
>>> > folks add dumps in various places while developing/debugging.
>>> > These functions support that effort without having to hunt down
>>> > the name.
>>>
>>> But having both interfaces is bad.  As you are unconditionally
>>> "dumping" to stderr using debug () is correct.  Sorry that I
>>> don't follow each and every proposal - nobody follows up my
>>> proposals either.
>>>
>>> The dump_ namespace is claimed by dumping to dumpfiles and
>>> diagnostics.
>>>
>>> > > In which case it should be debug (), not dump ().
>>> >
>>> > I will use whatever name you wish, but I would have preferred
>>> > that we addressed naming issues when we published the plan,
>>> > not after I've done the implementation.  What name do you wish?
>>>
>>> debug ().
>>
>> Okay.
>>
>>> And I'd like you to remove the alternate debug_ interface that
>>> is obsoleted by the overloads.
>>
>> I'm sure that folks have the old interfaces baked into scripts and
>> dot files.  I think it would should not remove the old interface
>> until they have had some time to migrate.
>>
>>> Btw, the overloading will provide extra headache to one of the
>>> most used ways to the debug_ routines:
>>>
>>> (gdb) call debug_tree (fndecl)
>>> <function_decl 0x7ffff6e1b900 foo
>>>    type <function_type 0x7ffff6d28c78
>>>        type <integer_type 0x7ffff6d175e8 int public SI
>>>            size <integer_cst 0x7ffff6d1a0c0 constant 32>
>>>            unit size <integer_cst 0x7ffff6d1a0e0 constant 4>
>>>            align 32 symtab 0 alias set -1 canonical type
>>> 0x7ffff6d175e8 precision 32 min <integer_cst 0x7ffff6d1a060
>>> -2147483648> max <integer_cst 0x7ffff6d1a080 2147483647>
>>> ...
>>> (gdb) call debug_tree (0x7ffff6d175e8)
>>> Cannot resolve function debug_tree to any overloaded instance
>>> (gdb) call debug_tree<tab><tab>
>>> debug_tree(tree_node*)
>>> debug_tree_chain(tree_node*)
>>> debug_tree_chain(tree_node*)::__FUNCTION__
>>> debug_tree_ssa()
>>> debug_tree_ssa_stats()
>>> <aha! (ok, I know this one is 'tree')>
>>> (gdb) call debug_tree ((tree_node*)0x7ffff6d175e8)
>>> <integer_type 0x7ffff6d175e8 int public SI
>>>    size <integer_cst 0x7ffff6d1a0c0 type <integer_type 0x7ffff6d170a8
>>> bitsizetype> constant 32>
>>>    unit size <integer_cst 0x7ffff6d1a0e0 type <integer_type
>>> 0x7ffff6d17000 sizetype> constant 4>
>>>    align 32 symtab 0 alias set -1 canonical type 0x7ffff6d175e8
>>> precision 32 min <integer_cst 0x7ffff6d1a060 -2147483648> max
>>> <integer_cst 0x7ffff6d1a080 2147483647>
>>>    pointer_to_this <pointer_type 0x7ffff6d1f2a0>>
>>>
>>> but with debug () having overloads to each and every thing we'd
>>> ever want to debug the list of possible types I have to cast that
>>> literal address I cut&pasted will be endless.
>>>
>>> Any suggestion on how to improve this situation?  Yes, it's already
>>> bad as with typing debug_tree I know it's a tree I am interested
>>> in and
>>>
>>> (gdb) call debug_<tab><tab>
>>> ... endless list of functions and overloads ...
>>>
>>> is probably as useless as
>>>
>>> (gdb) call debug<tab><tab>
>>>
>>> is after your patch.
>>
>> I have three suggestions, in increasing order of difficulty.
>>
>> First, modify the dumpers to print the type cast in front of
>> the hex value.  The cut and paste is just a bit wider.
>>
>> Second, modify the dumpers to print the access expression (which
>> would then not require the hex value).  I'm not actually sure how
>> well this would work in practice.  It's a thought.
>>
>> Third, modify gdb to have an interactive data explorer.  As a
>> straw man, "explorer foo" would open up a window with all of
>> foo's elements.  Each pointer is clickable and changes your view to
>> its referrent.  I've used such a tool, and while it was sometimes
>> at too low a level of abstraction, it was generally quite handy
>> for exploring the data.  In retrospect, it would be nice to fold
>> sub-objects (in the editor sense).
>
> Patch with rename to debug attached.
> Tested on x86_64.
>
>
> Add uniform debug dump function names.
>
>
> Add some overloaded functions that provide uniform debug dump
> function names.  These names are:
>
>   debug: the general debug dumper
>   debug_verbose: for more details
>   debug_raw: for the gory details
>   debug_head: for the heads of declarations, e.g. function heads
>   debug_body: for the bodies of declarations, e.g. function bodies
>
> Not all types have the last four versions.
>
> The debug functions come in two flavors, those that take pointers
> to the type, and those that take references to the type.  The first
> handles printing of '<nil>' for null pointers.  The second assumes
> a valid reference, and prints the content.
>
>
> Example uses are as follows:
>
>   cp_token t, *p;
>   debug (t);
>   debug (p);
>
> From the debugger, use
>
>   call debug (t)
>
>
> The functions sets implemented are:
>
> debug (only)
>
>     basic_block_def, const bitmap_head_def, cp_binding_level,
>     cp_parser, cp_token, data_reference, die_struct, edge_def,
>     gimple_statement_d, ira_allocno, ira_allocno_copy, live_range,
>     lra_live_range, omega_pb_d, pt_solution, const rtx_def, sreal,
>     tree_live_info_d, _var_map,
>
>     vec<cp_token, va_gc>, vec<data_reference_p>, vec<ddr_p>,
>     vec<rtx>, vec<tree, va_gc>,
>
> debug and debug_raw
>
>     simple_bitmap_def
>
> debug and debug_verbose
>
>     expr_def, struct loop, vinsn_def
>
> debug, debug_raw, debug_verbose, debug_head, debug_body
>
>     const tree_node
>
>
> This patch is somewhat different from the original plan at
> gcc.gnu.org/wiki/cxx-conversion/debugging-dumps.  The reason
> is that gdb has an incomplete implementation of C++ call syntax;
> requiring explicit specification of template arguments and explicit
> specification of function arguments even when they have default
> values.  So, the original plan would have required typing
>
>   call dump <cp_token> (t, 0, 0, stderr)
>
> which is undesireable.  Instead instead of templates, we overload
> plain functions.  This adds a small burden of manually adding
> the pointer version of dump for each type.  Instead of default
> function arguments, we simply assume the default values.  Most of
> the underlying dump functions did not use the options and indent
> parameters anyway.  Several provide FILE* parameters, but we expect
> debugging to use stderr anyway.  So, the explicit specification of
> arguments was not as valuable as we thought initially.

Note that generally modules should provide a

 print_FOO (FILE *, FOO *...)

interface which should be the worker for the dump_* interface
which prints to dumpfiles (and stdout/stderr with -fopt-info) and
the debug_* interface (which just prints to stderr).

>  Finally,
> a change of name from dump to debug reflect the implicit output
> to stderr.

A few more questions.  As we are now using C++ and these functions
are not called by GCC itself - do we really need all the extern declarations
in the header files?  We don't have -Wstrict-prototypes issues with C++
and I do not consider the debug () interface part of the public API of
a module.  This avoids people ending up calling debug () from inside
GCC.

+  if (ptr)
+    debug (*ptr);
+  else
+    fprintf (stderr, "<nil>\n");

can we avoid repeating this using a common helper?  I'd use a simple
#define like

#define DO_DEBUG_PTR(p) do { if (p) debug (*(p)) else fprintf (stderr,
"<nil>\n"); } while (0)

but I suppose you can come up with something more clever using C++?

Thanks,
Richard.

>
> Index: gcc/cp/ChangeLog
>
> 2013-03-26  Lawrence Crowl  <crowl@google.com>
>
>         * Make-lang.in
>         (CXX_PARSER_H): Add header dependence.
>         * cp-tree.h
>         (extern debug (cp_binding_level &)): New.
>         (extern debug (cp_binding_level *)): New.
>         * name-lookup.h
>         (debug (cp_binding_level &)): New.
>         (debug (cp_binding_level *)): New.
>         * parser.c
>         (debug (cp_parser &)): New.
>         (debug (cp_parser *)): New.
>         (debug (cp_token &)): New.
>         (debug (cp_token *)): New.
>         (debug (vec<cp_token, va_gc> &)): New.
>         (debug (vec<cp_token, va_gc> *)): New.
>         * parser.c: Add header dependence.
>         (extern debug (cp_parser &)): New.
>         (extern debug (cp_parser *)): New.
>         (extern debug (cp_token &)): New.
>         (extern debug (cp_token *)): New.
>         (extern debug (vec<cp_token, va_gc> &)): New.
>         (extern debug (vec<cp_token, va_gc> *)): New.
>
> Index: gcc/ChangeLog
>
> 2013-03-26  Lawrence Crowl  <crowl@google.com>
>
>         * Makefile.in: Add several missing include dependences.
>         (DUMPFILE_H): New.
>         (test-dump.o): New.  This object is not added to any executable,
>         but is present for ad-hoc testing.
>         * bitmap.c
>         (debug (const bitmap_head_def &)): New.
>         (debug (const bitmap_head_def *)): New.
>         * bitmap.h
>         (extern debug (const bitmap_head_def &)): New.
>         (extern debug (const bitmap_head_def *)): New.
>         * cfg.c
>         (debug (edge_def &)): New.
>         (debug (edge_def *)): New.
>         * cfghooks.c
>         (debug (basic_block_def &)): New.
>         (debug (basic_block_def *)): New.
>         * dumpfile.h
>         (dump_node (const_tree, int, FILE *)): Correct source file.
>         * dwarf2out.c
>         (debug (die_struct &)): New.
>         (debug (die_struct *)): New.
>         * dwarf2out.h
>         (extern debug (die_struct &)): New.
>         (extern debug (die_struct *)): New.
>         * gimple-pretty-print.c
>         (debug (gimple_statement_d &)): New.
>         (debug (gimple_statement_d *)): New.
>         * gimple-pretty-print.h
>         (extern debug (gimple_statement_d &)): New.
>         (extern debug (gimple_statement_d *)): New.
>         * ira-build.c
>         (debug (ira_allocno_copy &)): New.
>         (debug (ira_allocno_copy *)): New.
>         (debug (ira_allocno &)): New.
>         (debug (ira_allocno *)): New.
>         * ira-int.h
>         (extern debug (ira_allocno_copy &)): New.
>         (extern debug (ira_allocno_copy *)): New.
>         (extern debug (ira_allocno &)): New.
>         (extern debug (ira_allocno *)): New.
>         * ira-lives.c
>         (debug (live_range &)): New.
>         (debug (live_range *)): New.
>         * lra-int.h
>         (debug (lra_live_range &)): New.
>         (debug (lra_live_range *)): New.
>         * lra-lives.c
>         (debug (lra_live_range &)): New.
>         (debug (lra_live_range *)): New.
>         * omega.c
>         (debug (omega_pb_d &)): New.
>         (debug (omega_pb_d *)): New.
>         * omega.h
>         (extern debug (omega_pb_d &)): New.
>         (extern debug (omega_pb_d *)): New.
>         * print-rtl.c
>         (debug (const rtx_def &)): New.
>         (debug (const rtx_def *)): New.
>         * print-tree.c
>         (debug_tree (tree): Move within file.
>         (debug_raw (const tree_node &)): New.
>         (debug_raw (const tree_node *)): New.
>         (dump_tree_via_hooks (const tree_node *, int)): New.
>         (debug (const tree_node &)): New.
>         (debug (const tree_node *)): New.
>         (debug_verbose (const tree_node &)): New.
>         (debug_verbose (const tree_node *)): New.
>         (debug_head (const tree_node &)): New.
>         (debug_head (const tree_node *)): New.
>         (debug_body (const tree_node &)): New.
>         (debug_body (const tree_node *)): New.
>         (debug_vec_tree (tree): Move and reimplement in terms of dump.
>         (debug (vec<tree, va_gc> &)): New.
>         (debug (vec<tree, va_gc> *)): New.
>         * rtl.h
>         (extern debug (const rtx_def &)): New.
>         (extern debug (const rtx_def *)): New.
>         * sbitmap.c
>         (debug_raw (simple_bitmap_def &)): New.
>         (debug_raw (simple_bitmap_def *)): New.
>         (debug (simple_bitmap_def &)): New.
>         (debug (simple_bitmap_def *)): New.
>         * sbitmap.h
>         (extern debug (simple_bitmap_def &)): New.
>         (extern debug (simple_bitmap_def *)): New.
>         (extern debug_raw (simple_bitmap_def &)): New.
>         (extern debug_raw (simple_bitmap_def *)): New.
>         * sel-sched-dump.c
>         (debug (vinsn_def &)): New.
>         (debug (vinsn_def *)): New.
>         (debug_verbose (vinsn_def &)): New.
>         (debug_verbose (vinsn_def *)): New.
>         (debug (expr_def &)): New.
>         (debug (expr_def *)): New.
>         (debug_verbose (expr_def &)): New.
>         (debug_verbose (expr_def *)): New.
>         (debug (vec<rtx> &)): New.
>         (debug (vec<rtx> *)): New.
>         * sel-sched-dump.h
>         (extern debug (vinsn_def &)): New.
>         (extern debug (vinsn_def *)): New.
>         (extern debug_verbose (vinsn_def &)): New.
>         (extern debug_verbose (vinsn_def *)): New.
>         (extern debug (expr_def &)): New.
>         (extern debug (expr_def *)): New.
>         (extern debug_verbose (expr_def &)): New.
>         (extern debug_verbose (expr_def *)): New.
>         (extern debug (vec<rtx> &)): New.
>         (extern debug (vec<rtx> *)): New.
>         * sel-sched-ir.h
>         (_list_iter_cond_expr): Make inline instead of static.
>         * sreal.c
>         (debug (sreal &)): New.
>         (debug (sreal *)): New.
>         * sreal.h
>         (extern debug (sreal &)): New.
>         (extern debug (sreal *)): New.
>         * tree.h
>         (extern debug_raw (const tree_node &)): New.
>         (extern debug_raw (const tree_node *)): New.
>         (extern debug (const tree_node &)): New.
>         (extern debug (const tree_node *)): New.
>         (extern debug_verbose (const tree_node &)): New.
>         (extern debug_verbose (const tree_node *)): New.
>         (extern debug_head (const tree_node &)): New.
>         (extern debug_head (const tree_node *)): New.
>         (extern debug_body (const tree_node &)): New.
>         (extern debug_body (const tree_node *)): New.
>         (extern debug (vec<tree, va_gc> &)): New.
>         (extern debug (vec<tree, va_gc> *)): New.
>         * tree-cfg.c
>         (debug (struct loop &)): New.
>         (debug (struct loop *)): New.
>         (debug_verbose (struct loop &)): New.
>         (debug_verbose (struct loop *)): New.
>         * tree-dump.c: Add header dependence.
>         * tree-flow.h
>         (extern debug (struct loop &)): New.
>         (extern debug (struct loop *)): New.
>         (extern debug_verbose (struct loop &)): New.
>         (extern debug_verbose (struct loop *)): New.
>         * tree-data-ref.c
>         (debug (data_reference &)): New.
>         (debug (data_reference *)): New.
>         (debug (vec<data_reference_p> &)): New.
>         (debug (vec<data_reference_p> *)): New.
>         (debug (vec<ddr_p> &)): New.
>         (debug (vec<ddr_p> *)): New.
>         * tree-data-ref.h
>         (extern debug (data_reference &)): New.
>         (extern debug (data_reference *)): New.
>         (extern debug (vec<data_reference_p> &)): New.
>         (extern debug (vec<data_reference_p> *)): New.
>         (extern debug (vec<ddr_p> &)): New.
>         (extern debug (vec<ddr_p> *)): New.
>         * tree-ssa-alias.c
>         (debug (pt_solution &)): New.
>         (debug (pt_solution *)): New.
>         * tree-ssa-alias.h
>         (extern debug (pt_solution &)): New.
>         (extern debug (pt_solution *)): New.
>         * tree-ssa-alias.c
>         (debug (_var_map &)): New.
>         (debug (_var_map *)): New.
>         (debug (tree_live_info_d &)): New.
>         (debug (tree_live_info_d *)): New.
>         * tree-ssa-alias.h
>         (extern debug (_var_map &)): New.
>         (extern debug (_var_map *)): New.
>         (extern debug (tree_live_info_d &)): New.
>         (extern debug (tree_live_info_d *)): New.
>
>
> --
> Lawrence Crowl


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