This is the mail archive of the gcc@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: New dump infrastructure


sounds good.

thanks,

David

On Thu, Oct 18, 2012 at 1:52 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Oct 17, 2012 at 6:55 PM, Xinliang David Li <davidxl@google.com> wrote:
>> On Wed, Oct 17, 2012 at 2:08 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> On Wed, Oct 17, 2012 at 9:05 AM, Xinliang David Li <davidxl@google.com> wrote:
>>>> A more simpler use model is not to guard the dump statement at all --
>>>> just express the intention a) what to dump; b) as what kind or to
>>>> where
>>>>
>>>>
>>>> 1) I want to dump the something as optimized message:
>>>>
>>>>   dump_printf (MSG_OPTIMIZED, "blah...")
>>>>
>>>>   dump_printf_loc (MSG_OPTIMIZED, "blah")
>>>>
>>>> 2) I want to dump something together with tree dump regardless of the flag:
>>>>
>>>>    dump_printf (TDF_TREE, ...);
>>>>
>>>> 3) I want to dump something with tree dump when detailed flags is set:
>>>>
>>>>   dump_printf (TDF_DETAILS, ...);
>>>>
>>>>
>>>> The dumping code is not in the critical path, so the compile time
>>>> should not be a concern.
>>>
>>> That's not true in all cases which is why I asked for the predicate to be
>>> available, especially for the case where it guards multiple dump_* statement.
>>>
>>> Look for example at CCPs ccp_visit_stmt which calls
>>>
>>>   if (dump_file && (dump_flags & TDF_DETAILS))
>>>     {
>>>       fprintf (dump_file, "\nVisiting statement:\n");
>>>       print_gimple_stmt (dump_file, stmt, 0, dump_flags);
>>>     }
>>>
>>
>> Yes -- that means guarding is for performance reason and can be made optional.
>>
>>> and ends up calling evaluate_stmt most of the time:
>>
>> That looks too expensive even with the guard. Can it be turned off in
>> non debug build?
>>
>>>
>>>   if (dump_file && (dump_flags & TDF_DETAILS))
>>>     {
>>>       fprintf (dump_file, "which is likely ");
>>>       switch (likelyvalue)
>>>         {
>>>         case CONSTANT:
>>>           fprintf (dump_file, "CONSTANT");
>>>           break;
>>>         case UNDEFINED:
>>>           fprintf (dump_file, "UNDEFINED");
>>>           break;
>>>         case VARYING:
>>>           fprintf (dump_file, "VARYING");
>>>           break;
>>>         default:;
>>>         }
>>>       fprintf (dump_file, "\n");
>>>     }
>>>
>>> such code is not something you'd want to do unconditionally.
>>>
>>
>> ok.
>>
>>
>>> I agree the use of the predicate can be reduced in some cases but it has
>>> to be available for cases like the above.  And eventually have a fast-path
>>> inlined like
>>>
>>> inline bool dump_kind_p (int flags)
>>> {
>>>   return any_dump_enabled_p ? dump_kind_p_1 (flags) : false;
>>> }
>>>
>>> or a new inline function
>>>
>>> inline bool dump_enabled_p ()
>>> {
>>>   return any_dump_enabled_p;
>>> }
>>>
>>> at the cost of one extra global flag we'd need to maintain.  Probably the
>>> latter would be what you prefer?  That way we don't get redundant
>>> or even conflicting flags on the predicate check vs. the dump stmts.
>>>
>>
>> Another way:
>>
>> To make the dumping code as concise as possible, we can make
>> dump_printf a inlinable wrapper:
>>
>> inline void dump_printf (...)
>> {
>>    if (!any_dump_enabled_p)
>>       return;
>>
>>   // regular stuff
>> }
>
> You still have the issue that // regular stuff may appear to possibly
> clobber any_dump_enabled_p and thus repeated any_dump_enabled_p
> checks are not optimized by the compiler (we don't have predicated
> value-numbering (yet)).
>
> So I prefer the guard.  I suppose after this discussion I prefer
> a any_dump_enabled_p () guard instead of one with (repeated) flags.
>
> Richard.
>
>> thanks,
>>
>> David
>>
>>
>>> Thanks,
>>> Richard.
>>>
>>>> David
>>>>
>>>>
>>>> On Tue, Oct 16, 2012 at 11:42 PM, Sharad Singhai <singhai@google.com> wrote:
>>>>>> 1. OK, I understand that e.g.
>>>>>>
>>>>>>      if (dump_file && (dump_flags & TDF_DETAILS))
>>>>>>
>>>>>>    should be converted into:
>>>>>>
>>>>>>      if (dump_kind_p (TDF_DETAILS))
>>>>>>
>>>>>>    But what about current code that does not care about dump_flags?
>>>>>>    E.g. converting simple
>>>>>>
>>>>>>      if (dump_file)
>>>>>>
>>>>>>    to
>>>>>>
>>>>>>      if (dump_kind_p (0))
>>>>>>
>>>>>>    won't work, dump_kind_p will always return zero in such cases.
>>>>>
>>>>>
>>>>> Yes, you are right, the conversion is not completely mechanical and
>>>>> some care must be taken to preserve the original intent. I think one
>>>>> of the following might work in the case where a pass doesn't care
>>>>> about the dump_flags
>>>>>
>>>>> 1. use generic pass type flags, such as TDF_TREE, TDF_IPA, TDF_RTL
>>>>> which are guaranteed to be set depending on the pass type,
>>>>> 2. this dump statement might be a better fit for MSG_* flags if it
>>>>> deals with optimizations. Sometimes "if (dump_file) fpritnf
>>>>> (dump_file, ...)" idiom was used for these situations and now these
>>>>> sites might be perfect candidate for converting into MSG_* flags.
>>>>>
>>>>> If a cleaner way to handle this is desired, perhaps we can add an
>>>>> unconditional "dump_printf_always (...)", but currently it seems
>>>>> unnecessary. Note that for optimization messages which should always
>>>>> be printed, one can use MSG_ALL flag. However, no analogous flag
>>>>> exists for regular dumps. How about adding a corresponding TDF_ALL
>>>>> flag? Would that work?
>>>>>
>>>>>>
>>>>>>
>>>>>> 2. dump_kind_p seems to always return 0 if current_function_decl is
>>>>>>    NULL.  However, that precludes its use in IPA passes in which this
>>>>>>    can happen regularly.  Why is this restriction necessary?
>>>>>
>>>>>
>>>>> This is an oversight on my part. Originally, I wanted to be able to
>>>>> print source location information and this is a remnant of that. I am
>>>>> testing a patch to fix that.
>>>>>
>>>>> Thanks,
>>>>> Sharad


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