[PR,82027] Also stream opt_info of former_clones

Message ID ri6zi6b4uou.fsf@suse.cz
State New
Headers show
Series
  • [PR,82027] Also stream opt_info of former_clones
Related show

Commit Message

Martin Jambor Dec. 21, 2017, 9:50 p.m.
Hi,

the problem in PR 82027 is a general one with thunks that are created
for IPA-CP clones.  At WPA time, IPA-CP creates a clone that is called
through a thunk.  An "artificial thunk" is created in
duplicate_thunk_for_node() in cgraphclones.c.  This function also
removes now unnecessary arguments from DECL_ARGUMENTS of the new thunk.
The node gets appropriate args_to_skip so that its call-sites also get
the counterpart actual arguments removed at inline transform time.
Because the artificial thunk is not a virtual clone, it is created with
clone_of NULL and former_clone_of set to the original thunk.

Unfortunately, NULL clone_of means that output_cgraph_opt_summary_p()
returns false for the new artificial_thunk and therefore its
args_to_skip is not streamed and is lost.  At inline transform time, we
do not remove the counterpart actual arguments and we end up with
argument mismatch.

The patch below fixes this by streaming args_to_skip also of former
clones, which brings LTO in-line with non-LTO.  At WPA time, the only
nodes with non-NULL former_clone_of are artificial thunks created by
duplicate_thunk_for_node(), so it does not grow the streamed IL
unnecessarily.

Unfortunately, I do not remember why we have decided not to create
clones of thunks as virtual clones, which would be arguably a more
natural fix.  Perhaps those reasons no longer apply and in gcc 9 we
could do that.  However, for GCC 8, 7 and 6, I would prefer this small
non-intrusive one.

Bootstrapped and tested on x86_64-linux, I will attempt an LTO bootstrap
overnight, OK for trunk, gcc 7 and gcc 6 if it passes?

Thanks,

Martin


2017-12-21  Martin Jambor  <mjambor@suse.cz>

	PR lto/82027
	* lto-cgraph.c (output_cgraph_opt_summary_p): Also check former
	clones.

testsuite/
	* g++.dg/lto/pr82027_0.C: New test.
---
 gcc/lto-cgraph.c                     |  2 +-
 gcc/testsuite/g++.dg/lto/pr82027_0.C | 73 ++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr82027_0.C

-- 
2.15.1

Comments

Jeff Law Dec. 21, 2017, 11:17 p.m. | #1
On 12/21/2017 02:50 PM, Martin Jambor wrote:
> Hi,

> 

> the problem in PR 82027 is a general one with thunks that are created

> for IPA-CP clones.  At WPA time, IPA-CP creates a clone that is called

> through a thunk.  An "artificial thunk" is created in

> duplicate_thunk_for_node() in cgraphclones.c.  This function also

> removes now unnecessary arguments from DECL_ARGUMENTS of the new thunk.

> The node gets appropriate args_to_skip so that its call-sites also get

> the counterpart actual arguments removed at inline transform time.

> Because the artificial thunk is not a virtual clone, it is created with

> clone_of NULL and former_clone_of set to the original thunk.

> 

> Unfortunately, NULL clone_of means that output_cgraph_opt_summary_p()

> returns false for the new artificial_thunk and therefore its

> args_to_skip is not streamed and is lost.  At inline transform time, we

> do not remove the counterpart actual arguments and we end up with

> argument mismatch.

> 

> The patch below fixes this by streaming args_to_skip also of former

> clones, which brings LTO in-line with non-LTO.  At WPA time, the only

> nodes with non-NULL former_clone_of are artificial thunks created by

> duplicate_thunk_for_node(), so it does not grow the streamed IL

> unnecessarily.

> 

> Unfortunately, I do not remember why we have decided not to create

> clones of thunks as virtual clones, which would be arguably a more

> natural fix.  Perhaps those reasons no longer apply and in gcc 9 we

> could do that.  However, for GCC 8, 7 and 6, I would prefer this small

> non-intrusive one.

> 

> Bootstrapped and tested on x86_64-linux, I will attempt an LTO bootstrap

> overnight, OK for trunk, gcc 7 and gcc 6 if it passes?

> 

> Thanks,

> 

> Martin

> 

> 

> 2017-12-21  Martin Jambor  <mjambor@suse.cz>

> 

> 	PR lto/82027

> 	* lto-cgraph.c (output_cgraph_opt_summary_p): Also check former

> 	clones.

> 

> testsuite/

> 	* g++.dg/lto/pr82027_0.C: New test.

OK.
jeff

Patch

diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
index a19f8a13dfb..ed3df15b143 100644
--- a/gcc/lto-cgraph.c
+++ b/gcc/lto-cgraph.c
@@ -1958,7 +1958,7 @@  input_offload_tables (bool do_force_output)
 static int
 output_cgraph_opt_summary_p (struct cgraph_node *node)
 {
-  return (node->clone_of
+  return ((node->clone_of || node->former_clone_of)
 	  && (node->clone.tree_map
 	      || node->clone.args_to_skip
 	      || node->clone.combined_args_to_skip));
diff --git a/gcc/testsuite/g++.dg/lto/pr82027_0.C b/gcc/testsuite/g++.dg/lto/pr82027_0.C
new file mode 100644
index 00000000000..70cc776b2db
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr82027_0.C
@@ -0,0 +1,73 @@ 
+// { dg-lto-do run }
+// { dg-lto-options { { -O3 -flto } } }
+
+class Position
+{
+  public:
+    Position( void ) {}
+    virtual ~Position() {}
+
+    virtual void calcPercent( const char *name,int pos,int size ) {}
+};
+
+
+class Looper
+{
+  public:
+    Looper( Position *cc,int size )
+      : m_cc(cc), m_size(size) {}
+    virtual ~Looper() {}
+
+    void loop( void )
+    {
+      for( int pos=0; pos<m_size; pos++ )
+      {
+        m_cc->calcPercent( "",pos,m_size );
+      }
+    }
+
+  private:
+    Position *m_cc;
+    int m_size;
+};
+
+
+class EmptyClass
+{
+  public:
+    EmptyClass( void ) {}
+    virtual ~EmptyClass() {}
+};
+
+
+class Combined : public EmptyClass, public Position
+{
+  public:
+    Combined( void ) : m_percent(0) {}
+    ~Combined() {}
+
+    void calcPercent( const char *name,int pos,int size )
+    {
+      int percent = 100*pos/size;
+      if( percent!=m_percent )
+        m_percent = percent;
+    }
+
+  private:
+    int m_percent;
+};
+
+
+
+int main( int argc,char **argv )
+{
+  Combined *comb = new Combined();
+  Looper *looper = new Looper( comb,argc );
+
+  looper->loop();
+
+  delete comb;
+  delete looper;
+
+  return( 0 );
+}