Ping On Tue, Sep 21, 2021 at 7:58 PM David Blaikie <dblai...@gmail.com> wrote:
> Ping > > On Sun, Sep 5, 2021 at 11:28 AM David Blaikie <dblai...@gmail.com> wrote: > >> Hey Richard - was just going back over some of the modular codegen code >> (due to a discussion on the EWG mailing list about file extensions that >> ended up touching on the nature of how modules are built) - and I came >> across this code & had the same question I see I wrote up here already but >> got lost in the *-commits mail. >> >> Wondering if you've got some thoughts on why this choice was implemented >> for C++20 modules - not homing inline functions/variables, only the extern >> linkage ones for correctness? >> >> On Sun, Jul 16, 2017 at 8:26 PM David Blaikie <dblai...@gmail.com> wrote: >> >>> Looks good - does this support available_externally definitions of >>> strong external linkage functions in the users of a module? (is that >>> tested?) Should it? >>> >>> Also should we consider having two flags for modular codegen - one for >>> correctness (external function definitions), one for linkage size >>> optimization (inline functions). Given the current data on optimized builds >>> with inline functions (that it hurts object size to emit >>> weak+available_externally definitions rather than linkonce_odr because so >>> many definitions are optimized away entirely, that the bytes for the weak >>> definition are wasted/unnecessary) - or at least something to keep in >>> mind/run numbers on in the future for more generic codebases than Google's >>> protobuf-heavy (& only protobuf modularized) code. >>> >>> On Wed, Jul 5, 2017 at 5:30 PM Richard Smith via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> >>>> Author: rsmith >>>> Date: Wed Jul 5 17:30:00 2017 >>>> New Revision: 307232 >>>> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=307232&view=rev >>>> Log: >>>> [modules ts] Do not emit strong function definitions from the module >>>> interface unit in every user. >>>> >>>> Added: >>>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/ >>>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/ >>>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp >>>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm >>>> cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp >>>> Modified: >>>> cfe/trunk/lib/Serialization/ASTWriterDecl.cpp >>>> >>>> Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=307232&r1=307231&r2=307232&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original) >>>> +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Wed Jul 5 17:30:00 >>>> 2017 >>>> @@ -2233,8 +2233,18 @@ void ASTRecordWriter::AddFunctionDefinit >>>> Writer->ClearSwitchCaseIDs(); >>>> >>>> assert(FD->doesThisDeclarationHaveABody()); >>>> - bool ModulesCodegen = Writer->Context->getLangOpts().ModulesCodegen >>>> && >>>> - Writer->WritingModule && >>>> !FD->isDependentContext(); >>>> + bool ModulesCodegen = false; >>>> + if (Writer->WritingModule && !FD->isDependentContext()) { >>>> + // Under -fmodules-codegen, codegen is performed for all defined >>>> functions. >>>> + // When building a C++ Modules TS module interface unit, a strong >>>> definition >>>> + // in the module interface is provided by the compilation of that >>>> module >>>> + // interface unit, not by its users. (Inline functions are still >>>> emitted >>>> + // in module users.) >>>> + ModulesCodegen = >>>> + Writer->Context->getLangOpts().ModulesCodegen || >>>> + (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit && >>>> + Writer->Context->GetGVALinkageForFunction(FD) == >>>> GVA_StrongExternal); >>>> + } >>>> Record->push_back(ModulesCodegen); >>>> if (ModulesCodegen) >>>> Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(FD)); >>>> >>>> Added: cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp?rev=307232&view=auto >>>> >>>> ============================================================================== >>>> --- cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp >>>> (added) >>>> +++ cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp Wed >>>> Jul 5 17:30:00 2017 >>>> @@ -0,0 +1,23 @@ >>>> +// RUN: %clang_cc1 -fmodules-ts %S/module.cppm -triple >>>> %itanium_abi_triple -emit-module-interface -o %t >>>> +// RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple >>>> -fmodule-file=%t -emit-llvm -o - | FileCheck %s --implicit-check-not=unused >>>> --implicit-check-not=global_module >>>> + >>>> +module Module; >>>> + >>>> +void use() { >>>> + // CHECK: define linkonce_odr {{.*}}@_Z20used_inline_exportedv >>>> + used_inline_exported(); >>>> + // CHECK: declare {{.*}}@_Z18noninline_exportedv >>>> + noninline_exported(); >>>> + >>>> + // FIXME: This symbol should not be visible here. >>>> + // CHECK: define internal {{.*}}@_ZL26used_static_module_linkagev >>>> + used_static_module_linkage(); >>>> + >>>> + // FIXME: The module name should be mangled into the name of this >>>> function. >>>> + // CHECK: define linkonce_odr {{.*}}@_Z26used_inline_module_linkagev >>>> + used_inline_module_linkage(); >>>> + >>>> + // FIXME: The module name should be mangled into the name of this >>>> function. >>>> + // CHECK: declare {{.*}}@_Z24noninline_module_linkagev >>>> + noninline_module_linkage(); >>>> +} >>>> >>>> Added: cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm?rev=307232&view=auto >>>> >>>> ============================================================================== >>>> --- cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm >>>> (added) >>>> +++ cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm >>>> Wed Jul 5 17:30:00 2017 >>>> @@ -0,0 +1,54 @@ >>>> +// RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple >>>> -emit-llvm -o - | FileCheck %s --implicit-check-not=unused >>>> + >>>> +static void unused_static_global_module() {} >>>> +static void used_static_global_module() {} >>>> +inline void unused_inline_global_module() {} >>>> +inline void used_inline_global_module() {} >>>> +// CHECK: define void {{.*}}@_Z23noninline_global_modulev >>>> +void noninline_global_module() { >>>> + // FIXME: This should be promoted to module linkage and given a >>>> + // module-mangled name, if it's called from an inline function within >>>> + // the module interface. >>>> + // (We should try to avoid this when it's not reachable from outside >>>> + // the module interface unit.) >>>> + // CHECK: define internal {{.*}}@_ZL25used_static_global_modulev >>>> + used_static_global_module(); >>>> + // CHECK: define linkonce_odr {{.*}}@_Z25used_inline_global_modulev >>>> + used_inline_global_module(); >>>> +} >>>> + >>>> +export module Module; >>>> + >>>> +export { >>>> + // FIXME: These should be ill-formed: you can't export an internal >>>> linkage >>>> + // symbol, per [dcl.module.interface]p2. >>>> + static void unused_static_exported() {} >>>> + static void used_static_exported() {} >>>> + >>>> + inline void unused_inline_exported() {} >>>> + inline void used_inline_exported() {} >>>> + // CHECK: define void {{.*}}@_Z18noninline_exportedv >>>> + void noninline_exported() { >>>> + // CHECK: define internal {{.*}}@_ZL20used_static_exportedv >>>> + used_static_exported(); >>>> + // CHECK: define linkonce_odr {{.*}}@_Z20used_inline_exportedv >>>> + used_inline_exported(); >>>> + } >>>> +} >>>> + >>>> +static void unused_static_module_linkage() {} >>>> +static void used_static_module_linkage() {} >>>> +inline void unused_inline_module_linkage() {} >>>> +inline void used_inline_module_linkage() {} >>>> +// FIXME: The module name should be mangled into the name of this >>>> function. >>>> +// CHECK: define void {{.*}}@_Z24noninline_module_linkagev >>>> +void noninline_module_linkage() { >>>> + // FIXME: This should be promoted to module linkage and given a >>>> + // module-mangled name, if it's called from an inline function within >>>> + // the module interface. >>>> + // CHECK: define internal {{.*}}@_ZL26used_static_module_linkagev >>>> + used_static_module_linkage(); >>>> + // FIXME: The module name should be mangled into the name of this >>>> function. >>>> + // CHECK: define linkonce_odr {{.*}}@_Z26used_inline_module_linkagev >>>> + used_inline_module_linkage(); >>>> +} >>>> >>>> Added: cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp?rev=307232&view=auto >>>> >>>> ============================================================================== >>>> --- cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp >>>> (added) >>>> +++ cfe/trunk/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp Wed >>>> Jul 5 17:30:00 2017 >>>> @@ -0,0 +1,13 @@ >>>> +// RUN: %clang_cc1 -fmodules-ts %S/module.cppm -triple >>>> %itanium_abi_triple -emit-module-interface -o %t >>>> +// RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple >>>> -fmodule-file=%t -emit-llvm -o - | FileCheck %s --implicit-check-not=unused >>>> --implicit-check-not=global_module >>>> + >>>> +import Module; >>>> + >>>> +void use() { >>>> + // CHECK: define linkonce_odr {{.*}}@_Z20used_inline_exportedv >>>> + used_inline_exported(); >>>> + // CHECK: declare {{.*}}@_Z18noninline_exportedv >>>> + noninline_exported(); >>>> + >>>> + // Module-linkage declarations are not visible here. >>>> +} >>>> >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> cfe-commits@lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>> >>>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits