On Wed, Oct 17, 2012 at 2:08 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Wed, Oct 17, 2012 at 9:05 AM, Xinliang David Li <davi...@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
}
thanks,
David
> Thanks,
> Richard.
>
>> David
>>
>>
>> On Tue, Oct 16, 2012 at 11:42 PM, Sharad Singhai <sing...@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
