rjmccall added a comment. In D154658#4482202 <https://reviews.llvm.org/D154658#4482202>, @rsmith wrote:
> In D154658#4481225 <https://reviews.llvm.org/D154658#4481225>, @rjmccall > wrote: > >> In D154658#4479213 <https://reviews.llvm.org/D154658#4479213>, @rsmith wrote: >> >>> I think (hope?) we should be able to apply this to a much larger set of >>> cases. Would it be correct to do this optimization unless the vtable might >>> be emitted with vague linkage and non-default visibility (that is, unless >>> we're in the odd case where people expect non-default visibility classes to >>> be the same type across DSOs)? Or are there cases where we might be using a >>> vtable that (eg) doesn't even have the right symbol? >> >> I don't know of any problems other than the total failure of vague linkage >> across DSO boundaries, so if we just treat that as an implicit exception to >> the vtable uniqueness guarantee in the ABI, I think you've got the condition >> exactly right: we could do this for any class where either the v-table >> doesn't have vague linkage or the class's formal visibility is not >> `default`. Our experience at Apple with enforcing type visibility is that >> it's usually good for one or two bug reports a year, but it's pretty easy to >> explain to users what they did wrong, and Clang's visibility attributes are >> pretty simple to use. (`type_visibility` helps a lot with managing >> tradeoffs.) > > I did some checking through the Clang implementation and found another two > cases: > > - under `-fapple-kext`, vague-linkage vtables get emitted in each translation > unit that references them > - under `-fno-rtti`, identical vtables for distinct types could get merged > because we emit vtables as `unnamed_addr` (this doesn't affect > `dynamic_cast`, because `-fno-rtti` also disables `dynamic_cast` entirely) > > The good news seems to be that if you don't use any language extensions (type > visibility, `-fno-rtti`, `-fapple-kext`), then we do actually provide the > guarantee that the ABI describes. :) > > In D154658#4479170 <https://reviews.llvm.org/D154658#4479170>, @rjmccall > wrote: > >> If there are multiple subobjects of the source type in the destination type, >> consider just casting to `void*` first instead of doing multiple comparisons. > > This turned out to be a little subtle: the vptr comparison we do after the > cast requires loading a vptr of an object of entirely-unknown type. This is > the first time we're doing that in IR generation, and `GetVTablePtr` expected > to be told which class it's loading the vtable for (presumably, so that it > can load from the right offset within the class). However, the implementation > of `GetVTablePtr` didn't use the class for anything; it's always at the start > for all of our supported ABIs. But this patch would make it harder to support > an ABI that didn't put the vptr at the start of the most-derived object. Oh, this does matter on platforms using pointer authentication, because each vptr field is signed with a constant discriminator derived from the name of the class that introduced it. ================ Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:204 + // another type. + if (!CGM.GetAddrOfRTTIDescriptor(RecordTy)) + return false; ---------------- Is there no reasonable way of checking this without actually triggering the emission of RTTI? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154658/new/ https://reviews.llvm.org/D154658 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
