-fmodules-codegen should not emit extern templates

[llunak]

Summary

See the test for a testcase. If a header contains 'extern template', then the template should be provided somewhere by an explicit instantiation, so it is not necessary to generate a copy. Worse (at least with PCH -building-pch-with-obj), the current code actually leads to an unresolved symbol, because the PCH's object file will not actually contain functions from such a template (because of the GVA_AvailableExternally?), but the object file for the explicit instantiation will not contain them either because it will be blocked by the information provided by the PCH. I don't know if the same problem exists with modules (nor I know how to check for sure), all tests pass. If the problem is not there for modules, I can make the change PCH-specific.

[llunak]

Ping.

[dblaikie]

Hey - thanks for this! Does look like it reproduces in modules:

foo.h:

#pragma once

template <typename T>
struct outer {
static void func() {
}
};

template<typename T>
void func() {
}

extern template struct outer<int>;
extern template void func<int>();

inline void caller() {
  outer<int>::func();
  func<int>();
}

foo.modulemap:

module foo { header "foo.h" }

use.cpp:

#include "foo.h"
template struct outer<int>;
template void func<int>();
int main() {
}

Commands:

clang-tot -cc1 -fmodules-codegen -fmodules -emit-module -fmodule-name=foo foo.modulemap  -x c++ -o foo.pcm
clang-tot -cc1 -fmodules -fmodule-file=foo.pcm use.cpp -emit-obj
clang-tot -c foo.pcm
clang-tot foo.o use.o

This test case exercises both a member function of a class template, and a standalone function template. I'd like to better understand why this shows up for function templates but not member functions of class templates - perhaps there's somewhere the solution to both cases can be unified.

Ah, if I mark the standalone function template 'inline' (the implicit linkage of member functions) then I get the same failure for both. Haven't tested whether the fix is the same fix for both yet.

[llunak]

In D69779#1801771, @dblaikie wrote:

Ah, if I mark the standalone function template 'inline' (the implicit linkage of member functions) then I get the same failure for both. Haven't tested whether the fix is the same fix for both yet.

With my patch there are no errors with your testcase, even with the 'inline' added.

[llunak]

Do you need some more information about the patch? It'd be nice if this could make it into 10.0.

[dblaikie]

clang/test/PCH/codegen-extern-template.cpp

1–9 ↗

(On Diff #227635)

Please reduce the test to not rely on linking - it should check for the IR output of clang (not object output) for the symbols that should/shouldn't be present.

Specifically the main function is probably unneeded - and only the explicit template specialization definition is needed. The test would then verify that the required function definition is present in the IR output.

Also - to simplify this, could you change this to use an inline function template - rather than a member function? It'd make it more clear that it's the function template with 'inline' (rather than relying on the implicit inline from member functions).

clang/test/PCH/codegen-extern-template.h

1–16 ↗

(On Diff #227635)

Please run this (& the .cpp file) through clang-format, and change the #pragma once to standard header include guards (#define/etc).

[llunak]

I've updated the test as requested.

However I've noticed that a PCH-based test for this relies on https://reviews.llvm.org/D69585 . The fix works (of course), but it requires a PCH/module with the instantiation.

[llunak]

This version uses a module based on the code posted above.

[dblaikie]

Looks good - thanks for your patience!

clang/lib/Serialization/ASTWriterDecl.cpp

if (Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
       // 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.)
       Linkage = Writer->Context->GetGVALinkageForFunction(FD);
       ModulesCodegen = *Linkage == GVA_StrongExternal;
     }
     if (Writer->Context->getLangOpts().ModulesCodegen) {
       // Under -fmodules-codegen, codegen is performed for all non-internal,
-      // non-always_inline functions.
+      // non-always_inline functions, unless they are available elsewhere.
       if (!FD->hasAttr<AlwaysInlineAttr>()) {
         if (!Linkage)
           Linkage = Writer->Context->GetGVALinkageForFunction(FD);
-        ModulesCodegen = *Linkage != GVA_Internal;
+        ModulesCodegen =
+            *Linkage != GVA_Internal && *Linkage != GVA_AvailableExternally;
       }
     }
   }
   Record->push_back(ModulesCodegen);
   if (ModulesCodegen)
     Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(FD));
   if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) {
     Record->push_back(CD->getNumCtorInitializers());
     if (CD->getNumCtorInitializers())
       AddCXXCtorInitializers(
           llvm::makeArrayRef(CD->init_begin(), CD->init_end()));
   }
   AddStmt(FD->getBody());
 }

clang/test/Modules/codegen-extern-template.h

+// header for codegen-extern-template.cpp
+#ifndef CODEGEN_EXTERN_TEMPLATE_H
+#define CODEGEN_EXTERN_TEMPLATE_H
+
+template <typename T>
+inline T foo() { return 10; }
+
+extern template int foo<int>();
+
+inline int bar() { return foo<int>(); }
+
+#endif

clang/test/Modules/codegen-extern-template.cpp

+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules -fmodules-codegen -emit-module -fmodule-name=foo %S/codegen-extern-template.modulemap -x c++ -o %t.pcm
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fmodules -fmodule-file=%t.pcm %s -emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include "codegen-extern-template.h"
+
+template int foo<int>();
+
+// CHECK: define weak_odr i32 @_Z3fooIiET_v

clang/test/Modules/codegen-extern-template.modulemap

+module foo { header "codegen-extern-template.h" }