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" }
> } */
