[PR 89330] Avoid adding dead speculative edges to inlinig heap

[PR 89330] Avoid adding dead speculative edges to inlinig heap

[Martin Jambor]

Hi,

Martin discovered that inliner was adding deleted call graph edges to
its heap when supposedly processing newly discovered direct edges.  The
problem is that a new edge created in the speculation part of the
indirect inlining machinery created speculative edges that were
immediately afterwards removed by check_speculations() after it figured
out the edge is not speculation_useful_p().

The fix below avoids creating such non-speculation_useful_p edges in the
first place.  The edge is not useful because it cannot be inlined
because the callee calls comdat local functions.  I had to split
can_inline_edge_p into two functions to allow perform the caller and
callee checks before actually creating an edge.

I think this is safe and beneficial to commit now, maybe with the
exception of the newly added assert in add_new_edges_to_heap, since
inlining apparently can cope with such nonsensical edges in the heap.
But in that case I'd add the assert in the next stage1.

Bootstrapped and tested on x86_64-linux.  IIUC, Martin even
LTO-bootstrapped it.  OK for trunk?

Thanks,

Martin



2019-02-15  Martin Jambor  <mjambor@suse.cz>

	PR ipa/89330
	* ipa-inline.c (can_inline_edge_p): Move most of the checks...
	(call_not_inlinable_p): ...this new function.
	(add_new_edges_to_heap): Assert a caller is known.
	* ipa-inline.h (call_not_inlinable_p): Declare.
	* ipa-prop.c: Include ipa-inline.h
	(try_make_edge_direct_virtual_call): Create speculative edges only
	if there is any chance of inlining them.

	testsuite/
	* g++.dg/lto/pr89330_[01].C: New test.
---
 gcc/ipa-inline.c                     | 128 ++++++++++++---------------
 gcc/ipa-inline.h                     |   4 +-
 gcc/ipa-prop.c                       |   8 +-
 gcc/testsuite/g++.dg/lto/pr89330_0.C |  50 +++++++++++
 gcc/testsuite/g++.dg/lto/pr89330_1.C |  36 ++++++++
 5 files changed, 154 insertions(+), 72 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr89330_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr89330_1.C

diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 360c3de3289..ae330943571 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -299,12 +299,60 @@ sanitize_attrs_match_for_inline_p (const_tree caller, const_tree callee)
       (opts_for_fn (caller->decl)->x_##flag		\
        != opts_for_fn (callee->decl)->x_##flag)
 
+/* Return CIF_OK if a call from CALLER to CALLEE is or would be inlineable.
+   Otherwise, return the reason why it cannot.  EARLY should be set when
+   deciding about early inlining.  */
+
+enum cgraph_inline_failed_t
+call_not_inlinable_p (cgraph_node *caller, cgraph_node *callee,
+		      bool early)
+{
+  enum availability avail;
+  caller = caller->global.inlined_to ? caller->global.inlined_to : caller;
+  callee = callee->ultimate_alias_target (&avail, caller);
+
+  if (!callee->definition)
+    return CIF_BODY_NOT_AVAILABLE;
+  if (!early && (!opt_for_fn (callee->decl, optimize)
+		 || !opt_for_fn (caller->decl, optimize)))
+    return CIF_FUNCTION_NOT_OPTIMIZED;
+  else if (callee->calls_comdat_local)
+    return CIF_USES_COMDAT_LOCAL;
+  else if (avail <= AVAIL_INTERPOSABLE)
+    return CIF_OVERWRITABLE;
+  /* Don't inline if the functions have different EH personalities.  */
+  else if (DECL_FUNCTION_PERSONALITY (caller->decl)
+	   && DECL_FUNCTION_PERSONALITY (callee->decl)
+	   && (DECL_FUNCTION_PERSONALITY (caller->decl)
+	       != DECL_FUNCTION_PERSONALITY (callee->decl)))
+    return CIF_EH_PERSONALITY;
+  /* TM pure functions should not be inlined into non-TM_pure
+     functions.  */
+  else if (is_tm_pure (callee->decl) && !is_tm_pure (caller->decl))
+    return CIF_UNSPECIFIED;
+  /* Check compatibility of target optimization options.  */
+  else if (!targetm.target_option.can_inline_p (caller->decl,
+						callee->decl))
+    return CIF_TARGET_OPTION_MISMATCH;
+  else if (ipa_fn_summaries->get (callee) == NULL
+	   || !ipa_fn_summaries->get (callee)->inlinable)
+    return CIF_FUNCTION_NOT_INLINABLE;
+  /* Don't inline a function with mismatched sanitization attributes. */
+  else if (!sanitize_attrs_match_for_inline_p (caller->decl, callee->decl))
+    return CIF_ATTRIBUTE_MISMATCH;
+  else if (callee->externally_visible
+	   && flag_live_patching == LIVE_PATCHING_INLINE_ONLY_STATIC)
+    return CIF_EXTERN_LIVE_ONLY_STATIC;
+  return CIF_OK;
+}
+
 /* Decide if we can inline the edge and possibly update
    inline_failed reason.  
    We check whether inlining is possible at all and whether
    caller growth limits allow doing so.  
 
-   if REPORT is true, output reason to the dump file. */
+   If REPORT is true, output reason to the dump file.  EARLY should be set when
+   deciding about early inlining.  */
 
 static bool
 can_inline_edge_p (struct cgraph_edge *e, bool report,
@@ -319,81 +367,22 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
       return false;
     }
 
-  bool inlinable = true;
-  enum availability avail;
-  cgraph_node *caller = e->caller->global.inlined_to
-		        ? e->caller->global.inlined_to : e->caller;
-  cgraph_node *callee = e->callee->ultimate_alias_target (&avail, caller);
+  enum cgraph_inline_failed_t fail_reason
+    = call_not_inlinable_p (e->caller, e->callee, early);
 
-  if (!callee->definition)
+
+  if (fail_reason != CIF_OK)
     {
-      e->inline_failed = CIF_BODY_NOT_AVAILABLE;
-      inlinable = false;
-    }
-  if (!early && (!opt_for_fn (callee->decl, optimize)
-		 || !opt_for_fn (caller->decl, optimize)))
-    {
-      e->inline_failed = CIF_FUNCTION_NOT_OPTIMIZED;
-      inlinable = false;
-    }
-  else if (callee->calls_comdat_local)
-    {
-      e->inline_failed = CIF_USES_COMDAT_LOCAL;
-      inlinable = false;
-    }
-  else if (avail <= AVAIL_INTERPOSABLE)
-    {
-      e->inline_failed = CIF_OVERWRITABLE;
-      inlinable = false;
+      e->inline_failed = fail_reason;
+      if (report)
+	report_inline_failed_reason (e);
     }
   /* All edges with call_stmt_cannot_inline_p should have inline_failed
      initialized to one of FINAL_ERROR reasons.  */
   else if (e->call_stmt_cannot_inline_p)
     gcc_unreachable ();
-  /* Don't inline if the functions have different EH personalities.  */
-  else if (DECL_FUNCTION_PERSONALITY (caller->decl)
-	   && DECL_FUNCTION_PERSONALITY (callee->decl)
-	   && (DECL_FUNCTION_PERSONALITY (caller->decl)
-	       != DECL_FUNCTION_PERSONALITY (callee->decl)))
-    {
-      e->inline_failed = CIF_EH_PERSONALITY;
-      inlinable = false;
-    }
-  /* TM pure functions should not be inlined into non-TM_pure
-     functions.  */
-  else if (is_tm_pure (callee->decl) && !is_tm_pure (caller->decl))
-    {
-      e->inline_failed = CIF_UNSPECIFIED;
-      inlinable = false;
-    }
-  /* Check compatibility of target optimization options.  */
-  else if (!targetm.target_option.can_inline_p (caller->decl,
-						callee->decl))
-    {
-      e->inline_failed = CIF_TARGET_OPTION_MISMATCH;
-      inlinable = false;
-    }
-  else if (ipa_fn_summaries->get (callee) == NULL
-	   || !ipa_fn_summaries->get (callee)->inlinable)
-    {
-      e->inline_failed = CIF_FUNCTION_NOT_INLINABLE;
-      inlinable = false;
-    }
-  /* Don't inline a function with mismatched sanitization attributes. */
-  else if (!sanitize_attrs_match_for_inline_p (caller->decl, callee->decl))
-    {
-      e->inline_failed = CIF_ATTRIBUTE_MISMATCH;
-      inlinable = false;
-    }
-  else if (callee->externally_visible
-	   && flag_live_patching == LIVE_PATCHING_INLINE_ONLY_STATIC)
-    {
-      e->inline_failed = CIF_EXTERN_LIVE_ONLY_STATIC;
-      inlinable = false;
-    }
-  if (!inlinable && report)
-    report_inline_failed_reason (e);
-  return inlinable;
+
+  return fail_reason == CIF_OK;
 }
 
 /* Decide if we can inline the edge and possibly update
@@ -1627,6 +1616,7 @@ add_new_edges_to_heap (edge_heap_t *heap, vec<cgraph_edge *> new_edges)
     {
       struct cgraph_edge *edge = new_edges.pop ();
 
+      gcc_assert (edge->callee);
       gcc_assert (!edge->aux);
       if (edge->inline_failed
 	  && can_inline_edge_p (edge, true)
diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h
index f6eb677d618..911df58d646 100644
--- a/gcc/ipa-inline.h
+++ b/gcc/ipa-inline.h
@@ -52,7 +52,9 @@ void free_growth_caches (void);
 /* In ipa-inline.c  */
 unsigned int early_inliner (function *fun);
 bool inline_account_function_p (struct cgraph_node *node);
-
+enum cgraph_inline_failed_t call_not_inlinable_p (cgraph_node *caller,
+						  cgraph_node *callee,
+						  bool early);
 
 /* In ipa-inline-transform.c  */
 bool inline_call (struct cgraph_edge *, bool, vec<cgraph_edge *> *, int *, bool,
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index d86c2f3db55..36ba91c3800 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -53,6 +53,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "domwalk.h"
 #include "builtins.h"
 #include "tree-cfgcleanup.h"
+#include "ipa-inline.h"
 
 /* Function summary where the parameter infos are actually stored. */
 ipa_node_params_t *ipa_node_params_sum = NULL;
@@ -3346,13 +3347,16 @@ try_make_edge_direct_virtual_call (struct cgraph_edge *ie,
 
   if (target)
     {
-      if (!possible_polymorphic_call_target_p
-	  (ie, cgraph_node::get_create (target)))
+      cgraph_node *target_node = cgraph_node::get_create (target);
+      if (!possible_polymorphic_call_target_p (ie, target_node))
 	{
 	  if (speculative)
 	    return NULL;
 	  target = ipa_impossible_devirt_target (ie, target);
 	}
+      if (speculative
+	  && call_not_inlinable_p (ie->caller, target_node, false))
+	return NULL;
       return ipa_make_edge_direct_to_target (ie, target, speculative);
     }
   else
diff --git a/gcc/testsuite/g++.dg/lto/pr89330_0.C b/gcc/testsuite/g++.dg/lto/pr89330_0.C
new file mode 100644
index 00000000000..10082f83412
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr89330_0.C
@@ -0,0 +1,50 @@
+// { dg-lto-do link }
+// { dg-lto-options { { -O3 -g -flto -shared -Wno-odr } } }
+
+namespace Inkscape {
+class Anchored {};
+namespace XML {
+enum NodeType {};
+class Node :Anchored {
+public:
+  virtual NodeType type() ;
+  virtual char name() ;
+  virtual int code() ;
+  virtual unsigned position() ;
+  virtual unsigned childCount() ;
+  virtual char content() ;
+  virtual char *attribute() const ;
+  virtual int attributeList() ;
+  virtual bool matchAttributeName() ;
+  virtual void setPosition() ;
+  virtual void setContent() ;
+  virtual void setAttribute() ;
+  virtual int document() ;
+  virtual int document() const ;
+  virtual Node *root() ;
+  virtual Node *root() const ;
+  virtual Node *parent() ;
+  virtual Node *next() const ;
+  virtual Node *firstChild() const ;
+
+};
+} } struct rdf_license_t {
+  };
+;
+class RDFImpl {
+;
+  rdf_license_t *getLicense();
+};
+static bool rdf_match_license(Inkscape::XML::Node const *repr) {
+  for (Inkscape::XML::Node *current = repr->firstChild(); current;
+       current->next()->attribute());
+  return 0;
+}
+rdf_license_t *RDFImpl::getLicense() {
+  Inkscape::XML::Node *repr ;
+  for (rdf_license_t *license ; license;
+       license) {
+    rdf_match_license(repr);
+  }
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/lto/pr89330_1.C b/gcc/testsuite/g++.dg/lto/pr89330_1.C
new file mode 100644
index 00000000000..5b999eee8d7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr89330_1.C
@@ -0,0 +1,36 @@
+typedef char gchar;
+namespace Inkscape {
+class Anchored {
+int _anchor;
+};
+namespace XML {
+enum NodeType {};
+class Node :Anchored {
+virtual NodeType type() ;
+  virtual char const *name() const ;
+  virtual int code() ;
+  virtual unsigned position() ;
+  virtual unsigned childCount() ;
+  virtual char content() ;
+  virtual char attribute() ;
+  virtual int attributeList() ;
+  virtual bool matchAttributeName() ;
+  virtual void setPosition() ;
+  virtual void setContent() ;
+  virtual int document() ;
+  virtual int document() const ;
+  virtual Node *root() ;
+  virtual Node *root() const ;
+  virtual Node *parent() ;
+  virtual Node *parent() const ;
+  virtual Node *next() ;
+  virtual Node const *next() const ;
+
+};
+class SimpleNode : virtual Node {
+char const *name() const;
+  Node *next() const { return _next; }
+  SimpleNode *_next;
+};
+gchar const *SimpleNode::name() const { return 0; }
+} } 
-- 
2.20.1

Re: [PR 89330] Avoid adding dead speculative edges to inlinig heap

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

Hi.

This is rebased version of the patch that Martin J. wrote.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Thanks,
Martin


[-- Attachment #2: 0001-Avoid-adding-dead-speculative-edges-to-inlining-heap.patch --]
[-- Type: text/x-patch, Size: 10779 bytes --]

From 17290dd6dc4412aee6c6484844f9edb149129d36 Mon Sep 17 00:00:00 2001
From: Martin Jambor <mjambor@suse.cz>
Date: Wed, 5 Jun 2019 16:11:44 +0200
Subject: [PATCH] Avoid adding dead speculative edges to inlining heap

2019-02-15  Martin Jambor  <mjambor@suse.cz>

	PR ipa/89330
	* ipa-inline.c (can_inline_edge_p): Move most of the checks...
	(call_not_inlinable_p): ...this new function.
	* ipa-inline.h (call_not_inlinable_p): Declare.
	* ipa-prop.c: Include ipa-inline.h
	(try_make_edge_direct_virtual_call): Create speculative edges only
	if there is any chance of inlining them.

	testsuite/
	* g++.dg/lto/pr89330_[01].C: New test.
---
 gcc/ipa-inline.c                     | 119 +++++++++++++--------------
 gcc/ipa-inline.h                     |   4 +-
 gcc/ipa-prop.c                       |   8 +-
 gcc/testsuite/g++.dg/lto/pr89330_0.C |  50 +++++++++++
 gcc/testsuite/g++.dg/lto/pr89330_1.C |  36 ++++++++
 5 files changed, 151 insertions(+), 66 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr89330_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr89330_1.C

diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 745bdf3002a..ba8155f006e 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -299,95 +299,87 @@ sanitize_attrs_match_for_inline_p (const_tree caller, const_tree callee)
       (opts_for_fn (caller->decl)->x_##flag		\
        != opts_for_fn (callee->decl)->x_##flag)
 
-/* Decide if we can inline the edge and possibly update
-   inline_failed reason.  
-   We check whether inlining is possible at all and whether
-   caller growth limits allow doing so.  
-
-   if REPORT is true, output reason to the dump file. */
+/* Return CIF_OK if a call from CALLER to CALLEE is or would be inlineable.
+   Otherwise, return the reason why it cannot.  EARLY should be set when
+   deciding about early inlining.  */
 
-static bool
-can_inline_edge_p (struct cgraph_edge *e, bool report,
-		   bool early = false)
+enum cgraph_inline_failed_t
+call_not_inlinable_p (cgraph_node *caller, cgraph_node *callee,
+		      bool early)
 {
-  gcc_checking_assert (e->inline_failed);
-
-  if (cgraph_inline_failed_type (e->inline_failed) == CIF_FINAL_ERROR)
-    {
-      if (report)
-        report_inline_failed_reason (e);
-      return false;
-    }
-
-  bool inlinable = true;
   enum availability avail;
-  cgraph_node *caller = e->caller->global.inlined_to
-		        ? e->caller->global.inlined_to : e->caller;
-  cgraph_node *callee = e->callee->ultimate_alias_target (&avail, caller);
+  caller = caller->global.inlined_to ? caller->global.inlined_to : caller;
+  callee = callee->ultimate_alias_target (&avail, caller);
 
   if (!callee->definition)
-    {
-      e->inline_failed = CIF_BODY_NOT_AVAILABLE;
-      inlinable = false;
-    }
+    return CIF_BODY_NOT_AVAILABLE;
   if (!early && (!opt_for_fn (callee->decl, optimize)
 		 || !opt_for_fn (caller->decl, optimize)))
-    {
-      e->inline_failed = CIF_FUNCTION_NOT_OPTIMIZED;
-      inlinable = false;
-    }
+    return CIF_FUNCTION_NOT_OPTIMIZED;
   else if (callee->calls_comdat_local)
-    {
-      e->inline_failed = CIF_USES_COMDAT_LOCAL;
-      inlinable = false;
-    }
+    return CIF_USES_COMDAT_LOCAL;
   else if (avail <= AVAIL_INTERPOSABLE)
-    {
-      e->inline_failed = CIF_OVERWRITABLE;
-      inlinable = false;
-    }
-  /* All edges with call_stmt_cannot_inline_p should have inline_failed
-     initialized to one of FINAL_ERROR reasons.  */
-  else if (e->call_stmt_cannot_inline_p)
-    gcc_unreachable ();
+    return CIF_OVERWRITABLE;
   /* Don't inline if the functions have different EH personalities.  */
   else if (DECL_FUNCTION_PERSONALITY (caller->decl)
 	   && DECL_FUNCTION_PERSONALITY (callee->decl)
 	   && (DECL_FUNCTION_PERSONALITY (caller->decl)
 	       != DECL_FUNCTION_PERSONALITY (callee->decl)))
-    {
-      e->inline_failed = CIF_EH_PERSONALITY;
-      inlinable = false;
-    }
+    return CIF_EH_PERSONALITY;
   /* TM pure functions should not be inlined into non-TM_pure
      functions.  */
   else if (is_tm_pure (callee->decl) && !is_tm_pure (caller->decl))
-    {
-      e->inline_failed = CIF_UNSPECIFIED;
-      inlinable = false;
-    }
+    return CIF_UNSPECIFIED;
   /* Check compatibility of target optimization options.  */
   else if (!targetm.target_option.can_inline_p (caller->decl,
 						callee->decl))
-    {
-      e->inline_failed = CIF_TARGET_OPTION_MISMATCH;
-      inlinable = false;
-    }
+    return CIF_TARGET_OPTION_MISMATCH;
   else if (ipa_fn_summaries->get (callee) == NULL
 	   || !ipa_fn_summaries->get (callee)->inlinable)
-    {
-      e->inline_failed = CIF_FUNCTION_NOT_INLINABLE;
-      inlinable = false;
-    }
+    return CIF_FUNCTION_NOT_INLINABLE;
   /* Don't inline a function with mismatched sanitization attributes. */
   else if (!sanitize_attrs_match_for_inline_p (caller->decl, callee->decl))
+    return CIF_ATTRIBUTE_MISMATCH;
+  return CIF_OK;
+}
+
+/* Decide if we can inline the edge and possibly update
+   inline_failed reason.
+   We check whether inlining is possible at all and whether
+   caller growth limits allow doing so.
+
+   If REPORT is true, output reason to the dump file.  EARLY should be set when
+   deciding about early inlining.  */
+
+static bool
+can_inline_edge_p (struct cgraph_edge *e, bool report,
+		   bool early = false)
+{
+  gcc_checking_assert (e->inline_failed);
+
+  if (cgraph_inline_failed_type (e->inline_failed) == CIF_FINAL_ERROR)
     {
-      e->inline_failed = CIF_ATTRIBUTE_MISMATCH;
-      inlinable = false;
+      if (report)
+	report_inline_failed_reason (e);
+      return false;
     }
-  if (!inlinable && report)
-    report_inline_failed_reason (e);
-  return inlinable;
+
+  enum cgraph_inline_failed_t fail_reason
+    = call_not_inlinable_p (e->caller, e->callee, early);
+
+
+  if (fail_reason != CIF_OK)
+    {
+      e->inline_failed = fail_reason;
+      if (report)
+	report_inline_failed_reason (e);
+    }
+  /* All edges with call_stmt_cannot_inline_p should have inline_failed
+     initialized to one of FINAL_ERROR reasons.  */
+  else if (e->call_stmt_cannot_inline_p)
+    gcc_unreachable ();
+
+  return fail_reason == CIF_OK;
 }
 
 /* Decide if we can inline the edge and possibly update
@@ -1628,6 +1620,7 @@ add_new_edges_to_heap (edge_heap_t *heap, vec<cgraph_edge *> new_edges)
     {
       struct cgraph_edge *edge = new_edges.pop ();
 
+      gcc_assert (edge->callee);
       gcc_assert (!edge->aux);
       if (edge->inline_failed
 	  && can_inline_edge_p (edge, true)
diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h
index f6eb677d618..911df58d646 100644
--- a/gcc/ipa-inline.h
+++ b/gcc/ipa-inline.h
@@ -52,7 +52,9 @@ void free_growth_caches (void);
 /* In ipa-inline.c  */
 unsigned int early_inliner (function *fun);
 bool inline_account_function_p (struct cgraph_node *node);
-
+enum cgraph_inline_failed_t call_not_inlinable_p (cgraph_node *caller,
+						  cgraph_node *callee,
+						  bool early);
 
 /* In ipa-inline-transform.c  */
 bool inline_call (struct cgraph_edge *, bool, vec<cgraph_edge *> *, int *, bool,
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index d86c2f3db55..36ba91c3800 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -53,6 +53,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "domwalk.h"
 #include "builtins.h"
 #include "tree-cfgcleanup.h"
+#include "ipa-inline.h"
 
 /* Function summary where the parameter infos are actually stored. */
 ipa_node_params_t *ipa_node_params_sum = NULL;
@@ -3346,13 +3347,16 @@ try_make_edge_direct_virtual_call (struct cgraph_edge *ie,
 
   if (target)
     {
-      if (!possible_polymorphic_call_target_p
-	  (ie, cgraph_node::get_create (target)))
+      cgraph_node *target_node = cgraph_node::get_create (target);
+      if (!possible_polymorphic_call_target_p (ie, target_node))
 	{
 	  if (speculative)
 	    return NULL;
 	  target = ipa_impossible_devirt_target (ie, target);
 	}
+      if (speculative
+	  && call_not_inlinable_p (ie->caller, target_node, false))
+	return NULL;
       return ipa_make_edge_direct_to_target (ie, target, speculative);
     }
   else
diff --git a/gcc/testsuite/g++.dg/lto/pr89330_0.C b/gcc/testsuite/g++.dg/lto/pr89330_0.C
new file mode 100644
index 00000000000..10082f83412
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr89330_0.C
@@ -0,0 +1,50 @@
+// { dg-lto-do link }
+// { dg-lto-options { { -O3 -g -flto -shared -Wno-odr } } }
+
+namespace Inkscape {
+class Anchored {};
+namespace XML {
+enum NodeType {};
+class Node :Anchored {
+public:
+  virtual NodeType type() ;
+  virtual char name() ;
+  virtual int code() ;
+  virtual unsigned position() ;
+  virtual unsigned childCount() ;
+  virtual char content() ;
+  virtual char *attribute() const ;
+  virtual int attributeList() ;
+  virtual bool matchAttributeName() ;
+  virtual void setPosition() ;
+  virtual void setContent() ;
+  virtual void setAttribute() ;
+  virtual int document() ;
+  virtual int document() const ;
+  virtual Node *root() ;
+  virtual Node *root() const ;
+  virtual Node *parent() ;
+  virtual Node *next() const ;
+  virtual Node *firstChild() const ;
+
+};
+} } struct rdf_license_t {
+  };
+;
+class RDFImpl {
+;
+  rdf_license_t *getLicense();
+};
+static bool rdf_match_license(Inkscape::XML::Node const *repr) {
+  for (Inkscape::XML::Node *current = repr->firstChild(); current;
+       current->next()->attribute());
+  return 0;
+}
+rdf_license_t *RDFImpl::getLicense() {
+  Inkscape::XML::Node *repr ;
+  for (rdf_license_t *license ; license;
+       license) {
+    rdf_match_license(repr);
+  }
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/lto/pr89330_1.C b/gcc/testsuite/g++.dg/lto/pr89330_1.C
new file mode 100644
index 00000000000..5b999eee8d7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr89330_1.C
@@ -0,0 +1,36 @@
+typedef char gchar;
+namespace Inkscape {
+class Anchored {
+int _anchor;
+};
+namespace XML {
+enum NodeType {};
+class Node :Anchored {
+virtual NodeType type() ;
+  virtual char const *name() const ;
+  virtual int code() ;
+  virtual unsigned position() ;
+  virtual unsigned childCount() ;
+  virtual char content() ;
+  virtual char attribute() ;
+  virtual int attributeList() ;
+  virtual bool matchAttributeName() ;
+  virtual void setPosition() ;
+  virtual void setContent() ;
+  virtual int document() ;
+  virtual int document() const ;
+  virtual Node *root() ;
+  virtual Node *root() const ;
+  virtual Node *parent() ;
+  virtual Node *parent() const ;
+  virtual Node *next() ;
+  virtual Node const *next() const ;
+
+};
+class SimpleNode : virtual Node {
+char const *name() const;
+  Node *next() const { return _next; }
+  SimpleNode *_next;
+};
+gchar const *SimpleNode::name() const { return 0; }
+} } 
-- 
2.21.0

[PATCH] Release cgraph_{node,edge} via ggc_free.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

Hi.

This is follow up patch that releases memory for removed
cgraph edges and nodes.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

[-- Attachment #2: 0001-Release-cgraph_-node-edge-via-ggc_free.patch --]
[-- Type: text/x-patch, Size: 5604 bytes --]

From 121138ee973b63bfdf7be04ab28113f479bc91b0 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 15 Feb 2019 14:19:59 +0100
Subject: [PATCH] Release cgraph_{node,edge} via ggc_free.

gcc/ChangeLog:

2019-02-19  Martin Liska  <mliska@suse.cz>

	* cgraph.c (symbol_table::create_edge): Always allocate
	a cgraph_edge.
	(symbol_table::free_edge): Store summary_id to
	edge_released_summary_ids if != -1;
	* cgraph.h (NEXT_FREE_NODE): Remove.
	(SET_NEXT_FREE_NODE): Likewise.
	(NEXT_FREE_EDGE): Likewise.
	(symbol_table::release_symbol): Store summary_id to
	cgraph_released_summary_ids if != -1;
	(symbol_table::allocate_cgraph_symbol): Always allocate
	a cgraph_node.
---
 gcc/cgraph.c | 26 +++++++----------------
 gcc/cgraph.h | 58 ++++++++++++++++++++++------------------------------
 2 files changed, 31 insertions(+), 53 deletions(-)

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 28019aba434..59dc70339f0 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -846,17 +846,8 @@ symbol_table::create_edge (cgraph_node *caller, cgraph_node *callee,
       gcc_assert (is_gimple_call (call_stmt));
     }
 
-  if (free_edges)
-    {
-      edge = free_edges;
-      free_edges = NEXT_FREE_EDGE (edge);
-    }
-  else
-    {
-      edge = ggc_alloc<cgraph_edge> ();
-      edge->m_summary_id = -1;
-    }
-
+  edge = ggc_alloc<cgraph_edge> ();
+  edge->m_summary_id = -1;
   edges_count++;
 
   gcc_assert (++edges_max_uid != 0);
@@ -1013,16 +1004,13 @@ cgraph_edge::remove_caller (void)
 void
 symbol_table::free_edge (cgraph_edge *e)
 {
+  edges_count--;
+  if (e->m_summary_id != -1)
+    edge_released_summary_ids.safe_push (e->m_summary_id);
+
   if (e->indirect_info)
     ggc_free (e->indirect_info);
-
-  /* Clear out the edge so we do not dangle pointers.  */
-  int summary_id = e->m_summary_id;
-  memset (e, 0, sizeof (*e));
-  e->m_summary_id = summary_id;
-  NEXT_FREE_EDGE (e) = free_edges;
-  free_edges = e;
-  edges_count--;
+  ggc_free (e);
 }
 
 /* Remove the edge in the cgraph.  */
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 18839a4a5ec..82ec5966a9b 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -2018,12 +2018,6 @@ is_a_helper <varpool_node *>::test (symtab_node *p)
   return p && p->type == SYMTAB_VARIABLE;
 }
 
-/* Macros to access the next item in the list of free cgraph nodes and
-   edges. */
-#define NEXT_FREE_NODE(NODE) dyn_cast<cgraph_node *> ((NODE)->next)
-#define SET_NEXT_FREE_NODE(NODE,NODE2) ((NODE))->next = NODE2
-#define NEXT_FREE_EDGE(EDGE) (EDGE)->prev_caller
-
 typedef void (*cgraph_edge_hook)(cgraph_edge *, void *);
 typedef void (*cgraph_node_hook)(cgraph_node *, void *);
 typedef void (*varpool_node_hook)(varpool_node *, void *);
@@ -2079,7 +2073,8 @@ public:
   friend class cgraph_edge;
 
   symbol_table (): cgraph_max_uid (1), cgraph_max_summary_id (0),
-  edges_max_uid (1), edges_max_summary_id (0)
+  edges_max_uid (1), edges_max_summary_id (0),
+  cgraph_released_summary_ids (), edge_released_summary_ids ()
   {
   }
 
@@ -2285,14 +2280,22 @@ public:
   /* Assign a new summary ID for the callgraph NODE.  */
   inline int assign_summary_id (cgraph_node *node)
   {
-    node->m_summary_id = cgraph_max_summary_id++;
+    if (!cgraph_released_summary_ids.is_empty ())
+      node->m_summary_id = cgraph_released_summary_ids.pop ();
+    else
+      node->m_summary_id = cgraph_max_summary_id++;
+
     return node->m_summary_id;
   }
 
   /* Assign a new summary ID for the callgraph EDGE.  */
   inline int assign_summary_id (cgraph_edge *edge)
   {
-    edge->m_summary_id = edges_max_summary_id++;
+    if (!edge_released_summary_ids.is_empty ())
+      edge->m_summary_id = edge_released_summary_ids.pop ();
+    else
+      edge->m_summary_id = edges_max_summary_id++;
+
     return edge->m_summary_id;
   }
 
@@ -2308,14 +2311,15 @@ public:
   int edges_max_uid;
   int edges_max_summary_id;
 
+  /* Vector of released summary IDS for cgraph nodes.  */
+  vec<int> GTY ((skip)) cgraph_released_summary_ids;
+
+  /* Vector of released summary IDS for cgraph nodes.  */
+  vec<int> GTY ((skip)) edge_released_summary_ids;
+
   symtab_node* GTY(()) nodes;
   asm_node* GTY(()) asmnodes;
   asm_node* GTY(()) asm_last_node;
-  cgraph_node* GTY(()) free_nodes;
-
-  /* Head of a linked list of unused (freed) call graph edges.
-     Do not GTY((delete)) this list so UIDs gets reliably recycled.  */
-  cgraph_edge * GTY(()) free_edges;
 
   /* The order index of the next symtab node to be created.  This is
      used so that we can sort the cgraph nodes in order by when we saw
@@ -2675,15 +2679,9 @@ inline void
 symbol_table::release_symbol (cgraph_node *node)
 {
   cgraph_count--;
-
-  /* Clear out the node to NULL all pointers and add the node to the free
-     list.  */
-  int summary_id = node->m_summary_id;
-  memset (node, 0, sizeof (*node));
-  node->type = SYMTAB_FUNCTION;
-  node->m_summary_id = summary_id;
-  SET_NEXT_FREE_NODE (node, free_nodes);
-  free_nodes = node;
+  if (node->m_summary_id != -1)
+    cgraph_released_summary_ids.safe_push (node->m_summary_id);
+  ggc_free (node);
 }
 
 /* Allocate new callgraph node.  */
@@ -2693,17 +2691,9 @@ symbol_table::allocate_cgraph_symbol (void)
 {
   cgraph_node *node;
 
-  if (free_nodes)
-    {
-      node = free_nodes;
-      free_nodes = NEXT_FREE_NODE (node);
-    }
-  else
-    {
-      node = ggc_cleared_alloc<cgraph_node> ();
-      node->m_summary_id = -1;
-    }
-
+  node = ggc_cleared_alloc<cgraph_node> ();
+  node->type = SYMTAB_FUNCTION;
+  node->m_summary_id = -1;
   node->m_uid = cgraph_max_uid++;
   return node;
 }
-- 
2.21.0

Re: [PR 89330] Avoid adding dead speculative edges to inlinig heap

[Jan Hubicka]

My understanding is that the problematic situation is when we create 
speculation,
insert it to the priority queue and then decide it is useless.
This is done by speculation_useful_p and that checks more than just the 
fact whether
function is inlinable (it accepts targets declared as PURE/CONST but 
also punts on cold
calls).
It also checks inline limits. So won't it break when these things are 
not in sync?

Honza

Dne 2019-06-06 10:46, Martin Liška napsal:
> Hi.
> 
> This is rebased version of the patch that Martin J. wrote.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Thanks,
> Martin