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
Tue Sep 21 19:58:10 PDT 2021
Ping
On Sun, Sep 5, 2021 at 11:28 AM David Blaikie <dblaikie at 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 <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/20210921/d621fffb/attachment-0001.html>
More information about the cfe-commits
mailing list