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