This is the mail archive of the gcc-patches@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: Statically propagate basic blocks which are likely executed 0 times


Hi,


On 9 June 2017 at 08:43, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> On 8 June 2017 14:52:49 CEST, Jan Hubicka <hubicka@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;
>>+}
>


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