pcc added inline comments.
================ Comment at: clang/test/CodeGenCXX/type-metadata-thinlto.cpp:2 +// RUN: %clang_cc1 -flto=thin -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm-bc -o %t %s +// RUN: llvm-modextract -o - -n 1 %t | llvm-dis | FileCheck %s + ---------------- tejohnson wrote: > mehdi_amini wrote: > > tejohnson wrote: > > > mehdi_amini wrote: > > > > pcc wrote: > > > > > mehdi_amini wrote: > > > > > > pcc wrote: > > > > > > > mehdi_amini wrote: > > > > > > > > tejohnson wrote: > > > > > > > > > Is it the case that now we will always split the module with > > > > > > > > > this change? Should that only be done under CFI options? > > > > > > > > Devirtualization may happen whenever you have a hidden virtual > > > > > > > > table IIUC, independently of CFI. > > > > > > > To be more precise: we normally add type metadata in LTO mode > > > > > > > when the class has hidden visibility. See: > > > > > > > http://clang.llvm.org/docs/LTOVisibility.html > > > > > > > > > > > > > > That doesn't necessarily imply devirtualization, which is > > > > > > > controlled by the flag `-fwhole-program-vtables`. > > > > > > So with hidden visibility but without CFI or > > > > > > -fwhole-program-vtables, do we split the module? What's the purpose? > > > > > At the moment we would. The purpose is to simplify the overall > > > > > interface. If I want to compile a subset of my TUs without CFI or > > > > > devirtualization, I should be able to do that by enabling LTO but not > > > > > passing the CFI or devirtualization flags. In that case the vtables > > > > > themselves should still have type metadata so that TUs compiled > > > > > without CFI/devirtualization can correctly interoperate with TUs > > > > > compiled with CFI/devirtualization (to handle the cases where a class > > > > > defined in a TU compiled without CFI/devirt is used by code compiled > > > > > with LTO/devirt, or where the linker/LTO selects a linkonce_odr > > > > > vtable from a TU compiled without CFI/devirt). > > > > > > > > > > I'd be open to changing the command line interface so that an > > > > > additional flag may be used to control the scope of the "LTO unit" > > > > > and which would just enable type metadata, but ideally I'd like to > > > > > keep things relatively simple. > > > > > At the moment we would. The purpose is to simplify the overall > > > > > interface. > > > > > > > > Right, if it was the only reason, I wouldn't be in favor, but you raise > > > > a real use case below. > > > > > > > > > If I want to compile a subset of my TUs without CFI or > > > > > devirtualization, I should be able to do that by enabling LTO but not > > > > > passing the CFI or devirtualization flags. > > > > > > > > Right, seems legit. > > > > > > > > > In that case the vtables themselves should still have type metadata > > > > > so that TUs compiled without CFI/devirtualization can correctly > > > > > interoperate with TUs compiled with CFI/devirtualization > > > > > > > > That's what I wasn't sure about :) > > > > > > > > > (to handle the cases where a class defined in a TU compiled without > > > > > CFI/devirt is used by code compiled with LTO/devirt, or where the > > > > > linker/LTO selects a linkonce_odr vtable from a TU compiled without > > > > > CFI/devirt). > > > > > > > > Make sense, LGTM as is with this explanation! > > > > Thanks. > > > I would like some way to disable this, at least for debugging. > > CC1 option then? > > CC1 option then? > > Sure D28877 https://reviews.llvm.org/D28843 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
