On Tue, Aug 6, 2013 at 8:57 AM, Xinliang David Li <davi...@google.com> wrote:
> On Tue, Aug 6, 2013 at 5:37 AM, Martin Jambor <mjam...@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.
Sure. I had user documentation in form of gcc info. But I will add
more developer details to a GCC wiki.
Thanks,
Sharad
> 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 <tejohn...@google.com>
>>> Dehao Chen <de...@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
