Fix devirtualiation in expanded thunks

Fix devirtualiation in expanded thunks

[Jan Hubicka]

Hi,
this patch fixes polymorphic call analysis in thunks.  Unlike normal
methods, thunks take THIS pointer offsetted by a known constant. This
needs t be compensated for when calculating address of outer type.

Bootstrapped/regtested x86_64-linux, also tested with Firefox where this
bug trigger misoptimization in spellchecker.  I plan to backport it to
release branches soon.

Honza

	PR ipa/88561
	* ipa-polymorphic-call.c
	(ipa_polymorphic_call_context::ipa_polymorphic_call_context): Handle
	arguments of thunks correctly.
	(ipa_polymorphic_call_context::get_dynamic_context): Be ready for
	NULL instance pinter.
	* lto-cgraph.c (lto_output_node): Always stream thunk info.
	* g++.dg/tree-prof/devirt.C: New testcase.
Index: ipa-polymorphic-call.c
===================================================================
--- ipa-polymorphic-call.c	(revision 267325)
+++ ipa-polymorphic-call.c	(working copy)
@@ -995,9 +995,22 @@ ipa_polymorphic_call_context::ipa_polymo
 	{
 	  outer_type
 	     = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (base_pointer)));
+	  cgraph_node *node = cgraph_node::get (current_function_decl);
 	  gcc_assert (TREE_CODE (outer_type) == RECORD_TYPE
 		      || TREE_CODE (outer_type) == UNION_TYPE);
 
+	  /* Handle the case we inlined into a thunk.  In this case
+	     thunk has THIS pointer of type bar, but it really receives
+	     address to its base type foo which sits in bar at 
+	     0-thunk.fixed_offset.  It starts with code that adds
+	     think.fixed_offset to the pointer to compensate for this.
+
+	     Because we walked all the way to the begining of thunk, we now
+	     see pointer &bar-thunk.fixed_offset and need to compensate
+	     for it.  */
+	  if (node->thunk.fixed_offset)
+	    offset -= node->thunk.fixed_offset * BITS_PER_UNIT;
+
 	  /* Dynamic casting has possibly upcasted the type
 	     in the hiearchy.  In this case outer type is less
 	     informative than inner type and we should forget
@@ -1005,7 +1018,11 @@ ipa_polymorphic_call_context::ipa_polymo
 	  if ((otr_type
 	       && !contains_type_p (outer_type, offset,
 				    otr_type))
-	      || !contains_polymorphic_type_p (outer_type))
+	      || !contains_polymorphic_type_p (outer_type)
+	      /* If we compile thunk with virtual offset, the THIS pointer
+		 is adjusted by unknown value.  We can't thus use outer info
+		 at all.  */
+	      || node->thunk.virtual_offset_p)
 	    {
 	      outer_type = NULL;
 	      if (instance)
@@ -1030,7 +1047,15 @@ ipa_polymorphic_call_context::ipa_polymo
 	      maybe_in_construction = false;
 	    }
 	  if (instance)
-	    *instance = base_pointer;
+	    {
+	      /* If method is expanded thunk, we need to apply thunk offset
+		 to instance pointer.  */
+	      if (node->thunk.virtual_offset_p
+		  || node->thunk.fixed_offset)
+		*instance = NULL;
+	      else
+	        *instance = base_pointer;
+	    }
 	  return;
 	}
       /* Non-PODs passed by value are really passed by invisible
@@ -1547,6 +1572,9 @@ ipa_polymorphic_call_context::get_dynami
   HOST_WIDE_INT instance_offset = offset;
   tree instance_outer_type = outer_type;
 
+  if (!instance)
+    return false;
+
   if (otr_type)
     otr_type = TYPE_MAIN_VARIANT (otr_type);
 
Index: lto-cgraph.c
===================================================================
--- lto-cgraph.c	(revision 267325)
+++ lto-cgraph.c	(working copy)
@@ -547,7 +547,11 @@ lto_output_node (struct lto_simple_outpu
   streamer_write_bitpack (&bp);
   streamer_write_data_stream (ob->main_stream, section, strlen (section) + 1);
 
-  if (node->thunk.thunk_p)
+  /* Stream thunk info always because we use it in
+     ipa_polymorphic_call_context::ipa_polymorphic_call_context
+     to properly interpret THIS pointers for thunks that has been converted
+     to Gimple.  */
+  if (node->definition)
     {
       streamer_write_uhwi_stream
 	 (ob->main_stream,
@@ -1295,7 +1299,7 @@ input_node (struct lto_file_decl_data *f
   if (section)
     node->set_section_for_node (section);
 
-  if (node->thunk.thunk_p)
+  if (node->definition)
     {
       int type = streamer_read_uhwi (ib);
       HOST_WIDE_INT fixed_offset = streamer_read_uhwi (ib);
Index: testsuite/g++.dg/tree-prof/devirt.C
===================================================================
--- testsuite/g++.dg/tree-prof/devirt.C	(nonexistent)
+++ testsuite/g++.dg/tree-prof/devirt.C	(working copy)
@@ -0,0 +1,123 @@
+/* { dg-options "-O3 -fdump-tree-dom3" } */
+struct nsISupports
+{
+  virtual int QueryInterface (const int &aIID, void **aInstancePtr) = 0;
+  virtual __attribute__((noinline, noclone)) unsigned AddRef (void) = 0;
+  virtual unsigned Release (void) = 0;
+};
+
+struct nsIObserver : public nsISupports
+{
+  virtual int Observe (nsISupports * aSubject, const char *aTopic, const unsigned short *aData) = 0;
+};
+
+struct nsISupportsWeakReference : public nsISupports
+{
+  virtual int GetWeakReference (void **_retval) = 0;
+};
+
+struct nsSupportsWeakReference : public nsISupportsWeakReference
+{
+  nsSupportsWeakReference () : mProxy (0) {}
+  virtual int GetWeakReference (void **_retval) override { return 0; }
+  ~nsSupportsWeakReference () {}
+  void NoticeProxyDestruction () { mProxy = nullptr; }
+  void *mProxy;
+  void ClearWeakReferences ();
+  bool HasWeakReferences () const { return !!mProxy; }
+};
+
+struct mozIPersonalDictionary : public nsISupports
+{
+  virtual int Load (void) = 0;
+  virtual int Save (void) = 0;
+  virtual int GetWordList (void **aWordList) = 0;
+  virtual int Check (const int &word, bool * _retval) = 0;
+  virtual int AddWord (const int &word) = 0;
+  virtual int RemoveWord (const int &word) = 0;
+  virtual int IgnoreWord (const int &word) = 0;
+  virtual int EndSession (void) = 0;
+};
+
+struct mozPersonalDictionary final
+  : public mozIPersonalDictionary, public nsIObserver, public nsSupportsWeakReference
+{
+  virtual int QueryInterface (const int &aIID, void **aInstancePtr) override;
+  virtual __attribute__((noinline, noclone)) unsigned AddRef (void) override;
+  virtual unsigned Release (void) override;
+  unsigned long mRefCnt;
+  virtual int Load (void) override { return 0; }
+  virtual int Save (void) override { return 0; }
+  virtual int GetWordList (void **aWordList) override { return 0; }
+  virtual int Check (const int &word, bool * _retval) override { return 0; }
+  virtual int AddWord (const int &word) override { return 0; }
+  virtual int RemoveWord (const int &word) override { return 0; }
+  virtual int IgnoreWord (const int &word) override { return 0; }
+  virtual int EndSession (void) override { return 0; }
+  virtual int Observe (nsISupports * aSubject, const char *aTopic, const unsigned short *aData) override { return 0; }
+  mozPersonalDictionary () : mRefCnt(0) {}
+  int Init () { return 0; }
+  virtual ~mozPersonalDictionary () {}
+  bool mIsLoaded;
+  bool mSavePending;
+  void *mFile;
+  char mMonitor[96];
+  char mMonitorSave[96];
+  char mDictionaryTable[32];
+  char mIgnoreTable[32];
+};
+
+unsigned
+mozPersonalDictionary::AddRef (void)
+{
+  unsigned count = ++mRefCnt;
+  return count;
+}
+
+unsigned
+mozPersonalDictionary::Release (void)
+{
+  unsigned count = --mRefCnt;
+  if (count == 0)
+    {
+      mRefCnt = 1;
+      delete (this);
+      return 0;
+    }
+  return count;
+}
+
+int
+mozPersonalDictionary::QueryInterface (const int &aIID, void **aInstancePtr)
+{
+  nsISupports *foundInterface;
+  if (aIID == 122)
+    foundInterface = static_cast <mozIPersonalDictionary *>(this);
+  else
+    foundInterface = static_cast <nsISupportsWeakReference *>(this);
+  int status;
+  foundInterface->AddRef ();
+  *aInstancePtr = foundInterface;
+  return status;
+}
+
+__attribute__((noipa)) int
+foo (nsISupports *p, const int &i)
+{
+  void *q;
+  return p->QueryInterface (i, &q);
+}
+
+int
+main ()
+{
+  mozPersonalDictionary m;
+  int j = 123;
+  for (int i = 0; i < 100000; i++)
+    foo (static_cast <nsISupportsWeakReference *>(&m), j);
+  if (m.mRefCnt != 100000)
+    __builtin_abort ();
+}
+
+/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" "dom3" } } */
+/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::AddRef" "dom3" } } */

Re: Fix devirtualiation in expanded thunks

[Sudakshina Das]

Hi Jan

On 21/12/18 7:20 PM, Jan Hubicka wrote:
> Hi,
> this patch fixes polymorphic call analysis in thunks.  Unlike normal
> methods, thunks take THIS pointer offsetted by a known constant. This
> needs t be compensated for when calculating address of outer type.
>
> Bootstrapped/regtested x86_64-linux, also tested with Firefox where this
> bug trigger misoptimization in spellchecker.  I plan to backport it to
> release branches soon.
>
> Honza
>
> 	PR ipa/88561
> 	* ipa-polymorphic-call.c
> 	(ipa_polymorphic_call_context::ipa_polymorphic_call_context): Handle
> 	arguments of thunks correctly.
> 	(ipa_polymorphic_call_context::get_dynamic_context): Be ready for
> 	NULL instance pinter.
> 	* lto-cgraph.c (lto_output_node): Always stream thunk info.
> 	* g++.dg/tree-prof/devirt.C: New testcase.
> Index: ipa-polymorphic-call.c
> ===================================================================
> --- ipa-polymorphic-call.c	(revision 267325)
> +++ ipa-polymorphic-call.c	(working copy)
> @@ -995,9 +995,22 @@ ipa_polymorphic_call_context::ipa_polymo
>   	{
>   	  outer_type
>   	     = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (base_pointer)));
> +	  cgraph_node *node = cgraph_node::get (current_function_decl);
>   	  gcc_assert (TREE_CODE (outer_type) == RECORD_TYPE
>   		      || TREE_CODE (outer_type) == UNION_TYPE);
>   
> +	  /* Handle the case we inlined into a thunk.  In this case
> +	     thunk has THIS pointer of type bar, but it really receives
> +	     address to its base type foo which sits in bar at
> +	     0-thunk.fixed_offset.  It starts with code that adds
> +	     think.fixed_offset to the pointer to compensate for this.
> +
> +	     Because we walked all the way to the begining of thunk, we now
> +	     see pointer &bar-thunk.fixed_offset and need to compensate
> +	     for it.  */
> +	  if (node->thunk.fixed_offset)
> +	    offset -= node->thunk.fixed_offset * BITS_PER_UNIT;
> +
>   	  /* Dynamic casting has possibly upcasted the type
>   	     in the hiearchy.  In this case outer type is less
>   	     informative than inner type and we should forget
> @@ -1005,7 +1018,11 @@ ipa_polymorphic_call_context::ipa_polymo
>   	  if ((otr_type
>   	       && !contains_type_p (outer_type, offset,
>   				    otr_type))
> -	      || !contains_polymorphic_type_p (outer_type))
> +	      || !contains_polymorphic_type_p (outer_type)
> +	      /* If we compile thunk with virtual offset, the THIS pointer
> +		 is adjusted by unknown value.  We can't thus use outer info
> +		 at all.  */
> +	      || node->thunk.virtual_offset_p)
>   	    {
>   	      outer_type = NULL;
>   	      if (instance)
> @@ -1030,7 +1047,15 @@ ipa_polymorphic_call_context::ipa_polymo
>   	      maybe_in_construction = false;
>   	    }
>   	  if (instance)
> -	    *instance = base_pointer;
> +	    {
> +	      /* If method is expanded thunk, we need to apply thunk offset
> +		 to instance pointer.  */
> +	      if (node->thunk.virtual_offset_p
> +		  || node->thunk.fixed_offset)
> +		*instance = NULL;
> +	      else
> +	        *instance = base_pointer;
> +	    }
>   	  return;
>   	}
>         /* Non-PODs passed by value are really passed by invisible
> @@ -1547,6 +1572,9 @@ ipa_polymorphic_call_context::get_dynami
>     HOST_WIDE_INT instance_offset = offset;
>     tree instance_outer_type = outer_type;
>   
> +  if (!instance)
> +    return false;
> +
>     if (otr_type)
>       otr_type = TYPE_MAIN_VARIANT (otr_type);
>   
> Index: lto-cgraph.c
> ===================================================================
> --- lto-cgraph.c	(revision 267325)
> +++ lto-cgraph.c	(working copy)
> @@ -547,7 +547,11 @@ lto_output_node (struct lto_simple_outpu
>     streamer_write_bitpack (&bp);
>     streamer_write_data_stream (ob->main_stream, section, strlen (section) + 1);
>   
> -  if (node->thunk.thunk_p)
> +  /* Stream thunk info always because we use it in
> +     ipa_polymorphic_call_context::ipa_polymorphic_call_context
> +     to properly interpret THIS pointers for thunks that has been converted
> +     to Gimple.  */
> +  if (node->definition)
>       {
>         streamer_write_uhwi_stream
>   	 (ob->main_stream,
> @@ -1295,7 +1299,7 @@ input_node (struct lto_file_decl_data *f
>     if (section)
>       node->set_section_for_node (section);
>   
> -  if (node->thunk.thunk_p)
> +  if (node->definition)
>       {
>         int type = streamer_read_uhwi (ib);
>         HOST_WIDE_INT fixed_offset = streamer_read_uhwi (ib);
> Index: testsuite/g++.dg/tree-prof/devirt.C
> ===================================================================
> --- testsuite/g++.dg/tree-prof/devirt.C	(nonexistent)
> +++ testsuite/g++.dg/tree-prof/devirt.C	(working copy)
> @@ -0,0 +1,123 @@
> +/* { dg-options "-O3 -fdump-tree-dom3" } */
> +struct nsISupports
> +{
> +  virtual int QueryInterface (const int &aIID, void **aInstancePtr) = 0;
> +  virtual __attribute__((noinline, noclone)) unsigned AddRef (void) = 0;
> +  virtual unsigned Release (void) = 0;
> +};
> +
> +struct nsIObserver : public nsISupports
> +{
> +  virtual int Observe (nsISupports * aSubject, const char *aTopic, const unsigned short *aData) = 0;
> +};
> +
> +struct nsISupportsWeakReference : public nsISupports
> +{
> +  virtual int GetWeakReference (void **_retval) = 0;
> +};
> +
> +struct nsSupportsWeakReference : public nsISupportsWeakReference
> +{
> +  nsSupportsWeakReference () : mProxy (0) {}
> +  virtual int GetWeakReference (void **_retval) override { return 0; }
> +  ~nsSupportsWeakReference () {}
> +  void NoticeProxyDestruction () { mProxy = nullptr; }
> +  void *mProxy;
> +  void ClearWeakReferences ();
> +  bool HasWeakReferences () const { return !!mProxy; }
> +};
> +
> +struct mozIPersonalDictionary : public nsISupports
> +{
> +  virtual int Load (void) = 0;
> +  virtual int Save (void) = 0;
> +  virtual int GetWordList (void **aWordList) = 0;
> +  virtual int Check (const int &word, bool * _retval) = 0;
> +  virtual int AddWord (const int &word) = 0;
> +  virtual int RemoveWord (const int &word) = 0;
> +  virtual int IgnoreWord (const int &word) = 0;
> +  virtual int EndSession (void) = 0;
> +};
> +
> +struct mozPersonalDictionary final
> +  : public mozIPersonalDictionary, public nsIObserver, public nsSupportsWeakReference
> +{
> +  virtual int QueryInterface (const int &aIID, void **aInstancePtr) override;
> +  virtual __attribute__((noinline, noclone)) unsigned AddRef (void) override;
> +  virtual unsigned Release (void) override;
> +  unsigned long mRefCnt;
> +  virtual int Load (void) override { return 0; }
> +  virtual int Save (void) override { return 0; }
> +  virtual int GetWordList (void **aWordList) override { return 0; }
> +  virtual int Check (const int &word, bool * _retval) override { return 0; }
> +  virtual int AddWord (const int &word) override { return 0; }
> +  virtual int RemoveWord (const int &word) override { return 0; }
> +  virtual int IgnoreWord (const int &word) override { return 0; }
> +  virtual int EndSession (void) override { return 0; }
> +  virtual int Observe (nsISupports * aSubject, const char *aTopic, const unsigned short *aData) override { return 0; }
> +  mozPersonalDictionary () : mRefCnt(0) {}
> +  int Init () { return 0; }
> +  virtual ~mozPersonalDictionary () {}
> +  bool mIsLoaded;
> +  bool mSavePending;
> +  void *mFile;
> +  char mMonitor[96];
> +  char mMonitorSave[96];
> +  char mDictionaryTable[32];
> +  char mIgnoreTable[32];
> +};
> +
> +unsigned
> +mozPersonalDictionary::AddRef (void)
> +{
> +  unsigned count = ++mRefCnt;
> +  return count;
> +}
> +
> +unsigned
> +mozPersonalDictionary::Release (void)
> +{
> +  unsigned count = --mRefCnt;
> +  if (count == 0)
> +    {
> +      mRefCnt = 1;
> +      delete (this);
> +      return 0;
> +    }
> +  return count;
> +}
> +
> +int
> +mozPersonalDictionary::QueryInterface (const int &aIID, void **aInstancePtr)
> +{
> +  nsISupports *foundInterface;
> +  if (aIID == 122)
> +    foundInterface = static_cast <mozIPersonalDictionary *>(this);
> +  else
> +    foundInterface = static_cast <nsISupportsWeakReference *>(this);
> +  int status;
> +  foundInterface->AddRef ();
> +  *aInstancePtr = foundInterface;
> +  return status;
> +}
> +
> +__attribute__((noipa)) int
> +foo (nsISupports *p, const int &i)
> +{
> +  void *q;
> +  return p->QueryInterface (i, &q);
> +}
> +
> +int
> +main ()
> +{
> +  mozPersonalDictionary m;
> +  int j = 123;
> +  for (int i = 0; i < 100000; i++)
> +    foo (static_cast <nsISupportsWeakReference *>(&m), j);
> +  if (m.mRefCnt != 100000)
> +    __builtin_abort ();
> +}
> +
This patch causes failures on aarch64-none-linux-gnu and 
arm-none-linux-gnueabihf

UNRESOLVED: g++.dg/tree-prof/devirt.C scan-ipa-dump-times dom3 "3" 
folding virtual function call to virtual unsigned int 
mozPersonalDictionary::AddRef
UNRESOLVED: g++.dg/tree-prof/devirt.C scan-ipa-dump-times dom3 "3" 
folding virtual function call to virtual unsigned int 
mozPersonalDictionary::_ZThn16

With

g++.dg/tree-prof/devirt.C: dump file does not exist

I had missed this one earlier thinking its the same with the LTO test 
failures and only realized this after the backport commit yesterday by 
Martin.

Thanks

Sudi

> +/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" "dom3" } } */
> +/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::AddRef" "dom3" } } */

Re: Fix devirtualiation in expanded thunks

[Martin Liška]

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

On 12/28/18 12:19 PM, Sudakshina Das wrote:
> Hi Jan
> 
> On 21/12/18 7:20 PM, Jan Hubicka wrote:
>> Hi,
>> this patch fixes polymorphic call analysis in thunks.  Unlike normal
>> methods, thunks take THIS pointer offsetted by a known constant. This
>> needs t be compensated for when calculating address of outer type.
>>
>> Bootstrapped/regtested x86_64-linux, also tested with Firefox where this
>> bug trigger misoptimization in spellchecker.  I plan to backport it to
>> release branches soon.
>>
>> Honza
>>
>> 	PR ipa/88561
>> 	* ipa-polymorphic-call.c
>> 	(ipa_polymorphic_call_context::ipa_polymorphic_call_context): Handle
>> 	arguments of thunks correctly.
>> 	(ipa_polymorphic_call_context::get_dynamic_context): Be ready for
>> 	NULL instance pinter.
>> 	* lto-cgraph.c (lto_output_node): Always stream thunk info.
>> 	* g++.dg/tree-prof/devirt.C: New testcase.
>> Index: ipa-polymorphic-call.c
>> ===================================================================
>> --- ipa-polymorphic-call.c	(revision 267325)
>> +++ ipa-polymorphic-call.c	(working copy)
>> @@ -995,9 +995,22 @@ ipa_polymorphic_call_context::ipa_polymo
>>   	{
>>   	  outer_type
>>   	     = TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (base_pointer)));
>> +	  cgraph_node *node = cgraph_node::get (current_function_decl);
>>   	  gcc_assert (TREE_CODE (outer_type) == RECORD_TYPE
>>   		      || TREE_CODE (outer_type) == UNION_TYPE);
>>   
>> +	  /* Handle the case we inlined into a thunk.  In this case
>> +	     thunk has THIS pointer of type bar, but it really receives
>> +	     address to its base type foo which sits in bar at
>> +	     0-thunk.fixed_offset.  It starts with code that adds
>> +	     think.fixed_offset to the pointer to compensate for this.
>> +
>> +	     Because we walked all the way to the begining of thunk, we now
>> +	     see pointer &bar-thunk.fixed_offset and need to compensate
>> +	     for it.  */
>> +	  if (node->thunk.fixed_offset)
>> +	    offset -= node->thunk.fixed_offset * BITS_PER_UNIT;
>> +
>>   	  /* Dynamic casting has possibly upcasted the type
>>   	     in the hiearchy.  In this case outer type is less
>>   	     informative than inner type and we should forget
>> @@ -1005,7 +1018,11 @@ ipa_polymorphic_call_context::ipa_polymo
>>   	  if ((otr_type
>>   	       && !contains_type_p (outer_type, offset,
>>   				    otr_type))
>> -	      || !contains_polymorphic_type_p (outer_type))
>> +	      || !contains_polymorphic_type_p (outer_type)
>> +	      /* If we compile thunk with virtual offset, the THIS pointer
>> +		 is adjusted by unknown value.  We can't thus use outer info
>> +		 at all.  */
>> +	      || node->thunk.virtual_offset_p)
>>   	    {
>>   	      outer_type = NULL;
>>   	      if (instance)
>> @@ -1030,7 +1047,15 @@ ipa_polymorphic_call_context::ipa_polymo
>>   	      maybe_in_construction = false;
>>   	    }
>>   	  if (instance)
>> -	    *instance = base_pointer;
>> +	    {
>> +	      /* If method is expanded thunk, we need to apply thunk offset
>> +		 to instance pointer.  */
>> +	      if (node->thunk.virtual_offset_p
>> +		  || node->thunk.fixed_offset)
>> +		*instance = NULL;
>> +	      else
>> +	        *instance = base_pointer;
>> +	    }
>>   	  return;
>>   	}
>>         /* Non-PODs passed by value are really passed by invisible
>> @@ -1547,6 +1572,9 @@ ipa_polymorphic_call_context::get_dynami
>>     HOST_WIDE_INT instance_offset = offset;
>>     tree instance_outer_type = outer_type;
>>   
>> +  if (!instance)
>> +    return false;
>> +
>>     if (otr_type)
>>       otr_type = TYPE_MAIN_VARIANT (otr_type);
>>   
>> Index: lto-cgraph.c
>> ===================================================================
>> --- lto-cgraph.c	(revision 267325)
>> +++ lto-cgraph.c	(working copy)
>> @@ -547,7 +547,11 @@ lto_output_node (struct lto_simple_outpu
>>     streamer_write_bitpack (&bp);
>>     streamer_write_data_stream (ob->main_stream, section, strlen (section) + 1);
>>   
>> -  if (node->thunk.thunk_p)
>> +  /* Stream thunk info always because we use it in
>> +     ipa_polymorphic_call_context::ipa_polymorphic_call_context
>> +     to properly interpret THIS pointers for thunks that has been converted
>> +     to Gimple.  */
>> +  if (node->definition)
>>       {
>>         streamer_write_uhwi_stream
>>   	 (ob->main_stream,
>> @@ -1295,7 +1299,7 @@ input_node (struct lto_file_decl_data *f
>>     if (section)
>>       node->set_section_for_node (section);
>>   
>> -  if (node->thunk.thunk_p)
>> +  if (node->definition)
>>       {
>>         int type = streamer_read_uhwi (ib);
>>         HOST_WIDE_INT fixed_offset = streamer_read_uhwi (ib);
>> Index: testsuite/g++.dg/tree-prof/devirt.C
>> ===================================================================
>> --- testsuite/g++.dg/tree-prof/devirt.C	(nonexistent)
>> +++ testsuite/g++.dg/tree-prof/devirt.C	(working copy)
>> @@ -0,0 +1,123 @@
>> +/* { dg-options "-O3 -fdump-tree-dom3" } */
>> +struct nsISupports
>> +{
>> +  virtual int QueryInterface (const int &aIID, void **aInstancePtr) = 0;
>> +  virtual __attribute__((noinline, noclone)) unsigned AddRef (void) = 0;
>> +  virtual unsigned Release (void) = 0;
>> +};
>> +
>> +struct nsIObserver : public nsISupports
>> +{
>> +  virtual int Observe (nsISupports * aSubject, const char *aTopic, const unsigned short *aData) = 0;
>> +};
>> +
>> +struct nsISupportsWeakReference : public nsISupports
>> +{
>> +  virtual int GetWeakReference (void **_retval) = 0;
>> +};
>> +
>> +struct nsSupportsWeakReference : public nsISupportsWeakReference
>> +{
>> +  nsSupportsWeakReference () : mProxy (0) {}
>> +  virtual int GetWeakReference (void **_retval) override { return 0; }
>> +  ~nsSupportsWeakReference () {}
>> +  void NoticeProxyDestruction () { mProxy = nullptr; }
>> +  void *mProxy;
>> +  void ClearWeakReferences ();
>> +  bool HasWeakReferences () const { return !!mProxy; }
>> +};
>> +
>> +struct mozIPersonalDictionary : public nsISupports
>> +{
>> +  virtual int Load (void) = 0;
>> +  virtual int Save (void) = 0;
>> +  virtual int GetWordList (void **aWordList) = 0;
>> +  virtual int Check (const int &word, bool * _retval) = 0;
>> +  virtual int AddWord (const int &word) = 0;
>> +  virtual int RemoveWord (const int &word) = 0;
>> +  virtual int IgnoreWord (const int &word) = 0;
>> +  virtual int EndSession (void) = 0;
>> +};
>> +
>> +struct mozPersonalDictionary final
>> +  : public mozIPersonalDictionary, public nsIObserver, public nsSupportsWeakReference
>> +{
>> +  virtual int QueryInterface (const int &aIID, void **aInstancePtr) override;
>> +  virtual __attribute__((noinline, noclone)) unsigned AddRef (void) override;
>> +  virtual unsigned Release (void) override;
>> +  unsigned long mRefCnt;
>> +  virtual int Load (void) override { return 0; }
>> +  virtual int Save (void) override { return 0; }
>> +  virtual int GetWordList (void **aWordList) override { return 0; }
>> +  virtual int Check (const int &word, bool * _retval) override { return 0; }
>> +  virtual int AddWord (const int &word) override { return 0; }
>> +  virtual int RemoveWord (const int &word) override { return 0; }
>> +  virtual int IgnoreWord (const int &word) override { return 0; }
>> +  virtual int EndSession (void) override { return 0; }
>> +  virtual int Observe (nsISupports * aSubject, const char *aTopic, const unsigned short *aData) override { return 0; }
>> +  mozPersonalDictionary () : mRefCnt(0) {}
>> +  int Init () { return 0; }
>> +  virtual ~mozPersonalDictionary () {}
>> +  bool mIsLoaded;
>> +  bool mSavePending;
>> +  void *mFile;
>> +  char mMonitor[96];
>> +  char mMonitorSave[96];
>> +  char mDictionaryTable[32];
>> +  char mIgnoreTable[32];
>> +};
>> +
>> +unsigned
>> +mozPersonalDictionary::AddRef (void)
>> +{
>> +  unsigned count = ++mRefCnt;
>> +  return count;
>> +}
>> +
>> +unsigned
>> +mozPersonalDictionary::Release (void)
>> +{
>> +  unsigned count = --mRefCnt;
>> +  if (count == 0)
>> +    {
>> +      mRefCnt = 1;
>> +      delete (this);
>> +      return 0;
>> +    }
>> +  return count;
>> +}
>> +
>> +int
>> +mozPersonalDictionary::QueryInterface (const int &aIID, void **aInstancePtr)
>> +{
>> +  nsISupports *foundInterface;
>> +  if (aIID == 122)
>> +    foundInterface = static_cast <mozIPersonalDictionary *>(this);
>> +  else
>> +    foundInterface = static_cast <nsISupportsWeakReference *>(this);
>> +  int status;
>> +  foundInterface->AddRef ();
>> +  *aInstancePtr = foundInterface;
>> +  return status;
>> +}
>> +
>> +__attribute__((noipa)) int
>> +foo (nsISupports *p, const int &i)
>> +{
>> +  void *q;
>> +  return p->QueryInterface (i, &q);
>> +}
>> +
>> +int
>> +main ()
>> +{
>> +  mozPersonalDictionary m;
>> +  int j = 123;
>> +  for (int i = 0; i < 100000; i++)
>> +    foo (static_cast <nsISupportsWeakReference *>(&m), j);
>> +  if (m.mRefCnt != 100000)
>> +    __builtin_abort ();
>> +}
>> +
> This patch causes failures on aarch64-none-linux-gnu and 
> arm-none-linux-gnueabihf
> 
> UNRESOLVED: g++.dg/tree-prof/devirt.C scan-ipa-dump-times dom3 "3" 
> folding virtual function call to virtual unsigned int 
> mozPersonalDictionary::AddRef
> UNRESOLVED: g++.dg/tree-prof/devirt.C scan-ipa-dump-times dom3 "3" 
> folding virtual function call to virtual unsigned int 
> mozPersonalDictionary::_ZThn16
> 
> With
> 
> g++.dg/tree-prof/devirt.C: dump file does not exist
> 
> I had missed this one earlier thinking its the same with the LTO test 
> failures and only realized this after the backport commit yesterday by 
> Martin.
> 
> Thanks

Thanks for heads up. I'm going to install following obvious fix.

Martin

> 
> Sudi
> 
>> +/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" "dom3" } } */
>> +/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::AddRef" "dom3" } } */


[-- Attachment #2: 0001-Fix-scan-pattern-of-a-test-case.patch --]
[-- Type: text/x-patch, Size: 1561 bytes --]

From 25ad262ed646778a3ea39b56a58a9bcf6d9f9c52 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Mon, 31 Dec 2018 15:06:40 +0100
Subject: [PATCH] Fix scan pattern of a test-case.

gcc/testsuite/ChangeLog:

2018-12-31  Martin Liska  <mliska@suse.cz>

	* g++.dg/tree-prof/devirt.C: Fix scan pattern and test options.
---
 gcc/testsuite/g++.dg/tree-prof/devirt.C | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/g++.dg/tree-prof/devirt.C b/gcc/testsuite/g++.dg/tree-prof/devirt.C
index 05c9a26e7a4..86cba41452e 100644
--- a/gcc/testsuite/g++.dg/tree-prof/devirt.C
+++ b/gcc/testsuite/g++.dg/tree-prof/devirt.C
@@ -1,4 +1,4 @@
-/* { dg-options "-O3 -fdump-tree-dom3" } */
+/* { dg-options "-O3 -fdump-tree-dom3-details" } */
 struct nsISupports
 {
   virtual int QueryInterface (const int &aIID, void **aInstancePtr) = 0;
@@ -119,5 +119,5 @@ main ()
     __builtin_abort ();
 }
 
-/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" "dom3" } } */
-/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::AddRef" "dom3" } } */
+/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 3 "dom3" } } */
+/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function call to virtual unsigned int mozPersonalDictionary::AddRef" 3 "dom3" } } */
-- 
2.20.1

Re: Fix devirtualiation in expanded thunks

[Andreas Schwab]

On Dez 31 2018, Martin Liška <mliska@suse.cz> wrote:

> -/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" "dom3" } } */
> -/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::AddRef" "dom3" } } */
> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 3 "dom3" } } */
> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function call to virtual unsigned int mozPersonalDictionary::AddRef" 3 "dom3" } } */

New tests that FAIL (1 tests):

unix/-mabi=ilp32: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 3

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Fix devirtualiation in expanded thunks

[Rainer Orth]

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Dez 31 2018, Martin Liška <mliska@suse.cz> wrote:
>
>> -/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual
>> function call to virtual unsigned int mozPersonalDictionary::_ZThn16"
>> "dom3" } } */
>> -/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual
>> function call to virtual unsigned int mozPersonalDictionary::AddRef"
>> "dom3" } } */
>> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual
>> function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 3
>> "dom3" } } */
>> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual
>> function call to virtual unsigned int mozPersonalDictionary::AddRef" 3
>> "dom3" } } */
>
> New tests that FAIL (1 tests):
>
> unix/-mabi=ilp32: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3
> "folding virtual function call to virtual unsigned int
> mozPersonalDictionary::_ZThn16" 3

now it's 

+FAIL: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 1

and the failure seems to occur on every 32-bit target.  The log shows

g++.dg/tree-prof/devirt.C: pattern found 0 times

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Fix devirtualiation in expanded thunks

[Jan Hubicka]

> Andreas Schwab <schwab@linux-m68k.org> writes:
> 
> > On Dez 31 2018, Martin Liška <mliska@suse.cz> wrote:
> >
> >> -/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual
> >> function call to virtual unsigned int mozPersonalDictionary::_ZThn16"
> >> "dom3" } } */
> >> -/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual
> >> function call to virtual unsigned int mozPersonalDictionary::AddRef"
> >> "dom3" } } */
> >> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual
> >> function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 3
> >> "dom3" } } */
> >> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual
> >> function call to virtual unsigned int mozPersonalDictionary::AddRef" 3
> >> "dom3" } } */
> >
> > New tests that FAIL (1 tests):
> >
> > unix/-mabi=ilp32: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3
> > "folding virtual function call to virtual unsigned int
> > mozPersonalDictionary::_ZThn16" 3
> 
> now it's 
> 
> +FAIL: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3 "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 1
> 
> and the failure seems to occur on every 32-bit target.  The log shows
> 
> g++.dg/tree-prof/devirt.C: pattern found 0 times

I guess the ZThn16 should be replaced by ZThm8?
Is there a way to dump files for different patterns for 32bit or 64bit
tarets? Or shall we simply disable this test for 32bit?

Honza
> 
> 	Rainer
> 
> -- 
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Fix devirtualiation in expanded thunks

[Rainer Orth]

Hi Jan,

>> Andreas Schwab <schwab@linux-m68k.org> writes:
>> 
>> > On Dez 31 2018, Martin Liška <mliska@suse.cz> wrote:
>> >
>> >> -/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual
>> >> function call to virtual unsigned int mozPersonalDictionary::_ZThn16"
>> >> "dom3" } } */
>> >> -/* { dg-final-use-not-autofdo { scan-ipa-dump-times 3 "folding virtual
>> >> function call to virtual unsigned int mozPersonalDictionary::AddRef"
>> >> "dom3" } } */
>> >> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual
>> >> function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 3
>> >> "dom3" } } */
>> >> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual
>> >> function call to virtual unsigned int mozPersonalDictionary::AddRef" 3
>> >> "dom3" } } */
>> >
>> > New tests that FAIL (1 tests):
>> >
>> > unix/-mabi=ilp32: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3
>> > "folding virtual function call to virtual unsigned int
>> > mozPersonalDictionary::_ZThn16" 3
>> 
>> now it's 
>> 
>> +FAIL: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3 "folding
>> virtual function call to virtual unsigned int
>> mozPersonalDictionary::_ZThn16" 1
>> 
>> and the failure seems to occur on every 32-bit target.  The log shows
>> 
>> g++.dg/tree-prof/devirt.C: pattern found 0 times
>
> I guess the ZThn16 should be replaced by ZThm8?

not exactly: the dump file has

/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/tree-prof/devirt.C:99:26: optimized: folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn8_N21mozPersonalDictionary6AddRefEv()

> Is there a way to dump files for different patterns for 32bit or 64bit

Why would we: many similar scans just use _ZThn\[0-9\]+ instead.

> tarets? Or shall we simply disable this test for 32bit?

That's counterproductive, I believe.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Fix devirtualiation in expanded thunks

[Jan Hubicka]

> /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/tree-prof/devirt.C:99:26: optimized: folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn8_N21mozPersonalDictionary6AddRefEv()
> 
> > Is there a way to dump files for different patterns for 32bit or 64bit
> 
> Why would we: many similar scans just use _ZThn\[0-9\]+ instead.

Becuase the testcase was formely dispatching into ZThn8 (or some other
constant I forgot) instead of ZThn16 causing wrong code.

Well, there is runtime test that right thunk is used, so perhaps we can
just drop the part maching the actual value since most probably testcase
will ICE if wrong variant is picked again.

Honza
> 
> > tarets? Or shall we simply disable this test for 32bit?
> 
> That's counterproductive, I believe.
> 
> 	Rainer
> 
> -- 
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University

Re: Fix devirtualiation in expanded thunks

[Jakub Jelinek]

On Wed, Jan 02, 2019 at 12:15:53AM +0100, Jan Hubicka wrote:
> > /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/tree-prof/devirt.C:99:26: optimized: folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn8_N21mozPersonalDictionary6AddRefEv()
> > 
> > > Is there a way to dump files for different patterns for 32bit or 64bit
> > 
> > Why would we: many similar scans just use _ZThn\[0-9\]+ instead.
> 
> Becuase the testcase was formely dispatching into ZThn8 (or some other
> constant I forgot) instead of ZThn16 causing wrong code.
> 
> Well, there is runtime test that right thunk is used, so perhaps we can
> just drop the part maching the actual value since most probably testcase
> will ICE if wrong variant is picked again.

The testcase was written so that it aborts if it is miscompiled and succeeds
otherwise, so I agree there is no urgent need for further dg-final scans on it.

I don't think we have effective targets for various exact pointer sizes
though, what we could do to cover most of the targets is e.g. following.
Tested on x86_64-linux with both -m64 and -m32, ok for trunk?

2019-01-02  Jakub Jelinek  <jakub@redhat.com>

	PR ipa/88561
	* g++.dg/tree-prof/devirt.C: Expect _ZThn16 only for lp64 and llp64 targets
	and expect _ZThn8 for ilp32 targets.

--- gcc/testsuite/g++.dg/tree-prof/devirt.C.jj	2019-01-01 12:14:22.973597202 +0100
+++ gcc/testsuite/g++.dg/tree-prof/devirt.C	2019-01-02 10:07:42.389899657 +0100
@@ -1,4 +1,6 @@
+/* PR ipa/88561 */
 /* { dg-options "-O3 -fdump-tree-dom3-details" } */
+
 struct nsISupports
 {
   virtual int QueryInterface (const int &aIID, void **aInstancePtr) = 0;
@@ -119,5 +121,6 @@ main ()
     __builtin_abort ();
 }
 
-/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 1 "dom3" } } */
+/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn16" 1 "dom3" { target { lp64 || llp64 } } } } */
+/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn8" 1 "dom3" { target ilp32 } } } */
 /* { dg-final-use-not-autofdo { scan-tree-dump-times "folding virtual function call to virtual unsigned int mozPersonalDictionary::AddRef" 1 "dom3" } } */


	Jakub

Re: Fix devirtualiation in expanded thunks

[Jan Hubicka]

> On Wed, Jan 02, 2019 at 12:15:53AM +0100, Jan Hubicka wrote:
> > > /vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/tree-prof/devirt.C:99:26: optimized: folding virtual function call to virtual unsigned int mozPersonalDictionary::_ZThn8_N21mozPersonalDictionary6AddRefEv()
> > > 
> > > > Is there a way to dump files for different patterns for 32bit or 64bit
> > > 
> > > Why would we: many similar scans just use _ZThn\[0-9\]+ instead.
> > 
> > Becuase the testcase was formely dispatching into ZThn8 (or some other
> > constant I forgot) instead of ZThn16 causing wrong code.
> > 
> > Well, there is runtime test that right thunk is used, so perhaps we can
> > just drop the part maching the actual value since most probably testcase
> > will ICE if wrong variant is picked again.
> 
> The testcase was written so that it aborts if it is miscompiled and succeeds
> otherwise, so I agree there is no urgent need for further dg-final scans on it.
> 
> I don't think we have effective targets for various exact pointer sizes
> though, what we could do to cover most of the targets is e.g. following.
> Tested on x86_64-linux with both -m64 and -m32, ok for trunk?
> 
> 2019-01-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR ipa/88561
> 	* g++.dg/tree-prof/devirt.C: Expect _ZThn16 only for lp64 and llp64 targets
> 	and expect _ZThn8 for ilp32 targets.

Looks good to me, thanks for working on it.
Another option would be to just remove the 16 from string so we check
that devirtualization happens and at runtime we check that it goes right
way.

Honza