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
