public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Feng Xue OS <fxue@os.amperecomputing.com>
To: Jason Merrill <jason@redhat.com>, Jan Hubicka <hubicka@ucw.cz>,
	"mjambor@suse.cz" <mjambor@suse.cz>,
	Richard Biener <rguenther@suse.de>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH/RFC 1/2] WPD: Enable whole program devirtualization
Date: Sat, 18 Sep 2021 09:38:39 +0000	[thread overview]
Message-ID: <SN6PR01MB4958A3BD0F68EA736759A28BF7DE9@SN6PR01MB4958.prod.exchangelabs.com> (raw)
In-Reply-To: <59eb3921-a948-613f-cf47-96e764626e42@redhat.com>

>On 9/16/21 22:29, Feng Xue OS wrote:
>>> On 9/16/21 05:25, Feng Xue OS via Gcc-patches wrote:
>>>> This and following patches are composed to enable full devirtualization
>>>> under whole program assumption (so also called whole-program
>>>> devirtualization, WPD for short), which is an enhancement to current
>>>> speculative devirtualization. The base of the optimization is how to
>>>> identify class type that is local in terms of whole-program scope, at
>>>> least  those class types in libstdc++ must be excluded in some way.
>>>> Our means is to use typeinfo symbol as identity marker of a class since
>>>> it is unique and always generated once the class or its derived type
>>>> is instantiated somewhere, and rely on symbol resolution by
>>>> lto-linker-plugin to detect whether  a typeinfo is referenced by regular
>>>> object/library, which indirectly tells class types are escaped or not.
>>>> The RFC at https://gcc.gnu.org/pipermail/gcc/2021-August/237132.html
>>>> gives more details on that.
>>>>
>>>> Bootstrapped/regtested on x86_64-linux and aarch64-linux.
>>>>
>>>> Thanks,
>>>> Feng
>>>>
>>>> ----
>>>> 2021-09-07  Feng Xue  <fxue@os.amperecomputing.com>
>>>>
>>>> gcc/
>>>>        * common.opt (-fdevirtualize-fully): New option.
>>>>        * class.c (build_rtti_vtbl_entries): Force generation of typeinfo
>>>>        even -fno-rtti is specificied under full devirtualization.
>>>
>>> This makes -fno-rtti useless; rather than this, you should warn about
>>> the combination of flags and force flag_rtti on.  It also sounds like
>>> you depend on the library not being built with -fno-rtti.
>>
>> Although rtti is generated by front-end, we will remove it after lto symtab
>> merge, which is meant to keep same behavior as -fno-rtti.
>
> Ah, the cp/ change is OK, then, with a comment about that.
>
>> Yes, regular library to be linked with should contain rtti data, otherwise
>> WPD could not deduce class type usage safely. By default, we can think
>> that it should work for libstdc++, but it probably becomes a problem for
>> user library, which might be avoided if we properly document this
>> requirement and suggest user doing that when using WPD.
>
> Yes, I would expect that external libraries would be built with RTTI on
> to allow users to use RTTI features even if they aren't used within the
> library.  But it's good to document it as a requirement.
>
>> +           /* If a class with virtual base is only instantiated as
>> +              subobjects of derived classes, and has no complete object in
>> +              compilation unit, merely construction vtables will be involved,
>> +              its primary vtable is really not needed, and subject to being
>> +              removed.  So once a vtable node is encountered, for all
>> +              polymorphic base classes of the vtable's context class, always
>> +              force generation of primary vtable nodes when full
>> +              devirtualization is enabled.  */
>
> Why do you need the primary vtable if you're relying on RTTI info?
> Construction vtables will point to the same RTTI node.

At middle end, the easiest way to get vtable of type is via TYPE_BINFO(type),
it is the primary one. And WPD relies on existence of varpool_node of the
vtable decl to determine if the type has been removed (when it is never
instantiated), so we will force generation of vtable node at very early stage.
Additionally, construction vtable (C-in-D) belongs to the class (D) of complete
object, not the class (C) of subobject actually being constructed for, it is not
easy to correlate construction vtable with the subobject class (C) after front
end.

>
>> +       /* Public class w/o key member function (or local class in a public
>> +          inline function) requires COMDAT-like vtable so as to be shared
>> +          among units.  But C++ privatizing via -fno-weak would introduce
>> +          multiple static vtable copies for one class in merged lto symbol
>> +          table.  This breaks one-to-one correspondence between class and
>> +          vtable, and makes class liveness check become not that easy.  To
>> +          be simple, we exclude such kind of class from our choice list.
>
> Same question.  Also, why would you use -fno-weak?  Forcing multiple
> copies of things we're perfectly capable of combining seems like a
> strange choice.  You can privatize things with the symbol visibility
> controls or RTLD_LOCAL.

We expect that user does not specify -fno-weak for WPD. But if
specified, we should correctly handle that and bypass the type. And
indeed there is no need to force generation of vtable under this
situation.  But if vtable is not keyed to any compilation unit, we might
never have any copy of it in ordinary build, while its class type is
meaningful to whole-program analysis, such as an abstract root class.

Thanks,
Feng

  reply	other threads:[~2021-09-18  9:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-16  9:25 Feng Xue OS
2021-09-16 17:53 ` Jason Merrill
2021-09-17  2:29   ` Feng Xue OS
2021-09-18  5:53     ` Jason Merrill
2021-09-18  9:38       ` Feng Xue OS [this message]
2021-09-30  3:57         ` PING: " Feng Xue OS
2021-10-15  3:49           ` PING^2: " Feng Xue OS

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SN6PR01MB4958A3BD0F68EA736759A28BF7DE9@SN6PR01MB4958.prod.exchangelabs.com \
    --to=fxue@os.amperecomputing.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=jason@redhat.com \
    --cc=mjambor@suse.cz \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).