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