Fix removal of ctors/dtors

Fix removal of ctors/dtors

[Jan Hubicka]

Hi,
while implementing code removing pure & const constructors I considered correct option of
dropping CONSTRUCTOR/DESTRUCTOR flag for functions found to be const or pure or slightly hacky
of simply teaching dead function removal to consider them dead.  The second is suboptimal
for static constructors & destructors that are also called dirrectly.  I concluded that no one
does that, but Zdenek did and it triggers one overparanoid sanity check.

So this patch takes the cleaner route of just dropping the flags and letting
unreachable code removal to remove the functions later.  This happens in
visibility pass where we sanitize flags for pure/const constructors comming
from forntends or via cgraph_set_pure_flag/cgraph_set_const_flags when they are
discovered pure/const later.

While working on it I also noticed that we don't handle correctly extern
declarations (we consider them extern inlines and keep them in the cgraph.
This is wasteful especially for LTO) and extern inlines (we keep them in the
program even if they are obviously dead - they are never called nor have address
taken up to the IPA inliner). Cleaning this issue shown that both inliner and
ipa-cp compute code size costs incorrectly by assuming that optimizing out
external copy of extern function it will save code size.

Finally both inliner and ipa-cp confuse handling of functions with address
taken.  ipa-cp is considering functions with address taken to be cloning candidates.
This is wrong.  We should clone only when we think original function is not called at all
but we can't say for sure since it is externally visible.  Clonning is of course possible,
but current impementation leads to wrong code when updating call sites of calls promoted to
be direct.

Inliner on the other hand produce invalid callgraph with inline clones having
address taken.

Bootstrapped/regtested x86_64-linux, lto-bootstrapped x86_64-linux and tested
with Firefox builds.   I also tested that tramp3d does not care about the
inliner mertric fixes.

Honza

	* cgraph.c (cgraph_set_readonly_flag): Rename to...
	(cgraph_set_const_flags) ... this one; get also looping argument;
	clear constructor/destructor flags.
	(cgraph_set_pure_flag): Likewise.
	(cgraph_set_looping_const_or_pure_flag): Remove.
	(cgraph_can_remove_if_no_direct_calls_and_refs): Do not try
	to optimize away static ctors/dtors; it does not work on inline clones;
	external functions can always be rmeoved.
	(cgraph_will_be_removed_from_program_if_no_direct_calls): Assert on inline
	clones; in LTO external functions always can go.
	(cgraph_used_from_object_file_p): Handle EXTERNAL functions correctly.
	(cgraph_mark_address_taken_node): Assert that we are not taking address of
	inline clone.
	(cgraph_can_remove_if_no_direct_calls_p): We always eventually remove
	external functions.
	* ipa-cp.c (ipcp_cloning_candidate_p): Do not clone functions with address taken.
	(ipcp_initialize_node_lattices): Only local functions can be handled without cloning.
	* cgraph.h (cgraph_set_readonly_flag,
	cgraph_set_looping_const_or_pure_flag): Remove.
	(cgraph_set_const_flag): Declare.
	(cgraph_set_pure_flag): Update.
	* ipa-pure-const (propagate_pure_const, local_pure_const): Update
	flags setting code.
	* ipa.c (cgraph_remove_unreachable_nodes): Fix formating; do not look at inline
	clones; fix handling of external definitions.
	(cgraph_postorder): Do not look at inline clones in the first pass.
	(function_and_variable_visibility): Drop constructors/destructor
	flags at pure and const functions.
	* tree-profile.c (tree_profiling): Update.
	* ipa-inline.c (cgraph_clone_inlined_nodes): Always clone functions with
	address taken; external functions do not account to whole program size.
	(cgraph_decide_inlining): Likewise; do not try to inline functions already
	inlined.

	* testsuite/gcc.dg/lto/pr45736_0.c: New function.
Index: cgraph.c
===================================================================
*** cgraph.c	(revision 165925)
--- cgraph.c	(working copy)
*************** cgraph_mark_needed_node (struct cgraph_n
*** 1723,1728 ****
--- 1723,1729 ----
  void
  cgraph_mark_address_taken_node (struct cgraph_node *node)
  {
+   gcc_assert (!node->global.inlined_to);
    cgraph_mark_reachable_node (node);
    node->address_taken = 1;
  }
*************** cgraph_set_nothrow_flag (struct cgraph_n
*** 2591,2627 ****
     if any to READONLY.  */
  
  void
! cgraph_set_readonly_flag (struct cgraph_node *node, bool readonly)
  {
    struct cgraph_node *alias;
    TREE_READONLY (node->decl) = readonly;
    for (alias = node->same_body; alias; alias = alias->next)
!     TREE_READONLY (alias->decl) = readonly;
  }
  
  /* Set DECL_PURE_P on NODE's decl and on same_body aliases of NODE
     if any to PURE.  */
  
  void
! cgraph_set_pure_flag (struct cgraph_node *node, bool pure)
  {
    struct cgraph_node *alias;
    DECL_PURE_P (node->decl) = pure;
    for (alias = node->same_body; alias; alias = alias->next)
!     DECL_PURE_P (alias->decl) = pure;
! }
! 
! /* Set DECL_LOOPING_CONST_OR_PURE_P on NODE's decl and on
!    same_body aliases of NODE if any to LOOPING_CONST_OR_PURE.  */
! 
! void
! cgraph_set_looping_const_or_pure_flag (struct cgraph_node *node,
! 				       bool looping_const_or_pure)
! {
!   struct cgraph_node *alias;
!   DECL_LOOPING_CONST_OR_PURE_P (node->decl) = looping_const_or_pure;
!   for (alias = node->same_body; alias; alias = alias->next)
!     DECL_LOOPING_CONST_OR_PURE_P (alias->decl) = looping_const_or_pure;
  }
  
  /* See if the frequency of NODE can be updated based on frequencies of its
--- 2592,2641 ----
     if any to READONLY.  */
  
  void
! cgraph_set_const_flag (struct cgraph_node *node, bool readonly, bool looping)
  {
    struct cgraph_node *alias;
+   /* Static constructors and destructors without a side effect can be
+      optimized out.  */
+   if (!looping && readonly)
+     {
+       if (DECL_STATIC_CONSTRUCTOR (node->decl))
+ 	DECL_STATIC_CONSTRUCTOR (node->decl) = 0;
+       if (DECL_STATIC_DESTRUCTOR (node->decl))
+ 	DECL_STATIC_DESTRUCTOR (node->decl) = 0;
+     }
    TREE_READONLY (node->decl) = readonly;
+   DECL_LOOPING_CONST_OR_PURE_P (node->decl) = looping;
    for (alias = node->same_body; alias; alias = alias->next)
!     {
!       TREE_READONLY (alias->decl) = readonly;
!       DECL_LOOPING_CONST_OR_PURE_P (alias->decl) = looping;
!     }
  }
  
  /* Set DECL_PURE_P on NODE's decl and on same_body aliases of NODE
     if any to PURE.  */
  
  void
! cgraph_set_pure_flag (struct cgraph_node *node, bool pure, bool looping)
  {
    struct cgraph_node *alias;
+   /* Static constructors and destructors without a side effect can be
+      optimized out.  */
+   if (!looping && pure)
+     {
+       if (DECL_STATIC_CONSTRUCTOR (node->decl))
+ 	DECL_STATIC_CONSTRUCTOR (node->decl) = 0;
+       if (DECL_STATIC_DESTRUCTOR (node->decl))
+ 	DECL_STATIC_DESTRUCTOR (node->decl) = 0;
+     }
    DECL_PURE_P (node->decl) = pure;
+   DECL_LOOPING_CONST_OR_PURE_P (node->decl) = looping;
    for (alias = node->same_body; alias; alias = alias->next)
!     {
!       DECL_PURE_P (alias->decl) = pure;
!       DECL_LOOPING_CONST_OR_PURE_P (alias->decl) = looping;
!     }
  }
  
  /* See if the frequency of NODE can be updated based on frequencies of its
*************** cgraph_edge_cannot_lead_to_return (struc
*** 2768,2792 ****
  bool
  cgraph_can_remove_if_no_direct_calls_and_refs_p (struct cgraph_node *node)
  {
    /* When function is needed, we can not remove it.  */
    if (node->needed || node->reachable_from_other_partition)
      return false;
    /* Only COMDAT functions can be removed if externally visible.  */
    if (node->local.externally_visible
        && (!DECL_COMDAT (node->decl)
  	  || cgraph_used_from_object_file_p (node)))
      return false;
-   /* Constructors and destructors are executed by the runtime, however
-      we can get rid of all pure constructors and destructors.  */
-   if (DECL_STATIC_CONSTRUCTOR (node->decl)
-       || DECL_STATIC_DESTRUCTOR (node->decl))
-     {
-       int flags = flags_from_decl_or_type (node->decl);
-       if (!optimize
- 	  || !(flags & (ECF_CONST | ECF_PURE))
- 	  || (flags & ECF_LOOPING_CONST_OR_PURE))
- 	return false;
-     }
    return true;
  }
  
--- 2782,2802 ----
  bool
  cgraph_can_remove_if_no_direct_calls_and_refs_p (struct cgraph_node *node)
  {
+   gcc_assert (!node->global.inlined_to);
+   /* Extern inlines can always go, we will use the external definition.  */
+   if (DECL_EXTERNAL (node->decl))
+     return true;
    /* When function is needed, we can not remove it.  */
    if (node->needed || node->reachable_from_other_partition)
      return false;
+   if (DECL_STATIC_CONSTRUCTOR (node->decl)
+       || DECL_STATIC_DESTRUCTOR (node->decl))
+     return false;
    /* Only COMDAT functions can be removed if externally visible.  */
    if (node->local.externally_visible
        && (!DECL_COMDAT (node->decl)
  	  || cgraph_used_from_object_file_p (node)))
      return false;
    return true;
  }
  
*************** cgraph_can_remove_if_no_direct_calls_and
*** 2807,2818 ****
  bool
  cgraph_will_be_removed_from_program_if_no_direct_calls (struct cgraph_node *node)
  {
    if (cgraph_used_from_object_file_p (node))
      return false;
    if (!in_lto_p && !flag_whole_program)
      return cgraph_only_called_directly_p (node);
    else
!     return cgraph_can_remove_if_no_direct_calls_p (node);
  }
  
  /* Return true when RESOLUTION indicate that linker will use
--- 2817,2833 ----
  bool
  cgraph_will_be_removed_from_program_if_no_direct_calls (struct cgraph_node *node)
  {
+   gcc_assert (!node->global.inlined_to);
    if (cgraph_used_from_object_file_p (node))
      return false;
    if (!in_lto_p && !flag_whole_program)
      return cgraph_only_called_directly_p (node);
    else
!     {
!        if (DECL_EXTERNAL (node->decl))
!          return true;
!       return cgraph_can_remove_if_no_direct_calls_p (node);
!     }
  }
  
  /* Return true when RESOLUTION indicate that linker will use
*************** cgraph_used_from_object_file_p (struct c
*** 2835,2841 ****
  {
    struct cgraph_node *alias;
  
!   if (!TREE_PUBLIC (node->decl))
      return false;
    if (resolution_used_from_other_file_p (node->resolution))
      return true;
--- 2850,2857 ----
  {
    struct cgraph_node *alias;
  
!   gcc_assert (!node->global.inlined_to);
!   if (!TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node->decl))
      return false;
    if (resolution_used_from_other_file_p (node->resolution))
      return true;
Index: cgraph.h
===================================================================
*** cgraph.h	(revision 165925)
--- cgraph.h	(working copy)
*************** struct cgraph_node * cgraph_create_virtu
*** 602,610 ****
  						  const char *clone_name);
  
  void cgraph_set_nothrow_flag (struct cgraph_node *, bool);
! void cgraph_set_readonly_flag (struct cgraph_node *, bool);
! void cgraph_set_pure_flag (struct cgraph_node *, bool);
! void cgraph_set_looping_const_or_pure_flag (struct cgraph_node *, bool);
  tree clone_function_name (tree decl, const char *);
  bool cgraph_node_cannot_return (struct cgraph_node *);
  bool cgraph_edge_cannot_lead_to_return (struct cgraph_edge *);
--- 602,609 ----
  						  const char *clone_name);
  
  void cgraph_set_nothrow_flag (struct cgraph_node *, bool);
! void cgraph_set_const_flag (struct cgraph_node *, bool, bool);
! void cgraph_set_pure_flag (struct cgraph_node *, bool, bool);
  tree clone_function_name (tree decl, const char *);
  bool cgraph_node_cannot_return (struct cgraph_node *);
  bool cgraph_edge_cannot_lead_to_return (struct cgraph_edge *);
*************** varpool_node_set_nonempty_p (varpool_nod
*** 909,914 ****
--- 908,914 ----
  static inline bool
  cgraph_only_called_directly_p (struct cgraph_node *node)
  {
+   gcc_assert (!node->global.inlined_to);
    return (!node->needed && !node->address_taken
  	  && !node->reachable_from_other_partition
  	  && !DECL_STATIC_CONSTRUCTOR (node->decl)
*************** cgraph_only_called_directly_p (struct cg
*** 922,927 ****
--- 922,930 ----
  static inline bool
  cgraph_can_remove_if_no_direct_calls_p (struct cgraph_node *node)
  {
+   /* Extern inlines can always go, we will use the external definition.  */
+   if (DECL_EXTERNAL (node->decl))
+     return true;
    return !node->address_taken && cgraph_can_remove_if_no_direct_calls_and_refs_p (node);
  }
  
Index: ipa-cp.c
===================================================================
*** ipa-cp.c	(revision 165925)
--- ipa-cp.c	(working copy)
*************** ipcp_cloning_candidate_p (struct cgraph_
*** 457,462 ****
--- 457,471 ----
    if (cgraph_only_called_directly_p (node) || !node->analyzed)
      return false;
  
+   /* When function address is taken, we are pretty sure it will be called in hidden way.  */
+   if (node->address_taken)
+     {
+       if (dump_file)
+         fprintf (dump_file, "Not considering %s for cloning; address is taken.\n",
+  	         cgraph_node_name (node));
+       return false;
+     }
+ 
    if (cgraph_function_body_availability (node) <= AVAIL_OVERWRITABLE)
      {
        if (dump_file)
*************** ipcp_initialize_node_lattices (struct cg
*** 561,567 ****
  
    if (ipa_is_called_with_var_arguments (info))
      type = IPA_BOTTOM;
!   else if (cgraph_only_called_directly_p (node))
      type = IPA_TOP;
    /* When cloning is allowed, we can assume that externally visible functions
       are not called.  We will compensate this by cloning later.  */
--- 570,576 ----
  
    if (ipa_is_called_with_var_arguments (info))
      type = IPA_BOTTOM;
!   else if (node->local.local)
      type = IPA_TOP;
    /* When cloning is allowed, we can assume that externally visible functions
       are not called.  We will compensate this by cloning later.  */
Index: testsuite/gcc.dg/lto/pr45736_0.c
===================================================================
*** testsuite/gcc.dg/lto/pr45736_0.c	(revision 0)
--- testsuite/gcc.dg/lto/pr45736_0.c	(revision 0)
***************
*** 0 ****
--- 1,16 ----
+ /* { dg-lto-do link } */
+ /* { dg-lto-options {{-flto -r -nostdlib -O}} } */
+ 
+ extern void baz (void);
+ 
+ static void __attribute__ ((constructor))
+ bar (void)
+ {
+   baz ();
+ }
+ 
+ void
+ foo (void)
+ {
+   bar ();
+ }
Index: ipa-pure-const.c
===================================================================
*** ipa-pure-const.c	(revision 165925)
--- ipa-pure-const.c	(working copy)
*************** propagate_pure_const (void)
*** 1322,1329 ****
  			     this_looping ? "looping " : "",
  			     cgraph_node_name (w));
  		}
! 	      cgraph_set_readonly_flag (w, true);
! 	      cgraph_set_looping_const_or_pure_flag (w, this_looping);
  	      break;
  
  	    case IPA_PURE:
--- 1322,1328 ----
  			     this_looping ? "looping " : "",
  			     cgraph_node_name (w));
  		}
! 	      cgraph_set_const_flag (w, true, this_looping);
  	      break;
  
  	    case IPA_PURE:
*************** propagate_pure_const (void)
*** 1335,1342 ****
  			     this_looping ? "looping " : "",
  			     cgraph_node_name (w));
  		}
! 	      cgraph_set_pure_flag (w, true);
! 	      cgraph_set_looping_const_or_pure_flag (w, this_looping);
  	      break;
  
  	    default:
--- 1334,1340 ----
  			     this_looping ? "looping " : "",
  			     cgraph_node_name (w));
  		}
! 	      cgraph_set_pure_flag (w, true, this_looping);
  	      break;
  
  	    default:
*************** local_pure_const (void)
*** 1599,1606 ****
  	  warn_function_const (current_function_decl, !l->looping);
  	  if (!skip)
  	    {
! 	      cgraph_set_readonly_flag (node, true);
! 	      cgraph_set_looping_const_or_pure_flag (node, l->looping);
  	      changed = true;
  	    }
  	  if (dump_file)
--- 1597,1603 ----
  	  warn_function_const (current_function_decl, !l->looping);
  	  if (!skip)
  	    {
! 	      cgraph_set_const_flag (node, true, l->looping);
  	      changed = true;
  	    }
  	  if (dump_file)
*************** local_pure_const (void)
*** 1614,1620 ****
  	{
  	  if (!skip)
  	    {
! 	      cgraph_set_looping_const_or_pure_flag (node, false);
  	      changed = true;
  	    }
  	  if (dump_file)
--- 1611,1617 ----
  	{
  	  if (!skip)
  	    {
! 	      cgraph_set_const_flag (node, true, false);
  	      changed = true;
  	    }
  	  if (dump_file)
*************** local_pure_const (void)
*** 1629,1636 ****
  	{
  	  if (!skip)
  	    {
! 	      cgraph_set_pure_flag (node, true);
! 	      cgraph_set_looping_const_or_pure_flag (node, l->looping);
  	      changed = true;
  	    }
  	  warn_function_pure (current_function_decl, !l->looping);
--- 1626,1632 ----
  	{
  	  if (!skip)
  	    {
! 	      cgraph_set_pure_flag (node, true, l->looping);
  	      changed = true;
  	    }
  	  warn_function_pure (current_function_decl, !l->looping);
*************** local_pure_const (void)
*** 1645,1651 ****
  	{
  	  if (!skip)
  	    {
! 	      cgraph_set_looping_const_or_pure_flag (node, false);
  	      changed = true;
  	    }
  	  if (dump_file)
--- 1641,1647 ----
  	{
  	  if (!skip)
  	    {
! 	      cgraph_set_pure_flag (node, true, false);
  	      changed = true;
  	    }
  	  if (dump_file)
Index: ipa-inline.c
===================================================================
*** ipa-inline.c	(revision 165925)
--- ipa-inline.c	(working copy)
*************** cgraph_clone_inlined_nodes (struct cgrap
*** 250,255 ****
--- 250,259 ----
        /* We may eliminate the need for out-of-line copy to be output.
  	 In that case just go ahead and re-use it.  */
        if (!e->callee->callers->next_caller
+ 	  /* FIXME: When address is taken of DECL_EXTERNAL function we still can remove its
+ 	     offline copy, but we would need to keep unanalyzed node in the callgraph so
+ 	     references can point to it.  */
+ 	  && !e->callee->address_taken
  	  && cgraph_can_remove_if_no_direct_calls_p (e->callee)
  	  /* Inlining might enable more devirtualizing, so we want to remove
  	     those only after all devirtualizable virtual calls are processed.
*************** cgraph_clone_inlined_nodes (struct cgrap
*** 264,270 ****
  	  && !cgraph_new_nodes)
  	{
  	  gcc_assert (!e->callee->global.inlined_to);
! 	  if (e->callee->analyzed)
  	    {
  	      overall_size -= e->callee->global.size;
  	      nfunctions_inlined++;
--- 268,274 ----
  	  && !cgraph_new_nodes)
  	{
  	  gcc_assert (!e->callee->global.inlined_to);
! 	  if (e->callee->analyzed && !DECL_EXTERNAL (e->callee->decl))
  	    {
  	      overall_size -= e->callee->global.size;
  	      nfunctions_inlined++;
*************** cgraph_decide_inlining (void)
*** 1449,1455 ****
  	struct cgraph_edge *e;
  
  	gcc_assert (inline_summary (node)->self_size == node->global.size);
! 	initial_size += node->global.size;
  	for (e = node->callees; e; e = e->next_callee)
  	  if (max_count < e->count)
  	    max_count = e->count;
--- 1453,1460 ----
  	struct cgraph_edge *e;
  
  	gcc_assert (inline_summary (node)->self_size == node->global.size);
! 	if (!DECL_EXTERNAL (node->decl))
! 	  initial_size += node->global.size;
  	for (e = node->callees; e; e = e->next_callee)
  	  if (max_count < e->count)
  	    max_count = e->count;
*************** cgraph_decide_inlining (void)
*** 1512,1517 ****
--- 1517,1523 ----
  
  	  if (node->callers
  	      && !node->callers->next_caller
+ 	      && !node->global.inlined_to
  	      && cgraph_will_be_removed_from_program_if_no_direct_calls (node)
  	      && node->local.inlinable
  	      && cgraph_function_body_availability (node) >= AVAIL_AVAILABLE
Index: ipa.c
===================================================================
*** ipa.c	(revision 165925)
--- ipa.c	(working copy)
*************** cgraph_postorder (struct cgraph_node **o
*** 57,64 ****
      for (node = cgraph_nodes; node; node = node->next)
        if (!node->aux
  	  && (pass
! 	      || (!cgraph_only_called_directly_p (node)
! 	  	  && !node->address_taken)))
  	{
  	  node2 = node;
  	  if (!node->callers)
--- 57,65 ----
      for (node = cgraph_nodes; node; node = node->next)
        if (!node->aux
  	  && (pass
! 	      || (!node->address_taken
! 		  && !node->global.inlined_to
! 		  && !cgraph_only_called_directly_p (node))))
  	{
  	  node2 = node;
  	  if (!node->callers)
*************** cgraph_remove_unreachable_nodes (bool be
*** 237,251 ****
      gcc_assert (!vnode->aux);
  #endif
    varpool_reset_queue ();
    for (node = cgraph_nodes; node; node = node->next)
!     if ((!cgraph_can_remove_if_no_direct_calls_and_refs_p (node)
! 	 /* Keep around virtual functions for possible devirtualization.  */
! 	 || (!before_inlining_p
! 	     && !node->global.inlined_to
! 	     && DECL_VIRTUAL_P (node->decl)
! 	     && (DECL_COMDAT (node->decl) || DECL_EXTERNAL (node->decl))))
! 	&& ((!DECL_EXTERNAL (node->decl))
!             || before_inlining_p))
        {
          gcc_assert (!node->global.inlined_to);
  	enqueue_cgraph_node (node, &first);
--- 238,259 ----
      gcc_assert (!vnode->aux);
  #endif
    varpool_reset_queue ();
+   /* Mark functions whose bodies are obviously needed.
+      This is mostly when they can be referenced externally.  Inline clones
+      are special since their declarations are shared with master clone and thus
+      cgraph_can_remove_if_no_direct_calls_and_refs_p should not be called on them.  */
    for (node = cgraph_nodes; node; node = node->next)
!     if (node->analyzed && !node->global.inlined_to
! 	&& (!cgraph_can_remove_if_no_direct_calls_and_refs_p (node)
! 	    /* Keep around virtual functions for possible devirtualization.  */
! 	    || (before_inlining_p
! 		&& DECL_VIRTUAL_P (node->decl)
! 		&& (DECL_COMDAT (node->decl) || DECL_EXTERNAL (node->decl)))
! 	    /* Also external functions with address taken are better to stay
! 	       for indirect inlining.  */
! 	    || (before_inlining_p
! 		&& DECL_EXTERNAL (node->decl)
! 		&& node->address_taken)))
        {
          gcc_assert (!node->global.inlined_to);
  	enqueue_cgraph_node (node, &first);
*************** cgraph_remove_unreachable_nodes (bool be
*** 256,261 ****
--- 264,271 ----
          gcc_assert (!node->aux);
  	node->reachable = false;
        }
+ 
+   /* Mark variables that are obviously needed.  */
    for (vnode = varpool_nodes; vnode; vnode = vnode->next)
      {
        vnode->next_needed = NULL;
*************** cgraph_remove_unreachable_nodes (bool be
*** 428,437 ****
  			node->clone_of->clones = node->next_sibling_clone;
  		      if (node->next_sibling_clone)
  			node->next_sibling_clone->prev_sibling_clone = node->prev_sibling_clone;
!     #ifdef ENABLE_CHECKING
  		      if (node->clone_of)
  			node->former_clone_of = node->clone_of->decl;
!     #endif
  		      node->clone_of = NULL;
  		      node->next_sibling_clone = NULL;
  		      node->prev_sibling_clone = NULL;
--- 438,447 ----
  			node->clone_of->clones = node->next_sibling_clone;
  		      if (node->next_sibling_clone)
  			node->next_sibling_clone->prev_sibling_clone = node->prev_sibling_clone;
! #ifdef ENABLE_CHECKING
  		      if (node->clone_of)
  			node->former_clone_of = node->clone_of->decl;
! #endif
  		      node->clone_of = NULL;
  		      node->next_sibling_clone = NULL;
  		      node->prev_sibling_clone = NULL;
*************** function_and_variable_visibility (bool w
*** 771,776 ****
--- 781,795 ----
  
    for (node = cgraph_nodes; node; node = node->next)
      {
+       int flags = flags_from_decl_or_type (node->decl);
+       if (optimize
+ 	  && (flags & (ECF_CONST | ECF_PURE))
+ 	  && !(flags & ECF_LOOPING_CONST_OR_PURE))
+ 	{
+ 	  DECL_STATIC_CONSTRUCTOR (node->decl) = 0;
+ 	  DECL_STATIC_DESTRUCTOR (node->decl) = 0;
+ 	}
+ 
        /* C++ FE on lack of COMDAT support create local COMDAT functions
  	 (that ought to be shared but can not due to object format
  	 limitations).  It is neccesary to keep the flag to make rest of C++ FE
Index: tree-profile.c
===================================================================
*** tree-profile.c	(revision 165925)
--- tree-profile.c	(working copy)
*************** tree_profiling (void)
*** 510,518 ****
  	  || DECL_STRUCT_FUNCTION (node->decl)->after_tree_profile)
  	continue;
  
!       cgraph_set_readonly_flag (node, false);
!       cgraph_set_pure_flag (node, false);
!       cgraph_set_looping_const_or_pure_flag (node, false);
      }
  
    /* Update call statements and rebuild the cgraph.  */
--- 510,517 ----
  	  || DECL_STRUCT_FUNCTION (node->decl)->after_tree_profile)
  	continue;
  
!       cgraph_set_const_flag (node, false, false);
!       cgraph_set_pure_flag (node, false, false);
      }
  
    /* Update call statements and rebuild the cgraph.  */

Re: Fix removal of ctors/dtors

[Martin Jambor]

Hi,

On Tue, Oct 26, 2010 at 04:16:04PM +0200, Jan Hubicka wrote:
> Finally both inliner and ipa-cp confuse handling of functions with
> address taken.  ipa-cp is considering functions with address taken
> to be cloning candidates.  This is wrong.  We should clone only when
> we think original function is not called at all but we can't say for
> sure since it is externally visible.  Clonning is of course
> possible, but current impementation leads to wrong code when
> updating call sites of calls promoted to be direct.
> 

We have had similar issues before but so far managed to get around
them without such a big hammer.  Do you have a testcase that will fail
if I remove the ipa-cp hunk?

Thanks,

Martin


> Bootstrapped/regtested x86_64-linux, lto-bootstrapped x86_64-linux and tested
> with Firefox builds.   I also tested that tramp3d does not care about the
> inliner mertric fixes.
> 
> Honza
> 
> 	* cgraph.c (cgraph_set_readonly_flag): Rename to...
> 	(cgraph_set_const_flags) ... this one; get also looping argument;
> 	clear constructor/destructor flags.
> 	(cgraph_set_pure_flag): Likewise.
> 	(cgraph_set_looping_const_or_pure_flag): Remove.
> 	(cgraph_can_remove_if_no_direct_calls_and_refs): Do not try
> 	to optimize away static ctors/dtors; it does not work on inline clones;
> 	external functions can always be rmeoved.
> 	(cgraph_will_be_removed_from_program_if_no_direct_calls): Assert on inline
> 	clones; in LTO external functions always can go.
> 	(cgraph_used_from_object_file_p): Handle EXTERNAL functions correctly.
> 	(cgraph_mark_address_taken_node): Assert that we are not taking address of
> 	inline clone.
> 	(cgraph_can_remove_if_no_direct_calls_p): We always eventually remove
> 	external functions.
> 	* ipa-cp.c (ipcp_cloning_candidate_p): Do not clone functions with address taken.
> 	(ipcp_initialize_node_lattices): Only local functions can be handled without cloning.
> 	* cgraph.h (cgraph_set_readonly_flag,
> 	cgraph_set_looping_const_or_pure_flag): Remove.
> 	(cgraph_set_const_flag): Declare.
> 	(cgraph_set_pure_flag): Update.
> 	* ipa-pure-const (propagate_pure_const, local_pure_const): Update
> 	flags setting code.
> 	* ipa.c (cgraph_remove_unreachable_nodes): Fix formating; do not look at inline
> 	clones; fix handling of external definitions.
> 	(cgraph_postorder): Do not look at inline clones in the first pass.
> 	(function_and_variable_visibility): Drop constructors/destructor
> 	flags at pure and const functions.
> 	* tree-profile.c (tree_profiling): Update.
> 	* ipa-inline.c (cgraph_clone_inlined_nodes): Always clone functions with
> 	address taken; external functions do not account to whole program size.
> 	(cgraph_decide_inlining): Likewise; do not try to inline functions already
> 	inlined.
> 
> 	* testsuite/gcc.dg/lto/pr45736_0.c: New function.

Re: Fix removal of ctors/dtors

[Jan Hubicka]

> Hi,
> 
> On Tue, Oct 26, 2010 at 04:16:04PM +0200, Jan Hubicka wrote:
> > Finally both inliner and ipa-cp confuse handling of functions with
> > address taken.  ipa-cp is considering functions with address taken
> > to be cloning candidates.  This is wrong.  We should clone only when
> > we think original function is not called at all but we can't say for
> > sure since it is externally visible.  Clonning is of course
> > possible, but current impementation leads to wrong code when
> > updating call sites of calls promoted to be direct.
> > 
> 
> We have had similar issues before but so far managed to get around
> them without such a big hammer.  Do you have a testcase that will fail
> if I remove the ipa-cp hunk?

The problem I ran into was that ipa-cp tried to propagate across function with
adress taken without clonning it.  This is obviously wrong, so the first part
of ipa-cp change (reducing non-clonning propagation to local function only) is
IMO only correct way.

We can support clonning functions with address taken, but I am not sure how
desriable it is: I think the idea of current ipa-cp implementation is to clone
only when all calls in current unit have same constant operand. Taking address
imply an indirect call where we know nothing.  So clonning functions with
address taken is more work for the clonning pass we don't have (yet:).

You might try to revert the second change and ensure that we do update edges
correctly when ipa-cp promote indirect call to dirrect call.

Honza

Re: Fix removal of ctors/dtors

[H.J. Lu]

On Tue, Oct 26, 2010 at 7:16 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> while implementing code removing pure & const constructors I considered correct option of
> dropping CONSTRUCTOR/DESTRUCTOR flag for functions found to be const or pure or slightly hacky
> of simply teaching dead function removal to consider them dead.  The second is suboptimal
> for static constructors & destructors that are also called dirrectly.  I concluded that no one
> does that, but Zdenek did and it triggers one overparanoid sanity check.
>
> So this patch takes the cleaner route of just dropping the flags and letting
> unreachable code removal to remove the functions later.  This happens in
> visibility pass where we sanitize flags for pure/const constructors comming
> from forntends or via cgraph_set_pure_flag/cgraph_set_const_flags when they are
> discovered pure/const later.
>
> While working on it I also noticed that we don't handle correctly extern
> declarations (we consider them extern inlines and keep them in the cgraph.
> This is wasteful especially for LTO) and extern inlines (we keep them in the
> program even if they are obviously dead - they are never called nor have address
> taken up to the IPA inliner). Cleaning this issue shown that both inliner and
> ipa-cp compute code size costs incorrectly by assuming that optimizing out
> external copy of extern function it will save code size.
>
> Finally both inliner and ipa-cp confuse handling of functions with address
> taken.  ipa-cp is considering functions with address taken to be cloning candidates.
> This is wrong.  We should clone only when we think original function is not called at all
> but we can't say for sure since it is externally visible.  Clonning is of course possible,
> but current impementation leads to wrong code when updating call sites of calls promoted to
> be direct.
>
> Inliner on the other hand produce invalid callgraph with inline clones having
> address taken.
>
> Bootstrapped/regtested x86_64-linux, lto-bootstrapped x86_64-linux and tested
> with Firefox builds.   I also tested that tramp3d does not care about the
> inliner mertric fixes.
>
> Honza
>
>        * cgraph.c (cgraph_set_readonly_flag): Rename to...
>        (cgraph_set_const_flags) ... this one; get also looping argument;
>        clear constructor/destructor flags.
>        (cgraph_set_pure_flag): Likewise.
>        (cgraph_set_looping_const_or_pure_flag): Remove.
>        (cgraph_can_remove_if_no_direct_calls_and_refs): Do not try
>        to optimize away static ctors/dtors; it does not work on inline clones;
>        external functions can always be rmeoved.
>        (cgraph_will_be_removed_from_program_if_no_direct_calls): Assert on inline
>        clones; in LTO external functions always can go.
>        (cgraph_used_from_object_file_p): Handle EXTERNAL functions correctly.
>        (cgraph_mark_address_taken_node): Assert that we are not taking address of
>        inline clone.
>        (cgraph_can_remove_if_no_direct_calls_p): We always eventually remove
>        external functions.
>        * ipa-cp.c (ipcp_cloning_candidate_p): Do not clone functions with address taken.
>        (ipcp_initialize_node_lattices): Only local functions can be handled without cloning.
>        * cgraph.h (cgraph_set_readonly_flag,
>        cgraph_set_looping_const_or_pure_flag): Remove.
>        (cgraph_set_const_flag): Declare.
>        (cgraph_set_pure_flag): Update.
>        * ipa-pure-const (propagate_pure_const, local_pure_const): Update
>        flags setting code.
>        * ipa.c (cgraph_remove_unreachable_nodes): Fix formating; do not look at inline
>        clones; fix handling of external definitions.
>        (cgraph_postorder): Do not look at inline clones in the first pass.
>        (function_and_variable_visibility): Drop constructors/destructor
>        flags at pure and const functions.
>        * tree-profile.c (tree_profiling): Update.
>        * ipa-inline.c (cgraph_clone_inlined_nodes): Always clone functions with
>        address taken; external functions do not account to whole program size.
>        (cgraph_decide_inlining): Likewise; do not try to inline functions already
>        inlined.
>
>        * testsuite/gcc.dg/lto/pr45736_0.c: New function.

This patch caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46367


H.J.

Re: Fix removal of ctors/dtors

[H.J. Lu]

On Mon, Nov 8, 2010 at 11:03 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Oct 26, 2010 at 7:16 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi,
>> while implementing code removing pure & const constructors I considered correct option of
>> dropping CONSTRUCTOR/DESTRUCTOR flag for functions found to be const or pure or slightly hacky
>> of simply teaching dead function removal to consider them dead.  The second is suboptimal
>> for static constructors & destructors that are also called dirrectly.  I concluded that no one
>> does that, but Zdenek did and it triggers one overparanoid sanity check.
>>
>> So this patch takes the cleaner route of just dropping the flags and letting
>> unreachable code removal to remove the functions later.  This happens in
>> visibility pass where we sanitize flags for pure/const constructors comming
>> from forntends or via cgraph_set_pure_flag/cgraph_set_const_flags when they are
>> discovered pure/const later.
>>
>> While working on it I also noticed that we don't handle correctly extern
>> declarations (we consider them extern inlines and keep them in the cgraph.
>> This is wasteful especially for LTO) and extern inlines (we keep them in the
>> program even if they are obviously dead - they are never called nor have address
>> taken up to the IPA inliner). Cleaning this issue shown that both inliner and
>> ipa-cp compute code size costs incorrectly by assuming that optimizing out
>> external copy of extern function it will save code size.
>>
>> Finally both inliner and ipa-cp confuse handling of functions with address
>> taken.  ipa-cp is considering functions with address taken to be cloning candidates.
>> This is wrong.  We should clone only when we think original function is not called at all
>> but we can't say for sure since it is externally visible.  Clonning is of course possible,
>> but current impementation leads to wrong code when updating call sites of calls promoted to
>> be direct.
>>
>> Inliner on the other hand produce invalid callgraph with inline clones having
>> address taken.
>>
>> Bootstrapped/regtested x86_64-linux, lto-bootstrapped x86_64-linux and tested
>> with Firefox builds.   I also tested that tramp3d does not care about the
>> inliner mertric fixes.
>>
>> Honza
>>
>>        * cgraph.c (cgraph_set_readonly_flag): Rename to...
>>        (cgraph_set_const_flags) ... this one; get also looping argument;
>>        clear constructor/destructor flags.
>>        (cgraph_set_pure_flag): Likewise.
>>        (cgraph_set_looping_const_or_pure_flag): Remove.
>>        (cgraph_can_remove_if_no_direct_calls_and_refs): Do not try
>>        to optimize away static ctors/dtors; it does not work on inline clones;
>>        external functions can always be rmeoved.
>>        (cgraph_will_be_removed_from_program_if_no_direct_calls): Assert on inline
>>        clones; in LTO external functions always can go.
>>        (cgraph_used_from_object_file_p): Handle EXTERNAL functions correctly.
>>        (cgraph_mark_address_taken_node): Assert that we are not taking address of
>>        inline clone.
>>        (cgraph_can_remove_if_no_direct_calls_p): We always eventually remove
>>        external functions.
>>        * ipa-cp.c (ipcp_cloning_candidate_p): Do not clone functions with address taken.
>>        (ipcp_initialize_node_lattices): Only local functions can be handled without cloning.
>>        * cgraph.h (cgraph_set_readonly_flag,
>>        cgraph_set_looping_const_or_pure_flag): Remove.
>>        (cgraph_set_const_flag): Declare.
>>        (cgraph_set_pure_flag): Update.
>>        * ipa-pure-const (propagate_pure_const, local_pure_const): Update
>>        flags setting code.
>>        * ipa.c (cgraph_remove_unreachable_nodes): Fix formating; do not look at inline
>>        clones; fix handling of external definitions.
>>        (cgraph_postorder): Do not look at inline clones in the first pass.
>>        (function_and_variable_visibility): Drop constructors/destructor
>>        flags at pure and const functions.
>>        * tree-profile.c (tree_profiling): Update.
>>        * ipa-inline.c (cgraph_clone_inlined_nodes): Always clone functions with
>>        address taken; external functions do not account to whole program size.
>>        (cgraph_decide_inlining): Likewise; do not try to inline functions already
>>        inlined.
>>
>>        * testsuite/gcc.dg/lto/pr45736_0.c: New function.
>
> This patch caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46367
>

This also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46469


-- 
H.J.

Re: Fix removal of ctors/dtors

[H.J. Lu]

On Sat, Nov 13, 2010 at 3:31 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Nov 8, 2010 at 11:03 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Oct 26, 2010 at 7:16 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> Hi,
>>> while implementing code removing pure & const constructors I considered correct option of
>>> dropping CONSTRUCTOR/DESTRUCTOR flag for functions found to be const or pure or slightly hacky
>>> of simply teaching dead function removal to consider them dead.  The second is suboptimal
>>> for static constructors & destructors that are also called dirrectly.  I concluded that no one
>>> does that, but Zdenek did and it triggers one overparanoid sanity check.
>>>
>>> So this patch takes the cleaner route of just dropping the flags and letting
>>> unreachable code removal to remove the functions later.  This happens in
>>> visibility pass where we sanitize flags for pure/const constructors comming
>>> from forntends or via cgraph_set_pure_flag/cgraph_set_const_flags when they are
>>> discovered pure/const later.
>>>
>>> While working on it I also noticed that we don't handle correctly extern
>>> declarations (we consider them extern inlines and keep them in the cgraph.
>>> This is wasteful especially for LTO) and extern inlines (we keep them in the
>>> program even if they are obviously dead - they are never called nor have address
>>> taken up to the IPA inliner). Cleaning this issue shown that both inliner and
>>> ipa-cp compute code size costs incorrectly by assuming that optimizing out
>>> external copy of extern function it will save code size.
>>>
>>> Finally both inliner and ipa-cp confuse handling of functions with address
>>> taken.  ipa-cp is considering functions with address taken to be cloning candidates.
>>> This is wrong.  We should clone only when we think original function is not called at all
>>> but we can't say for sure since it is externally visible.  Clonning is of course possible,
>>> but current impementation leads to wrong code when updating call sites of calls promoted to
>>> be direct.
>>>
>>> Inliner on the other hand produce invalid callgraph with inline clones having
>>> address taken.
>>>
>>> Bootstrapped/regtested x86_64-linux, lto-bootstrapped x86_64-linux and tested
>>> with Firefox builds.   I also tested that tramp3d does not care about the
>>> inliner mertric fixes.
>>>
>>> Honza
>>>
>>>        * cgraph.c (cgraph_set_readonly_flag): Rename to...
>>>        (cgraph_set_const_flags) ... this one; get also looping argument;
>>>        clear constructor/destructor flags.
>>>        (cgraph_set_pure_flag): Likewise.
>>>        (cgraph_set_looping_const_or_pure_flag): Remove.
>>>        (cgraph_can_remove_if_no_direct_calls_and_refs): Do not try
>>>        to optimize away static ctors/dtors; it does not work on inline clones;
>>>        external functions can always be rmeoved.
>>>        (cgraph_will_be_removed_from_program_if_no_direct_calls): Assert on inline
>>>        clones; in LTO external functions always can go.
>>>        (cgraph_used_from_object_file_p): Handle EXTERNAL functions correctly.
>>>        (cgraph_mark_address_taken_node): Assert that we are not taking address of
>>>        inline clone.
>>>        (cgraph_can_remove_if_no_direct_calls_p): We always eventually remove
>>>        external functions.
>>>        * ipa-cp.c (ipcp_cloning_candidate_p): Do not clone functions with address taken.
>>>        (ipcp_initialize_node_lattices): Only local functions can be handled without cloning.
>>>        * cgraph.h (cgraph_set_readonly_flag,
>>>        cgraph_set_looping_const_or_pure_flag): Remove.
>>>        (cgraph_set_const_flag): Declare.
>>>        (cgraph_set_pure_flag): Update.
>>>        * ipa-pure-const (propagate_pure_const, local_pure_const): Update
>>>        flags setting code.
>>>        * ipa.c (cgraph_remove_unreachable_nodes): Fix formating; do not look at inline
>>>        clones; fix handling of external definitions.
>>>        (cgraph_postorder): Do not look at inline clones in the first pass.
>>>        (function_and_variable_visibility): Drop constructors/destructor
>>>        flags at pure and const functions.
>>>        * tree-profile.c (tree_profiling): Update.
>>>        * ipa-inline.c (cgraph_clone_inlined_nodes): Always clone functions with
>>>        address taken; external functions do not account to whole program size.
>>>        (cgraph_decide_inlining): Likewise; do not try to inline functions already
>>>        inlined.
>>>
>>>        * testsuite/gcc.dg/lto/pr45736_0.c: New function.
>>
>> This patch caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46367
>>
>
> This also caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46469
>

This also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47009

-- 
H.J.