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);
}
and ends up calling evaluate_stmt most of the time:
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.
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.
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
