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
Mon Nov 1 13:36:43 PDT 2021
Ping
On Tue, Sep 21, 2021 at 7:58 PM David Blaikie <dblaikie at gmail.com> wrote:
> 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/20211101/ae23074b/attachment-0001.html>
More information about the cfe-commits
mailing list