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 Jul 16 20:26:57 PDT 2017


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/20170717/84203694/attachment-0001.html>


More information about the cfe-commits mailing list