Hi,
On 9 June 2017 at 08:43, Bernhard Reutner-Fischer <rep.dot....@gmail.com> wrote:
> On 8 June 2017 14:52:49 CEST, Jan Hubicka <hubi...@ucw.cz> wrote:
>>Hi,
>>this patch adds static code to detect basic block with 0 execution
>>count.
>>Those are basic block either reached only by EH or those which leads to
>>call of
>>function decorated with cold attribute.
>>
>>Function decorated by noreturn is not sufficient, because exit(0) is a
>>call that
>>is executed in most of program executions.
>
> But aren't paths to exit except in main cold resp unlikely, at least exit
> (!0)?
>
>>
>>Note that internally we have cold and unlikely functions where the
>>first is
>>optimized for size but the second is known to be unlikely executed by
>>the
>>program and is offloaded to special unlikely section.
>>Perhaps we want to expose this to user and also distinguish between
>>cold and
>>unlikely function attributes. I guess this however can be done
>>incrementally.
>>
>>As a followup I will decoreate trap/abort and friends with cold
>>attributes.
>>
>>Bootstrapped/regtested x86_64-linux, will commit it shortly.
>>
>
>>+/* Return true if STMT is known to be unlikely executed. */
>>+
>>+static bool
>>+unlikely_executed_stmt_p (gimple *stmt)
>>+{
>>+ if (!is_gimple_call (stmt))
>>+ return false;
>>+ /* NORETURN attribute enough is not strong enough: exit() may be
>
> s/attribute enough/attribute alone/
>
>>quite
>>+ likely executed once during program run. */
>>+ if (gimple_call_fntype (stmt)
>>+ && lookup_attribute ("cold",
>>+ TYPE_ATTRIBUTES (gimple_call_fntype (stmt)))
>>+ && !lookup_attribute ("cold", DECL_ATTRIBUTES
>>(current_function_decl)))
>>+ return true;
>>+ tree decl = gimple_call_fndecl (stmt);
>>+ if (!decl)
>>+ return false;
>
> Can decl ever be NULL here?
>
>>+ if (lookup_attribute ("cold", DECL_ATTRIBUTES (decl))
>>+ && !lookup_attribute ("cold", DECL_ATTRIBUTES
>>(current_function_decl)))
>>+ return true;
>>+
>>+ cgraph_node *n = cgraph_node::get (decl);
>>+ if (!n)
>>+ return false;
>>+ enum availability avail;
>
> Didn't style want us to omit enum here since its C++ now?
>
>>+ n = n->ultimate_alias_target (&avail);
>>+ if (avail < AVAIL_AVAILABLE)
>>+ return NULL;
>
> false
>
> thanks,
>
Since this commit (r249013), I've noticed a regression on arm targets:
FAIL: gcc.target/arm/cold-lc.c scan-assembler-not bl[^\n]*dump_stack
>>+ if (!n->analyzed
>>+ || n->decl == current_function_decl)
>>+ return false;
>>+ return n->frequency == NODE_FREQUENCY_UNLIKELY_EXECUTED;
>>+}
>
