On Wed, Mar 27, 2013 at 12:50 AM, Lawrence Crowl <cr...@googlers.com> wrote:
> On 3/26/13, Lawrence Crowl <cr...@googlers.com> wrote:
>> On 3/26/13, Richard Biener <richard.guent...@gmail.com> wrote:
>>> On Mar 25, 2013 Lawrence Crowl <cr...@googlers.com> wrote:
>>> > On 3/25/13, Richard Biener <richard.guent...@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 <cr...@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 <cr...@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
