diff mbox

[PR,46984] Move thunk_delta from cgraph_indirect_call_info to cgraph_edge

Message ID 20101228220912.GA3649@alvy.suse.cz
State New
Headers show

Commit Message

Martin Jambor Dec. 28, 2010, 10:09 p.m. UTC 
Hi,

On Mon, Dec 27, 2010 at 11:24:07AM +0100, Martin Jambor wrote:
> On Mon, Dec 27, 2010 at 01:39:43AM +0100, Jan Hubicka wrote:
> > > I guess that I'd have to add a vector of edges to lto_cgraph_encoder_d
> > > and to input_cgraph, input_cgraph_opt_summary and its callees.  Hardly
> > > something to be pre-approved :-)
> > > 
> > > I've already written some of this but before I finish it, test it and
> > > re-post, I'd like to ask you to re-consider.  If we ever build call
> > > graph edges for calls to thunks (especially as we plan to be able to
> > > do it when building the graph), such thunk information would become an
> > > integral part of an edge, without which the edge has an invalid
> > > meaning.  I'd therefore not consider this an "optimization"
> > > information in the sense that it is somehow optional (unlike
> > > args_to_skip or tree_map which we can ignore without miscompiling).
> > > 
> > > Or do I somehow misunderstand the intended meaning of the cgraphopt
> > > section?
> > 
> > Well, cgraphopt stands for cgraph optimization summary that is everything
> > streamed down from wpa to ltrans but not from lgen to wpa.
> > I see that we might want to store thunk info on edges to represent the
> > direct calls to thunks, but this opens another cam of worms since what
> > is thunk in one unit don't need to be tunk in another. So I think we rather
> > want to lower the calls...
> 
> Fair enough.  However, if we try to lower the calls to thunks up
> front, we might actually want to keep the thunk_delta field in
> cgraph_indirect_call_info in order to save memory for the vast
> majority of call graph edges that have nothing to do with thunks (but
> still convert it to HOST_WIDE_INT and stream as such in cgraphopt).
> 
> I'll continue working on this along these lines then.  Thanks,

So it seems I forgot how we stream edge-associated info, that we
simply depend on the order the caller node has them in its list
instead of doing UID translations.  Indeed streaming can be rather
simple and I will afterall probably use the pre-approval of the patch
below, which I have bootstrapped and tested on x86_64-linux and also
tested with make check-c++ in i686.  I will commit it tomorrow if
there are no objections.

Thanks a lot,

Martin

2010-12-27  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/46984
	* cgraph.h (cgraph_indirect_call_info): make field thunk_delta
	HOST_WIDE_INT.
	(cgraph_create_indirect_edge): Fixed line length.
	(cgraph_indirect_call_info): Declare.
	(cgraph_make_edge_direct) Update declaration.
	* cgraph.c (cgraph_allocate_init_indirect_info): New function.
	(cgraph_create_indirect_edge): Use it.
	(cgraph_make_edge_direct): Made delta HOST_WIDE_INT.  Updated all
	callees.
	* cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Update for
	the new thunk_delta representation.
	* ipa-prop.c (ipa_make_edge_direct_to_target): Convert delta to
	HOST_WIDE_INT.
	(ipa_write_indirect_edge_info): Remove streaming of thunk_delta.
	(ipa_read_indirect_edge_info): Likewise.
	* lto-cgraph.c (output_edge_opt_summary): New function.
	(output_node_opt_summary): Call it on all outgoing edges.
	(input_edge_opt_summary): New function.
	(input_node_opt_summary): Call it on all outgoing edges.xo

	* testsuite/g++.dg/ipa/pr46984.C: New test.

Comments

H.J. Lu Jan. 4, 2011, 5:54 a.m. UTC | #1 
On Tue, Dec 28, 2010 at 2:09 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Mon, Dec 27, 2010 at 11:24:07AM +0100, Martin Jambor wrote:
>> On Mon, Dec 27, 2010 at 01:39:43AM +0100, Jan Hubicka wrote:
>> > > I guess that I'd have to add a vector of edges to lto_cgraph_encoder_d
>> > > and to input_cgraph, input_cgraph_opt_summary and its callees.  Hardly
>> > > something to be pre-approved :-)
>> > >
>> > > I've already written some of this but before I finish it, test it and
>> > > re-post, I'd like to ask you to re-consider.  If we ever build call
>> > > graph edges for calls to thunks (especially as we plan to be able to
>> > > do it when building the graph), such thunk information would become an
>> > > integral part of an edge, without which the edge has an invalid
>> > > meaning.  I'd therefore not consider this an "optimization"
>> > > information in the sense that it is somehow optional (unlike
>> > > args_to_skip or tree_map which we can ignore without miscompiling).
>> > >
>> > > Or do I somehow misunderstand the intended meaning of the cgraphopt
>> > > section?
>> >
>> > Well, cgraphopt stands for cgraph optimization summary that is everything
>> > streamed down from wpa to ltrans but not from lgen to wpa.
>> > I see that we might want to store thunk info on edges to represent the
>> > direct calls to thunks, but this opens another cam of worms since what
>> > is thunk in one unit don't need to be tunk in another. So I think we rather
>> > want to lower the calls...
>>
>> Fair enough.  However, if we try to lower the calls to thunks up
>> front, we might actually want to keep the thunk_delta field in
>> cgraph_indirect_call_info in order to save memory for the vast
>> majority of call graph edges that have nothing to do with thunks (but
>> still convert it to HOST_WIDE_INT and stream as such in cgraphopt).
>>
>> I'll continue working on this along these lines then.  Thanks,
>
> So it seems I forgot how we stream edge-associated info, that we
> simply depend on the order the caller node has them in its list
> instead of doing UID translations.  Indeed streaming can be rather
> simple and I will afterall probably use the pre-approval of the patch
> below, which I have bootstrapped and tested on x86_64-linux and also
> tested with make check-c++ in i686.  I will commit it tomorrow if
> there are no objections.
>
> Thanks a lot,
>
> Martin
>
> 2010-12-27  Martin Jambor  <mjambor@suse.cz>
>
>        PR tree-optimization/46984
>        * cgraph.h (cgraph_indirect_call_info): make field thunk_delta
>        HOST_WIDE_INT.
>        (cgraph_create_indirect_edge): Fixed line length.
>        (cgraph_indirect_call_info): Declare.
>        (cgraph_make_edge_direct) Update declaration.
>        * cgraph.c (cgraph_allocate_init_indirect_info): New function.
>        (cgraph_create_indirect_edge): Use it.
>        (cgraph_make_edge_direct): Made delta HOST_WIDE_INT.  Updated all
>        callees.
>        * cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Update for
>        the new thunk_delta representation.
>        * ipa-prop.c (ipa_make_edge_direct_to_target): Convert delta to
>        HOST_WIDE_INT.
>        (ipa_write_indirect_edge_info): Remove streaming of thunk_delta.
>        (ipa_read_indirect_edge_info): Likewise.
>        * lto-cgraph.c (output_edge_opt_summary): New function.
>        (output_node_opt_summary): Call it on all outgoing edges.
>        (input_edge_opt_summary): New function.
>        (input_node_opt_summary): Call it on all outgoing edges.xo
>
>        * testsuite/g++.dg/ipa/pr46984.C: New test.
>

This patch breaks LTO:

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

You can reproduce it with SPEC CPU 2K on Linux/x86-64.
diff mbox

Patch

Index: icln/gcc/cgraph.c
===================================================================
--- icln.orig/gcc/cgraph.c	2010-12-27 13:48:19.000000000 +0100
+++ icln/gcc/cgraph.c	2010-12-27 16:19:24.000000000 +0100
@@ -860,7 +860,7 @@  cgraph_set_call_stmt (struct cgraph_edge
 	 indirect call into a direct one.  */
       struct cgraph_node *new_callee = cgraph_node (decl);
 
-      cgraph_make_edge_direct (e, new_callee, NULL);
+      cgraph_make_edge_direct (e, new_callee, 0);
     }
 
   push_cfun (DECL_STRUCT_FUNCTION (e->caller->decl));
@@ -1070,6 +1070,17 @@  cgraph_create_edge (struct cgraph_node *
   return edge;
 }
 
+/* Allocate cgraph_indirect_call_info and set its fields to default values. */
+
+struct cgraph_indirect_call_info *
+cgraph_allocate_init_indirect_info (void)
+{
+  struct cgraph_indirect_call_info *ii;
+
+  ii = ggc_alloc_cleared_cgraph_indirect_call_info ();
+  ii->param_index = -1;
+  return ii;
+}
 
 /* Create an indirect edge with a yet-undetermined callee where the call
    statement destination is a formal parameter of the caller with index
@@ -1086,8 +1097,7 @@  cgraph_create_indirect_edge (struct cgra
   edge->indirect_unknown_callee = 1;
   initialize_inline_failed (edge);
 
-  edge->indirect_info = ggc_alloc_cleared_cgraph_indirect_call_info ();
-  edge->indirect_info->param_index = -1;
+  edge->indirect_info = cgraph_allocate_init_indirect_info ();
   edge->indirect_info->ecf_flags = ecf_flags;
 
   edge->next_callee = caller->indirect_calls;
@@ -1195,12 +1205,12 @@  cgraph_redirect_edge_callee (struct cgra
 }
 
 /* Make an indirect EDGE with an unknown callee an ordinary edge leading to
-   CALLEE.  DELTA, if non-NULL, is an integer constant that is to be added to
-   the this pointer (first parameter).  */
+   CALLEE.  DELTA is an integer constant that is to be added to the this
+   pointer (first parameter) to compensate for skipping a thunk adjustment.  */
 
 void
 cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee,
-			 tree delta)
+			 HOST_WIDE_INT delta)
 {
   edge->indirect_unknown_callee = 0;
   edge->indirect_info->thunk_delta = delta;
Index: icln/gcc/cgraph.h
===================================================================
--- icln.orig/gcc/cgraph.h	2010-12-27 13:48:19.000000000 +0100
+++ icln/gcc/cgraph.h	2010-12-27 15:32:13.000000000 +0100
@@ -386,11 +386,11 @@  struct GTY(()) cgraph_indirect_call_info
   HOST_WIDE_INT anc_offset;
   /* OBJ_TYPE_REF_TOKEN of a polymorphic call (if polymorphic is set).  */
   HOST_WIDE_INT otr_token;
+  /* Delta by which must be added to this parameter to compensate for a skipped
+     this adjusting thunk.  */
+  HOST_WIDE_INT thunk_delta;
   /* Type of the object from OBJ_TYPE_REF_OBJECT. */
   tree otr_type;
-  /* Delta by which must be added to this parameter.  For polymorphic calls
-     only.  */
-  tree thunk_delta;
   /* Index of the parameter that is called.  */
   int param_index;
   /* ECF flags determined from the caller.  */
@@ -549,8 +549,9 @@  void cgraph_node_remove_callees (struct
 struct cgraph_edge *cgraph_create_edge (struct cgraph_node *,
 					struct cgraph_node *,
 					gimple, gcov_type, int, int);
-struct cgraph_edge *cgraph_create_indirect_edge (struct cgraph_node *, gimple, int,
-						 gcov_type, int, int);
+struct cgraph_edge *cgraph_create_indirect_edge (struct cgraph_node *, gimple,
+						 int, gcov_type, int, int);
+struct cgraph_indirect_call_info *cgraph_allocate_init_indirect_info (void);
 struct cgraph_node * cgraph_get_node (const_tree);
 struct cgraph_node * cgraph_get_node_or_alias (const_tree);
 struct cgraph_node * cgraph_node (tree);
@@ -578,7 +579,8 @@  struct cgraph_node * cgraph_clone_node (
 					int, bool, VEC(cgraph_edge_p,heap) *);
 
 void cgraph_redirect_edge_callee (struct cgraph_edge *, struct cgraph_node *);
-void cgraph_make_edge_direct (struct cgraph_edge *, struct cgraph_node *, tree);
+void cgraph_make_edge_direct (struct cgraph_edge *, struct cgraph_node *,
+			      HOST_WIDE_INT);
 
 struct cgraph_asm_node *cgraph_add_asm_node (tree);
 
Index: icln/gcc/cgraphunit.c
===================================================================
--- icln.orig/gcc/cgraphunit.c	2010-12-27 13:48:19.000000000 +0100
+++ icln/gcc/cgraphunit.c	2010-12-27 16:17:24.000000000 +0100
@@ -2168,22 +2168,20 @@  cgraph_redirect_edge_call_stmt_to_callee
 	}
     }
 
-  if (e->indirect_info && e->indirect_info->thunk_delta
-      && integer_nonzerop (e->indirect_info->thunk_delta)
+  if (e->indirect_info &&
+      e->indirect_info->thunk_delta != 0
       && (!e->callee->clone.combined_args_to_skip
 	  || !bitmap_bit_p (e->callee->clone.combined_args_to_skip, 0)))
     {
       if (cgraph_dump_file)
-	{
-	  fprintf (cgraph_dump_file, "          Thunk delta is ");
-	  print_generic_expr (cgraph_dump_file,
-			      e->indirect_info->thunk_delta, 0);
-	  fprintf (cgraph_dump_file, "\n");
-	}
+	fprintf (cgraph_dump_file, "          Thunk delta is "
+		 HOST_WIDE_INT_PRINT_DEC "\n", e->indirect_info->thunk_delta);
       gsi = gsi_for_stmt (e->call_stmt);
       gsi_computed = true;
-      gimple_adjust_this_by_delta (&gsi, e->indirect_info->thunk_delta);
-      e->indirect_info->thunk_delta = NULL_TREE;
+      gimple_adjust_this_by_delta (&gsi,
+				   build_int_cst (sizetype,
+					       e->indirect_info->thunk_delta));
+      e->indirect_info->thunk_delta = 0;
     }
 
   if (e->callee->clone.combined_args_to_skip)
Index: icln/gcc/ipa-prop.c
===================================================================
--- icln.orig/gcc/ipa-prop.c	2010-12-27 13:48:19.000000000 +0100
+++ icln/gcc/ipa-prop.c	2010-12-27 13:51:13.000000000 +0100
@@ -1483,7 +1483,7 @@  ipa_make_edge_direct_to_target (struct c
     return NULL;
   ipa_check_create_node_params ();
 
-  cgraph_make_edge_direct (ie, callee, delta);
+  cgraph_make_edge_direct (ie, callee, delta ? tree_low_cst (delta, 0) : 0);
   if (dump_file)
     {
       fprintf (dump_file, "ipa-prop: Discovered %s call to a known target "
@@ -2549,7 +2549,6 @@  ipa_write_indirect_edge_info (struct out
     {
       lto_output_sleb128_stream (ob->main_stream, ii->otr_token);
       lto_output_tree (ob, ii->otr_type, true);
-      lto_output_tree (ob, ii->thunk_delta, true);
     }
 }
 
@@ -2572,7 +2571,6 @@  ipa_read_indirect_edge_info (struct lto_
     {
       ii->otr_token = (HOST_WIDE_INT) lto_input_sleb128 (ib);
       ii->otr_type = lto_input_tree (ib, data_in);
-      ii->thunk_delta = lto_input_tree (ib, data_in);
     }
 }
 
Index: icln/gcc/lto-cgraph.c
===================================================================
--- icln.orig/gcc/lto-cgraph.c	2010-12-27 13:48:19.000000000 +0100
+++ icln/gcc/lto-cgraph.c	2010-12-27 15:34:13.000000000 +0100
@@ -1605,6 +1605,18 @@  output_cgraph_opt_summary_p (struct cgra
           || node->clone.combined_args_to_skip);
 }
 
+/* Output optimization summary for EDGE to OB.  */
+static void
+output_edge_opt_summary (struct output_block *ob,
+			 struct cgraph_edge *edge)
+{
+  if (edge->indirect_info)
+    lto_output_sleb128_stream (ob->main_stream,
+			       edge->indirect_info->thunk_delta);
+  else
+    lto_output_sleb128_stream (ob->main_stream, 0);
+}
+
 /* Output optimization summary for NODE to OB.  */
 
 static void
@@ -1616,6 +1628,7 @@  output_node_opt_summary (struct output_b
   struct ipa_replace_map *map;
   struct bitpack_d bp;
   int i;
+  struct cgraph_edge *e;
 
   lto_output_uleb128_stream (ob->main_stream,
 			     bitmap_count_bits (node->clone.args_to_skip));
@@ -1646,6 +1659,10 @@  output_node_opt_summary (struct output_b
       bp_pack_value (&bp, map->ref_p, 1);
       lto_output_bitpack (&bp);
     }
+  for (e = node->callees; e; e = e->next_callee)
+    output_edge_opt_summary (ob, e);
+  for (e = node->indirect_calls; e; e = e->next_callee)
+    output_edge_opt_summary (ob, e);
 }
 
 /* Output optimization summaries stored in callgraph.
@@ -1680,7 +1697,23 @@  output_cgraph_opt_summary (void)
   destroy_output_block (ob);
 }
 
-/* Input optimiation summary of NODE.  */
+/* Input optimisation summary of EDGE.  */
+
+static void
+input_edge_opt_summary (struct cgraph_edge *edge,
+			struct lto_input_block *ib_main)
+{
+  HOST_WIDE_INT thunk_delta;
+  thunk_delta = lto_input_sleb128 (ib_main);
+  if (thunk_delta != 0)
+    {
+      gcc_assert (!edge->indirect_info);
+      edge->indirect_info = cgraph_allocate_init_indirect_info ();
+      edge->indirect_info->thunk_delta = thunk_delta;
+    }
+}
+
+/* Input optimisation summary of NODE.  */
 
 static void
 input_node_opt_summary (struct cgraph_node *node,
@@ -1691,6 +1724,7 @@  input_node_opt_summary (struct cgraph_no
   int count;
   int bit;
   struct bitpack_d bp;
+  struct cgraph_edge *e;
 
   count = lto_input_uleb128 (ib_main);
   if (count)
@@ -1726,6 +1760,10 @@  input_node_opt_summary (struct cgraph_no
       map->replace_p = bp_unpack_value (&bp, 1);
       map->ref_p = bp_unpack_value (&bp, 1);
     }
+  for (e = node->callees; e; e = e->next_callee)
+    input_edge_opt_summary (e, ib_main);
+  for (e = node->indirect_calls; e; e = e->next_callee)
+    input_edge_opt_summary (e, ib_main);
 }
 
 /* Read section in file FILE_DATA of length LEN with data DATA.  */
Index: icln/gcc/testsuite/g++.dg/ipa/pr46984.C
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ icln/gcc/testsuite/g++.dg/ipa/pr46984.C	2010-12-27 13:51:13.000000000 +0100
@@ -0,0 +1,62 @@ 
+// { dg-options "-O -fipa-cp -fno-early-inlining -flto" }
+// { dg-do run }
+
+extern "C" void abort ();
+
+class A
+{
+public:
+  virtual void foo () {abort();}
+};
+
+class B : public A
+{
+public:
+  int z;
+  virtual void foo () {abort();}
+};
+
+class C : public A
+{
+public:
+  void *a[32];
+  unsigned long b;
+  long c[32];
+
+  virtual void foo () {abort();}
+};
+
+class D : public C, public B
+{
+public:
+  D () : C(), B()
+  {
+    int i;
+    for (i = 0; i < 32; i++)
+      {
+	a[i] = (void *) 0;
+	c[i] = 0;
+      }
+    b = 0xaaaa;
+  }
+
+  virtual void foo ();
+};
+
+void D::foo()
+{
+  if (b != 0xaaaa)
+    abort();
+}
+
+static inline void bar (B &b)
+{
+  b.foo ();
+}
+
+int main()
+{
+  D d;
+  bar (d);
+  return 0;
+}