Re: [PATCH 1/2] Add "optinfo" framework
On Tue, Jul 10, 2018 at 1:00 PM David Malcolm wrote: > > On Mon, 2018-07-09 at 15:00 +0200, Richard Biener wrote: > > On Mon, Jul 2, 2018 at 10:51 PM David Malcolm > > wrote: > > > > > > This patch implements a way to consolidate dump_* calls into > > > optinfo objects, as enabling work towards being able to write out > > > optimization records to a file, or emit them as diagnostic > > > "remarks". > > > > > > The patch adds the support for building optinfo instances from > > > dump_* > > > calls, but leaves implementing any *users* of them to followup > > > patches. > > > > > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > > > > > OK for trunk? > > > > Looks good overall, but ... > > > > To "fix" the GC issue you'd need to capture all possibly interesting > > information from tree/gimple while it is still in flight. This _may_ > > be > > necessary anyway since I remember writing code like > > > > fprintf (dump_file, "old: "); > > print_gimple_stmt (..., stmt); > > gimple_set_rhs1 (stmt, op); > > fprintf (dump_file, "new: "); > > print_gmple_stmt (..., stmt); > > fprintf (dump_file, "\n"); > > > > capturing interesting information means we know all targeted > > optinfo channels, right? And the optinfo consumers > > need to handle "streams" of input and may not look back. > > > I've yet have to look at the 2nd patch but can you comment on > > this? How difficult is it to re-wire how the data flows to make > > stmt re-use like the above possible? > > I *think* it's doable: rather than capture, say, a gimple *, the > optinfo_item would capture the result of pp_gimple_stmt_1, plus some > metadata. In fact, it would probably allow for removing the > optinfo_item subclasses, making optinfo_item concrete, containing > something like: > > /* Textual form. */ > char *m_text; > bool m_ownership_of_text; > > /* Metadata for optimization records. */ > enum optinfo_item_kind m_kind; > location_t m_location; > > or somesuch. > > I'll have a go at implementing this. Thanks, that would be much cleaner (if also a bit more fugly when you need to debug things) Richard. > Thanks > Dave > > > Thanks, > > Richard. > > > > > gcc/ChangeLog: > > > * Makefile.in (OBJS): Add optinfo.o. > > > * coretypes.h (class symtab_node): New forward decl. > > > (struct cgraph_node): New forward decl. > > > (class varpool_node): New forward decl. > > > * dump-context.h: New file. > > > * dumpfile.c: Include "optinfo.h", "dump-context.h", > > > "cgraph.h", > > > "tree-pass.h", "optinfo-internal.h". > > > (refresh_dumps_are_enabled): Use optinfo_enabled_p. > > > (set_dump_file): Call > > > dumpfile_ensure_any_optinfo_are_flushed. > > > (set_alt_dump_file): Likewise. > > > (dump_context::~dump_context): New dtor. > > > (dump_gimple_stmt): Move implementation to... > > > (dump_context::dump_gimple_stmt): ...this new member > > > function. > > > Add the stmt to any pending optinfo, creating one if need > > > be. > > > (dump_gimple_stmt_loc): Move implementation to... > > > (dump_context::dump_gimple_stmt_loc): ...this new member > > > function. > > > Convert param "loc" from location_t to const > > > dump_location_t &. > > > Start a new optinfo and add the stmt to it. > > > (dump_generic_expr): Move implementation to... > > > (dump_context::dump_generic_expr): ...this new member > > > function. > > > Add the tree to any pending optinfo, creating one if need > > > be. > > > (dump_generic_expr_loc): Move implementation to... > > > (dump_context::dump_generic_expr_loc): ...this new member > > > function. Add the tree to any pending optinfo, creating > > > one if > > > need be. > > > (dump_printf): Move implementation to... > > > (dump_context::dump_printf_va): ...this new member > > > function. Add > > > the text to any pending optinfo, creating one if need be. > > > (dump_printf_loc): Move implementation to... > > > (dump_context::dump_printf_loc_va): ...this new member > > > function. > > > Convert param "loc" from location_t to const > > > dump_location_t &. > > > Start a new optinfo and add the stmt to it. > > > (dump_dec): Move implementation to... > > > (dump_context::dump_dec): ...this new member function. Add > > > the > > > value to any pending optinfo, creating one if need be. > > > (dump_context::dump_symtab_node): New member function. > > > (dump_context::get_scope_depth): New member function. > > > (dump_context::begin_scope): New member function. > > > (dump_context::end_scope): New member function. > > > (dump_context::ensure_pending_optinfo): New member > > > function. > > > (dump_context::begin_next_optinfo): New member function. > > >
Re: [PATCH 1/2] Add "optinfo" framework
On Mon, 2018-07-09 at 15:00 +0200, Richard Biener wrote: > On Mon, Jul 2, 2018 at 10:51 PM David Malcolm > wrote: > > > > This patch implements a way to consolidate dump_* calls into > > optinfo objects, as enabling work towards being able to write out > > optimization records to a file, or emit them as diagnostic > > "remarks". > > > > The patch adds the support for building optinfo instances from > > dump_* > > calls, but leaves implementing any *users* of them to followup > > patches. > > > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > > > OK for trunk? > > Looks good overall, but ... > > To "fix" the GC issue you'd need to capture all possibly interesting > information from tree/gimple while it is still in flight. This _may_ > be > necessary anyway since I remember writing code like > > fprintf (dump_file, "old: "); > print_gimple_stmt (..., stmt); > gimple_set_rhs1 (stmt, op); > fprintf (dump_file, "new: "); > print_gmple_stmt (..., stmt); > fprintf (dump_file, "\n"); > > capturing interesting information means we know all targeted > optinfo channels, right? And the optinfo consumers > need to handle "streams" of input and may not look back. > I've yet have to look at the 2nd patch but can you comment on > this? How difficult is it to re-wire how the data flows to make > stmt re-use like the above possible? I *think* it's doable: rather than capture, say, a gimple *, the optinfo_item would capture the result of pp_gimple_stmt_1, plus some metadata. In fact, it would probably allow for removing the optinfo_item subclasses, making optinfo_item concrete, containing something like: /* Textual form. */ char *m_text; bool m_ownership_of_text; /* Metadata for optimization records. */ enum optinfo_item_kind m_kind; location_t m_location; or somesuch. I'll have a go at implementing this. Thanks Dave > Thanks, > Richard. > > > gcc/ChangeLog: > > * Makefile.in (OBJS): Add optinfo.o. > > * coretypes.h (class symtab_node): New forward decl. > > (struct cgraph_node): New forward decl. > > (class varpool_node): New forward decl. > > * dump-context.h: New file. > > * dumpfile.c: Include "optinfo.h", "dump-context.h", > > "cgraph.h", > > "tree-pass.h", "optinfo-internal.h". > > (refresh_dumps_are_enabled): Use optinfo_enabled_p. > > (set_dump_file): Call > > dumpfile_ensure_any_optinfo_are_flushed. > > (set_alt_dump_file): Likewise. > > (dump_context::~dump_context): New dtor. > > (dump_gimple_stmt): Move implementation to... > > (dump_context::dump_gimple_stmt): ...this new member > > function. > > Add the stmt to any pending optinfo, creating one if need > > be. > > (dump_gimple_stmt_loc): Move implementation to... > > (dump_context::dump_gimple_stmt_loc): ...this new member > > function. > > Convert param "loc" from location_t to const > > dump_location_t &. > > Start a new optinfo and add the stmt to it. > > (dump_generic_expr): Move implementation to... > > (dump_context::dump_generic_expr): ...this new member > > function. > > Add the tree to any pending optinfo, creating one if need > > be. > > (dump_generic_expr_loc): Move implementation to... > > (dump_context::dump_generic_expr_loc): ...this new member > > function. Add the tree to any pending optinfo, creating > > one if > > need be. > > (dump_printf): Move implementation to... > > (dump_context::dump_printf_va): ...this new member > > function. Add > > the text to any pending optinfo, creating one if need be. > > (dump_printf_loc): Move implementation to... > > (dump_context::dump_printf_loc_va): ...this new member > > function. > > Convert param "loc" from location_t to const > > dump_location_t &. > > Start a new optinfo and add the stmt to it. > > (dump_dec): Move implementation to... > > (dump_context::dump_dec): ...this new member function. Add > > the > > value to any pending optinfo, creating one if need be. > > (dump_context::dump_symtab_node): New member function. > > (dump_context::get_scope_depth): New member function. > > (dump_context::begin_scope): New member function. > > (dump_context::end_scope): New member function. > > (dump_context::ensure_pending_optinfo): New member > > function. > > (dump_context::begin_next_optinfo): New member function. > > (dump_context::end_any_optinfo): New member function. > > (dump_context::s_current): New global. > > (dump_context::s_default): New global. > > (dump_scope_depth): Delete global. > > (dumpfile_ensure_any_optinfo_are_flushed): New function. > > (dump_symtab_node): New function. > > (get_dump_scope_depth): Reimplement in terms of > > dump_context.
Re: [PATCH 1/2] Add "optinfo" framework
On Mon, Jul 2, 2018 at 10:51 PM David Malcolm wrote: > > This patch implements a way to consolidate dump_* calls into > optinfo objects, as enabling work towards being able to write out > optimization records to a file, or emit them as diagnostic "remarks". > > The patch adds the support for building optinfo instances from dump_* > calls, but leaves implementing any *users* of them to followup patches. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > OK for trunk? Looks good overall, but ... To "fix" the GC issue you'd need to capture all possibly interesting information from tree/gimple while it is still in flight. This _may_ be necessary anyway since I remember writing code like fprintf (dump_file, "old: "); print_gimple_stmt (..., stmt); gimple_set_rhs1 (stmt, op); fprintf (dump_file, "new: "); print_gmple_stmt (..., stmt); fprintf (dump_file, "\n"); capturing interesting information means we know all targeted optinfo channels, right? And the optinfo consumers need to handle "streams" of input and may not look back. I've yet have to look at the 2nd patch but can you comment on this? How difficult is it to re-wire how the data flows to make stmt re-use like the above possible? Thanks, Richard. > gcc/ChangeLog: > * Makefile.in (OBJS): Add optinfo.o. > * coretypes.h (class symtab_node): New forward decl. > (struct cgraph_node): New forward decl. > (class varpool_node): New forward decl. > * dump-context.h: New file. > * dumpfile.c: Include "optinfo.h", "dump-context.h", "cgraph.h", > "tree-pass.h", "optinfo-internal.h". > (refresh_dumps_are_enabled): Use optinfo_enabled_p. > (set_dump_file): Call dumpfile_ensure_any_optinfo_are_flushed. > (set_alt_dump_file): Likewise. > (dump_context::~dump_context): New dtor. > (dump_gimple_stmt): Move implementation to... > (dump_context::dump_gimple_stmt): ...this new member function. > Add the stmt to any pending optinfo, creating one if need be. > (dump_gimple_stmt_loc): Move implementation to... > (dump_context::dump_gimple_stmt_loc): ...this new member function. > Convert param "loc" from location_t to const dump_location_t &. > Start a new optinfo and add the stmt to it. > (dump_generic_expr): Move implementation to... > (dump_context::dump_generic_expr): ...this new member function. > Add the tree to any pending optinfo, creating one if need be. > (dump_generic_expr_loc): Move implementation to... > (dump_context::dump_generic_expr_loc): ...this new member > function. Add the tree to any pending optinfo, creating one if > need be. > (dump_printf): Move implementation to... > (dump_context::dump_printf_va): ...this new member function. Add > the text to any pending optinfo, creating one if need be. > (dump_printf_loc): Move implementation to... > (dump_context::dump_printf_loc_va): ...this new member function. > Convert param "loc" from location_t to const dump_location_t &. > Start a new optinfo and add the stmt to it. > (dump_dec): Move implementation to... > (dump_context::dump_dec): ...this new member function. Add the > value to any pending optinfo, creating one if need be. > (dump_context::dump_symtab_node): New member function. > (dump_context::get_scope_depth): New member function. > (dump_context::begin_scope): New member function. > (dump_context::end_scope): New member function. > (dump_context::ensure_pending_optinfo): New member function. > (dump_context::begin_next_optinfo): New member function. > (dump_context::end_any_optinfo): New member function. > (dump_context::s_current): New global. > (dump_context::s_default): New global. > (dump_scope_depth): Delete global. > (dumpfile_ensure_any_optinfo_are_flushed): New function. > (dump_symtab_node): New function. > (get_dump_scope_depth): Reimplement in terms of dump_context. > (dump_begin_scope): Likewise. > (dump_end_scope): Likewise. > (selftest::temp_dump_context::temp_dump_context): New ctor. > (selftest::temp_dump_context::~temp_dump_context): New dtor. > (selftest::assert_is_text): New support function. > (selftest::assert_is_tree): New support function. > (selftest::assert_is_gimple): New support function. > (selftest::test_capture_of_dump_calls): New test. > (selftest::dumpfile_c_tests): Call it. > * dumpfile.h (dump_printf, dump_printf_loc, dump_basic_block, > dump_generic_expr_loc, dump_generic_expr, dump_gimple_stmt_loc, > dump_gimple_stmt, dump_dec): Gather these related decls and add a > descriptive comment. > (dump_function, print_combine_total_stats,
[PATCH 1/2] Add "optinfo" framework
This patch implements a way to consolidate dump_* calls into optinfo objects, as enabling work towards being able to write out optimization records to a file, or emit them as diagnostic "remarks". The patch adds the support for building optinfo instances from dump_* calls, but leaves implementing any *users* of them to followup patches. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. OK for trunk? gcc/ChangeLog: * Makefile.in (OBJS): Add optinfo.o. * coretypes.h (class symtab_node): New forward decl. (struct cgraph_node): New forward decl. (class varpool_node): New forward decl. * dump-context.h: New file. * dumpfile.c: Include "optinfo.h", "dump-context.h", "cgraph.h", "tree-pass.h", "optinfo-internal.h". (refresh_dumps_are_enabled): Use optinfo_enabled_p. (set_dump_file): Call dumpfile_ensure_any_optinfo_are_flushed. (set_alt_dump_file): Likewise. (dump_context::~dump_context): New dtor. (dump_gimple_stmt): Move implementation to... (dump_context::dump_gimple_stmt): ...this new member function. Add the stmt to any pending optinfo, creating one if need be. (dump_gimple_stmt_loc): Move implementation to... (dump_context::dump_gimple_stmt_loc): ...this new member function. Convert param "loc" from location_t to const dump_location_t &. Start a new optinfo and add the stmt to it. (dump_generic_expr): Move implementation to... (dump_context::dump_generic_expr): ...this new member function. Add the tree to any pending optinfo, creating one if need be. (dump_generic_expr_loc): Move implementation to... (dump_context::dump_generic_expr_loc): ...this new member function. Add the tree to any pending optinfo, creating one if need be. (dump_printf): Move implementation to... (dump_context::dump_printf_va): ...this new member function. Add the text to any pending optinfo, creating one if need be. (dump_printf_loc): Move implementation to... (dump_context::dump_printf_loc_va): ...this new member function. Convert param "loc" from location_t to const dump_location_t &. Start a new optinfo and add the stmt to it. (dump_dec): Move implementation to... (dump_context::dump_dec): ...this new member function. Add the value to any pending optinfo, creating one if need be. (dump_context::dump_symtab_node): New member function. (dump_context::get_scope_depth): New member function. (dump_context::begin_scope): New member function. (dump_context::end_scope): New member function. (dump_context::ensure_pending_optinfo): New member function. (dump_context::begin_next_optinfo): New member function. (dump_context::end_any_optinfo): New member function. (dump_context::s_current): New global. (dump_context::s_default): New global. (dump_scope_depth): Delete global. (dumpfile_ensure_any_optinfo_are_flushed): New function. (dump_symtab_node): New function. (get_dump_scope_depth): Reimplement in terms of dump_context. (dump_begin_scope): Likewise. (dump_end_scope): Likewise. (selftest::temp_dump_context::temp_dump_context): New ctor. (selftest::temp_dump_context::~temp_dump_context): New dtor. (selftest::assert_is_text): New support function. (selftest::assert_is_tree): New support function. (selftest::assert_is_gimple): New support function. (selftest::test_capture_of_dump_calls): New test. (selftest::dumpfile_c_tests): Call it. * dumpfile.h (dump_printf, dump_printf_loc, dump_basic_block, dump_generic_expr_loc, dump_generic_expr, dump_gimple_stmt_loc, dump_gimple_stmt, dump_dec): Gather these related decls and add a descriptive comment. (dump_function, print_combine_total_stats, enable_rtl_dump_file, dump_node, dump_bb): Move these unrelated decls. (class dump_manager): Add leading comment. * ggc-page.c (ggc_collect): Call dumpfile_ensure_any_optinfo_are_flushed. * optinfo-internal.h: New file. * optinfo.cc: New file. * optinfo.h: New file. * selftest-run-tests.c (selftest::run_tests): Call selftest::optinfo_cc_tests. * selftest.h (selftest::optinfo_cc_tests): New decl. --- gcc/Makefile.in | 1 + gcc/coretypes.h | 7 + gcc/dump-context.h | 128 gcc/dumpfile.c | 498 +++ gcc/dumpfile.h | 82 +--- gcc/ggc-page.c | 2 + gcc/optinfo-internal.h | 148 ++ gcc/optinfo.cc | 251 gcc/optinfo.h| 175 + gcc/selftest-run-tests.c | 1 + gcc/selftest.h |
