r307232 - [modules ts] Do not emit strong function definitions from the module interface unit in every user.
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Sun Sep 5 11:28:43 PDT 2021
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 <dblaikie at 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 at 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210905/a2c499cd/attachment-0001.html>
More information about the cfe-commits
mailing list