diff mbox

Avoid redundant indirect_info computation during inderct edge cloning

Message ID 20140818120748.GA29976@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Aug. 18, 2014, 12:07 p.m. UTC 
On 15 Aug 23:08, Jan Hubicka wrote:
> > Hi,
> > 
> > I get a segafult in decl_maybe_in_construction_p during function versioning.  We have following steps in clone creation (e.g. as in create_version_clone_with_body):
> >  1. Create function decl
> >  2. Create clone of cgraph node
> >  3. Copy function body
> > After the first step there is no body attached to function and DECL_STRUCT_FUNCTION for new decl is NULL.  It is initialized on the third step.  But on the second step get_polymorphic_call_info may be called for new function; it calls decl_maybe_in_construction_p which assumes DECL_STRUCT_FUNCTION already exists.
> > 
> > I firstly wanted to fix decl_maybe_in_construction_p but then realized cgraph_clone_edge copy indirect_info from the original edge anyway and therefore its computation is not required at all.
> > 
> > Following patch removes redundant indirect_info computation.  Bootstrapped and regtested on linux-x86_64.  Does it look OK for trunk?
> 
> OK, plase also add testcase from https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61800
> 
> Thanks,
> Honza

Here is a version with testcase to be committed.

Thanks,
Ilya
--
gcc/

2014-08-18  Ilya Enkovich  <ilya.enkovich@intel.com>

	PR ipa/61800
	* cgraph.h (cgraph_node::create_indirect_edge): Add
	compute_indirect_info param.
	* cgraph.c (cgraph_node::create_indirect_edge): Compute
	indirect_info only when it is required.
	* cgraphclones.c (cgraph_clone_edge): Do not recompute
	indirect_info fore cloned indirect edge.

gcc/testsuite

2014-08-18  Ilya Enkovich  <ilya.enkovich@intel.com>

	* g++.dg/ipa/pr61800.C: New.

Comments

Jeff Law Aug. 30, 2014, 6:39 a.m. UTC | #1 
On 08/18/14 06:07, Ilya Enkovich wrote:
> On 15 Aug 23:08, Jan Hubicka wrote:
>>> Hi,
>>>
>>> I get a segafult in decl_maybe_in_construction_p during function versioning.  We have following steps in clone creation (e.g. as in create_version_clone_with_body):
>>>   1. Create function decl
>>>   2. Create clone of cgraph node
>>>   3. Copy function body
>>> After the first step there is no body attached to function and DECL_STRUCT_FUNCTION for new decl is NULL.  It is initialized on the third step.  But on the second step get_polymorphic_call_info may be called for new function; it calls decl_maybe_in_construction_p which assumes DECL_STRUCT_FUNCTION already exists.
>>>
>>> I firstly wanted to fix decl_maybe_in_construction_p but then realized cgraph_clone_edge copy indirect_info from the original edge anyway and therefore its computation is not required at all.
>>>
>>> Following patch removes redundant indirect_info computation.  Bootstrapped and regtested on linux-x86_64.  Does it look OK for trunk?
>>
>> OK, plase also add testcase from https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61800
>>
>> Thanks,
>> Honza
>
> Here is a version with testcase to be committed.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-08-18  Ilya Enkovich  <ilya.enkovich@intel.com>
>
> 	PR ipa/61800
> 	* cgraph.h (cgraph_node::create_indirect_edge): Add
> 	compute_indirect_info param.
> 	* cgraph.c (cgraph_node::create_indirect_edge): Compute
> 	indirect_info only when it is required.
> 	* cgraphclones.c (cgraph_clone_edge): Do not recompute
> 	indirect_info fore cloned indirect edge.
Just to be 100% clear, this version is OK as well.

jeff
diff mbox

Patch

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 370a96a..cb49cdc 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -942,7 +942,8 @@  cgraph_allocate_init_indirect_info (void)
 
 struct cgraph_edge *
 cgraph_node::create_indirect_edge (gimple call_stmt, int ecf_flags,
-				   gcov_type count, int freq)
+				   gcov_type count, int freq,
+				   bool compute_indirect_info)
 {
   struct cgraph_edge *edge = cgraph_node::create_edge (this, NULL, call_stmt,
 						       count, freq, true);
@@ -954,7 +955,8 @@  cgraph_node::create_indirect_edge (gimple call_stmt, int ecf_flags,
   edge->indirect_info->ecf_flags = ecf_flags;
 
   /* Record polymorphic call info.  */
-  if (call_stmt
+  if (compute_indirect_info
+      && call_stmt
       && (target = gimple_call_fn (call_stmt))
       && virtual_method_call_p (target))
     {
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 13c09af..2594ae5 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -915,7 +915,8 @@  public:
      statement destination is a formal parameter of the caller with index
      PARAM_INDEX. */
   struct cgraph_edge *create_indirect_edge (gimple call_stmt, int ecf_flags,
-					    gcov_type count, int freq);
+					    gcov_type count, int freq,
+					    bool compute_indirect_info = true);
 
   /* Like cgraph_create_edge walk the clone tree and update all clones sharing
    same function body.  If clones already have edge for OLD_STMT; only
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index c04b5c8..557f734 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -136,7 +136,7 @@  cgraph_clone_edge (struct cgraph_edge *e, struct cgraph_node *n,
 	{
 	  new_edge = n->create_indirect_edge (call_stmt,
 					      e->indirect_info->ecf_flags,
-					      count, freq);
+					      count, freq, false);
 	  *new_edge->indirect_info = *e->indirect_info;
 	}
     }
diff --git a/gcc/testsuite/g++.dg/ipa/pr61800.C b/gcc/testsuite/g++.dg/ipa/pr61800.C
new file mode 100644
index 0000000..b4da899
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr61800.C
@@ -0,0 +1,67 @@ 
+/* PR ipa/61800 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#pragma GCC visibility push(hidden)
+class A
+{
+public:
+  unsigned long m_fn1 () const;
+};
+class B;
+class C
+{
+public:
+  ;
+  virtual bool m_fn2 (void) = 0;
+};
+class D
+{
+  virtual bool m_fn3 (const int &p1, B *p2) = 0;
+};
+class F : public D
+{
+  bool m_fn3 (const int &p1, B *p2);
+  A mPredicates;
+};
+class B
+{
+};
+class G : public B
+{
+  virtual unsigned int m_fn4 () = 0;
+};
+class H : public G
+{
+public:
+  int txNodeSetContext_aContextNodeSet;
+  H (B *p1) {}
+  int
+  m_fn5 ()
+  {
+    return mPosition < m_fn4 ();
+  }
+  unsigned int m_fn4 ();
+  unsigned int mPosition;
+};
+
+unsigned int a;
+C *b;
+bool
+F::m_fn3 (const int &p1, B *p2)
+{
+  if (!b->m_fn2 ())
+    return false;
+  unsigned int c = mPredicates.m_fn1 ();
+  for (1; 1 < c; ++a)
+    {
+      H d (p2);
+      while (d.m_fn5 ())
+        {
+          do
+            {
+            }
+          while (0);
+        }
+    }
+}