This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Convert more passes to new dump framework
- From: Xinliang David Li <davidxl at google dot com>
- To: Teresa Johnson <tejohnson at google dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Dehao Chen <dehao at google dot com>, Sharad Singhai <singhai at google dot com>
- Date: Tue, 6 Aug 2013 08:57:52 -0700
- Subject: Re: [PATCH] Convert more passes to new dump framework
- References: <CAAe5K+W=brFLz3kZYFXTYAE+T2yJVPF=z+vp+uKh4d+9vQXJKQ at mail dot gmail dot com> <20130806123714 dot GD3166 at virgil dot suse>
On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Mon, Aug 05, 2013 at 10:37:00PM -0700, Teresa Johnson wrote:
>> This patch ports messages to the new dump framework,
>
> It would be great this new framework was documented somewhere. I lost
> track of what was agreed it would be and from the uses in the
> vectorizer I was never quite sure how to utilize it in other passes.
Sharad, can you put the documentation in GCC wiki.
In a nutshell, the new dumping interfaces produces information notes
which have 'dual' outputs -- controlled by different options. When
-fdump-<phase>-<pass> is on, the dump info will be dumped into the
pass specific dump file, and when -fopt-info=.. is on, the information
will be dumped into stderr.
The dump call should be guarded by dump_enabled_p().
thanks,
David
>
> I'd also like to point out two other minor things inline:
>
> [...]
>
>> 2013-08-06 Teresa Johnson <tejohnson@google.com>
>> Dehao Chen <dehao@google.com>
>>
>> * dumpfile.c (dump_loc): Add column number to output, make newlines
>> consistent.
>> * dumpfile.h (OPTGROUP_OTHER): Add and enable under OPTGROUP_ALL.
>> * ipa-inline-transform.c (clone_inlined_nodes):
>> (cgraph_node_opt_info): New function.
>> (cgraph_node_call_chain): Ditto.
>> (dump_inline_decision): Ditto.
>> (inline_call): Invoke dump_inline_decision.
>> * doc/invoke.texi: Document optall -fopt-info flag.
>> * profile.c (read_profile_edge_counts): Use new dump framework.
>> (compute_branch_probabilities): Ditto.
>> * passes.c (pass_manager::register_one_dump_file): Use OPTGROUP_OTHER
>> when pass not in any opt group.
>> * value-prof.c (check_counter): Use new dump framework.
>> (find_func_by_funcdef_no): Ditto.
>> (check_ic_target): Ditto.
>> * coverage.c (get_coverage_counts): Ditto.
>> (coverage_init): Setup new dump framework.
>> * ipa-inline.c (inline_small_functions): Set is_in_ipa_inline.
>> * ipa-inline.h (is_in_ipa_inline): Declare.
>>
>> * testsuite/gcc.dg/pr40209.c: Use -fopt-info.
>> * testsuite/gcc.dg/pr26570.c: Ditto.
>> * testsuite/gcc.dg/pr32773.c: Ditto.
>> * testsuite/g++.dg/tree-ssa/dom-invalid.C (struct C): Ditto.
>>
>
> [...]
>
>> Index: ipa-inline-transform.c
>> ===================================================================
>> --- ipa-inline-transform.c (revision 201461)
>> +++ ipa-inline-transform.c (working copy)
>> @@ -192,6 +192,108 @@ clone_inlined_nodes (struct cgraph_edge *e, bool d
>> }
>>
>>
>> +#define MAX_INT_LENGTH 20
>> +
>> +/* Return NODE's name and profile count, if available. */
>> +
>> +static const char *
>> +cgraph_node_opt_info (struct cgraph_node *node)
>> +{
>> + char *buf;
>> + size_t buf_size;
>> + const char *bfd_name = lang_hooks.dwarf_name (node->symbol.decl, 0);
>> +
>> + if (!bfd_name)
>> + bfd_name = "unknown";
>> +
>> + buf_size = strlen (bfd_name) + 1;
>> + if (profile_info)
>> + buf_size += (MAX_INT_LENGTH + 3);
>> +
>> + buf = (char *) xmalloc (buf_size);
>> +
>> + strcpy (buf, bfd_name);
>> +
>> + if (profile_info)
>> + sprintf (buf, "%s ("HOST_WIDEST_INT_PRINT_DEC")", buf, node->count);
>> + return buf;
>> +}
>
> I'm not sure if output of this function is aimed only at the user or
> if it is supposed to be used by gcc developers as well. If the
> latter, an incredibly useful thing is to also dump node->symbol.order
> too. We usually dump it after "/" sign separating it from node name.
> It is invaluable when examining decisions in C++ code where you can
> have lots of clones of a node (and also because existing dumps print
> it, it is easy to combine them).
>
> [...]
>
>> Index: ipa-inline.c
>> ===================================================================
>> --- ipa-inline.c (revision 201461)
>> +++ ipa-inline.c (working copy)
>> @@ -118,6 +118,9 @@ along with GCC; see the file COPYING3. If not see
>> static int overall_size;
>> static gcov_type max_count;
>>
>> +/* Global variable to denote if it is in ipa-inline pass. */
>> +bool is_in_ipa_inline = false;
>> +
>> /* Return false when inlining edge E would lead to violating
>> limits on function unit growth or stack usage growth.
>>
>
> In this age of removing global variables, are you sure you need this?
> The only user of this seems to be a function that is only being called
> from inline_call... can that ever happen when not inlining? If you
> plan to use this function also elsewhere, perhaps the callers will
> know whether we are inlining or not and can provide this in a
> parameter?
>
> Thanks,
>
> Martin