From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111314 invoked by alias); 31 Dec 2018 14:10:58 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 111300 invoked by uid 89); 31 Dec 2018 14:10:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,SPF_PASS autolearn=ham version=3.3.2 spammy=expanded, Das, Load, missed X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 31 Dec 2018 14:10:54 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 92B04AD43; Mon, 31 Dec 2018 14:10:51 +0000 (UTC) Subject: Re: Fix devirtualiation in expanded thunks To: Sudakshina Das , Jan Hubicka , "stransky@redhat.com" , "jakub@redhat.com" , "gcc-patches@gcc.gnu.org" Cc: nd References: <20181221192053.ut4rfit4mxtktklo@kam.mff.cuni.cz> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <5553c6a5-dd54-b9f1-4d5e-c9e8cfbe9be8@suse.cz> Date: Mon, 31 Dec 2018 15:13:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.3 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------34D310F0BC3B3132FE66C687" X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg01794.txt.bz2 This is a multi-part message in MIME format. --------------34D310F0BC3B3132FE66C687 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-length: 9927 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 (this); >> + else >> + foundInterface = static_cast (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 (&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" } } */ --------------34D310F0BC3B3132FE66C687 Content-Type: text/x-patch; name="0001-Fix-scan-pattern-of-a-test-case.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Fix-scan-pattern-of-a-test-case.patch" Content-length: 1562 >From 25ad262ed646778a3ea39b56a58a9bcf6d9f9c52 Mon Sep 17 00:00:00 2001 From: marxin 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 * 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 --------------34D310F0BC3B3132FE66C687--