[clang][ABI] New c++20 modules mangling scheme

[urnathan]

Summary

The existing module symbol mangling scheme turns out to be undemangleable. It is also desirable to switch to the strong-ownership model as the hoped-for C++17 compatibility turns out to be fragile, and we also now have a better way of controlling that.

The issue is captured on the ABI list at: https://github.com/itanium-cxx-abi/cxx-abi/issues/134

A document describing the issues and new mangling is at: https://drive.google.com/file/d/1quPYyByXqOpCyAPlxYt1JleA70TSFMrp/view?usp=sharing

This patch is the code-generation part. I have a demangler too, but that patch is based on some to-be-landed refactoring of the demangler.

The old mangling is unceremoniously dropped. No backwards compatibility, no deprectated old-mangling flag. It was always labelled experimental. (Old and new manglings cannot be confused.)

FWIW I'm fine with waiting until (a) demangler is ready and (b) clang-14 is out (just in case there's more changes needed)

[urnathan]

Document the grammar

[ChuanqiXu]

Is it a made decision that we prefer strong-ownership model now? Could you give some related discussion and rationale about this?

And I would like to see the test case about decls in global module fragment and I hope they wouldn't be mangled.

clang/lib/AST/ItaniumMangle.cpp

1007

I think it is worth to note that why we handle Namespace Decl specially here: https://eel.is/c++draft/basic.namespace.general#2

1681

Could we move the check into mangleModuleName? It seems simpler.

clang/test/CodeGenCXX/cxx20-module-nested-2.cppm

13

No newline at end of file

[urnathan]

The document I link to from the ABI bug tracker discusses strong ownership. The TL;DR; is that when Richard & I came up with the current mangling, we had hoped that a weak model would allow migration of non-module code to module-code without ABI break. That turns out to be so fragile we think it impossible in practice. Plus since that time, the semantics of 'extern "C++" {...}' got tweaked such that the declared entities have the semantics needed [I can't find the exact wording, but they're either in the global module or have equivalent linkage, permitting implementations to mangle them as such].

The strong ownership model means that (eg) linker diagnostics can now point at the module containing the [exported] symbol of interest.

[urnathan]

(a) namespace decls never have module-for-linkage
(b) added more global-module tests

thanks for the feedback!

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

1007

I moved the namespace check to getOwningModuleForLinkage

1681

I am not of that opinion

[ChuanqiXu]

In D118352#3278933, @urnathan wrote:

The document I link to from the ABI bug tracker discusses strong ownership. The TL;DR; is that when Richard & I came up with the current mangling, we had hoped that a weak model would allow migration of non-module code to module-code without ABI break. That turns out to be so fragile we think it impossible in practice. Plus since that time, the semantics of 'extern "C++" {...}' got tweaked such that the declared entities have the semantics needed [I can't find the exact wording, but they're either in the global module or have equivalent linkage, permitting implementations to mangle them as such].

I don't have a strong opinion on strong model or weak model. If it is fine if there is a consensus already. The overall patch looks fine but I plan to test it more locally before giving a LGTM. It would take sometime since I am going to leave the office for a week.

BTW, the wording about extern C++ should be http://eel.is/c++draft/module.unit#7.2.3. It should be good to add a test about it too.

clang/lib/AST/Decl.cpp

1544–1546

The general style might cite to the standard document like:

[basic.namespace.general]p2:
A namespace is never attached to a named module and never has a name with module linkage.

clang/lib/AST/ItaniumMangle.cpp

1681

OK, it is not bad too.

clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm

3

We lack a global module fragment declaration: module; The compiler lacks a check now... I wouldn't implement the check recently since there more important semantics to implement. But I am pretty sure that this one is invalid.

9

If this mimics std module, I think the name shouldn't be capitalized.

[urnathan]

Updated testcases, we don;t yet support extern "C++" semantics, so that case is xfailed.

[ChuanqiXu]

clang/lib/AST/ItaniumMangle.cpp

1038

My test shows that the line would cause a crash at extendSubstitutions. And we could avoid the crash if we rewrite this line to: ModuleSubstitutions.insert({Name, SeqID++});. Trying to offer a reduced example.

clang/test/CodeGenCXX/cxx20-module-extern-1.cppm

5

My bad. This should be completed in https://reviews.llvm.org/D115610. I would handle this after that one landed.

[ChuanqiXu]

clang/lib/AST/ItaniumMangle.cpp

1038

My bad. The rewriting should be ModuleSubstitutions.insert({Name, ModuleSubstitutions.size()});

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

1038

No, the module and entity substitution schemes share a single index 'SeqID'.

[rsmith]

There seems to be some amount of redundancy here: we seem to end up calling mangleModuleName before every (?) call to mangleUnqualifiedName. I see in the ABI changes that the module name is considered a part of the unqualified name in the grammar. Does it work out better to split it out in the implementation like you did here? (I guess if we put the module name in the unqualified name we'd want to pass in the effective DeclContext to avoid recomputing it, which would result in a similar blast radius for this patch either way, but I think it'd be nice to make the function names match the ABI grammar productions to the extent we reasonably can.)

clang/lib/AST/Decl.cpp

1543

Do we ever call this function on (say) a LinkageSpecDecl or an ExportDecl? (I think the right thing to do would be to skip over ExportDecls and say that a declaration has no owning module for linkage if there's any enclosing LinkageSpecDecl -- I think in C++ terminology, getOwningModule is determining purview and getOwningModuleForLinkage is determining attachment).

Probably this comment is best left for a different patch, though!

clang/lib/AST/ItaniumMangle.cpp

707–709

Fewer negatives would make this a bit easier for me to follow.

(I think the general rule we want is: variables are not mangled if the mangling would only contain their name -- eg, no namespace, ABI tags, module name, internal linkage specifier. Maybe phrasing it that way would be more useful to people who come to add things to this list later?)

1005

Temporal claims like "updated" tend to go stale fast. Also, I don't think referring to the Modules TS is useful any more. Maybe just drop this line?

5974–6031

Since we're dealing with std being reopened in a named module... do these need special-casing of the owning module for linkage too? Eg:

export module RenameString;
import <string>
export template<typename CT> using str = std::basic_string<char, CT>;

module Foo;
import RenameString;

namespace std {
  template<typename T> struct char_traits {};
}
// Should not use `Ss` mangling, because this is not the global module std::char_traits.
extern "C++" void f(str<std::char_traits<char>> s) {}

[urnathan]

Address comments. I've not moved mangleModule into mangleUnqualifiedName here, my concern in doing that is the collateral changes passing a context around. I seemed to fall into a rathole, but on the other hand it did seem to permit some other cleanups so I'll have another go.

This patch is based on D119333

[urnathan]

In D118352#3302835, @rsmith wrote:

There seems to be some amount of redundancy here: we seem to end up calling mangleModuleName before every (?) call to mangleUnqualifiedName. I see in the ABI changes that the module name is considered a part of the unqualified name in the grammar. Does it work out better to split it out in the implementation like you did here? (I guess if we put the module name in the unqualified name we'd want to pass in the effective DeclContext to avoid recomputing it, which would result in a similar blast radius for this patch either way, but I think it'd be nice to make the function names match the ABI grammar productions to the extent we reasonably can.)

Yes indeed what to do about DeclContext caused me to do it the way I did. I'll see what happens doing it in a grammar-matching manner.

clang/lib/AST/Decl.cpp

1543

perhaps renaming this to 'getModuleAttachment' might be less confusing? I didn't want to get into making language linkage DTRT with this patch.

clang/lib/AST/ItaniumMangle.cpp

707–709

... or are instantiations

5974–6031

good catch, added

[ChuanqiXu]

Some feedback from testing:

1. This one could address https://github.com/llvm/llvm-project/issues/52857 and https://github.com/llvm/llvm-project/issues/53232. Maybe we could add corresponding test.
2. Confirmed the crash at extendSubstitutions if we use ModuleSubstitutions.insert({Name, SeqID++}); instead of ModuleSubstitutions.insert({Name, ModuleSubstitutions.size()});. Failed to get a reduced example. I am going to make it again. It looks like it would check the SeqID from other TU. It looks like:

// A.cppm
export module A;
...
// B.cpp
import A;
... // crash at mangling name from A

3. This didn't handle implementation unit. It would make an undefined reference error for following pattern:

// A.cppm
export module A;
export {
void func();  // interface mangled with module name A.
}
// A-impl.cpp
module A;
void func() { // implementation wouldn't mangle with module name A.

}

I think the pattern above should be allowed by design. So we should support it.

[urnathan]

In D118352#3309891, @ChuanqiXu wrote:

Some feedback from testing:

1. This one could address https://github.com/llvm/llvm-project/issues/52857 and https://github.com/llvm/llvm-project/issues/53232. Maybe we could add corresponding test.

I will take a look, thanks for the pointer.

2. Confirmed the crash at extendSubstitutions if we use ModuleSubstitutions.insert({Name, SeqID++}); instead of ModuleSubstitutions.insert({Name, ModuleSubstitutions.size()});. Failed to get a reduced example. I am going to make it again. It looks like it would check the SeqID from other TU. It looks like:

Can you clarify what is crashing -- the compiler or something else? As I said, using ModuleSubstitutions.size() is wrong here, but it sounds like SeqID is not right either. I'm a little confused. We're mapping Name to an sequencing number that is common to the module names and the entity identities.

[ChuanqiXu]

In D118352#3310654, @urnathan wrote:

In D118352#3309891, @ChuanqiXu wrote:

2. Confirmed the crash at extendSubstitutions if we use ModuleSubstitutions.insert({Name, SeqID++}); instead of ModuleSubstitutions.insert({Name, ModuleSubstitutions.size()});. Failed to get a reduced example. I am going to make it again. It looks like it would check the SeqID from other TU. It looks like:

Can you clarify what is crashing -- the compiler or something else? As I said, using ModuleSubstitutions.size() is wrong here, but it sounds like SeqID is not right either. I'm a little confused. We're mapping Name to an sequencing number that is common to the module names and the entity identities.

The compiler crashes at: https://github.com/llvm/llvm-project/blob/fa2d31e9e64ab92b5f06e3be8110f9907709ad0a/clang/lib/AST/ItaniumMangle.cpp#L6123.

In D118352#3309891, @ChuanqiXu wrote:

3. This didn't handle implementation unit. It would make an undefined reference error for following pattern:

// A.cppm
export module A;
export {
void func();  // interface mangled with module name A.
}
// A-impl.cpp
module A;
void func() { // implementation wouldn't mangle with module name A.

}

I think the pattern above should be allowed by design. So we should support it.

Sorry to give the incorrect feedback. The patch itself handled implementation unit indeed. I made a mistake that I didn't realize that this patch didn't handle partitions and I implement partitions in downstream. So here is the mismatch.

clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp

15

Maybe it is good to add a CHECK line about the name of use here

[ChuanqiXu]

Although I failed to give a reduced example for the crash, I guess I found something.
The point here is that we shouldn't use SeqID and ModuleSubstitutions together. Without module, SeqID is used with Substitutions tightly. (SeqID would increase in addSubstitution which would increase Substitutions. And in extendSubstitutions, it would check SeqID and try to swap Substitutions. It doesn't matter about ModuleSubstitutions ). But now we use ModuleSubstitutions for modules instead of Substitutions. So SeqID is shared by both ModuleSubstitutions and Substitutions. And here is the question.
I made a simple experiment to create a new date member ModuleSeqID to replace the use of SeqID in mangleModuleNamePrefix and we could avoid the crash.

clang/lib/AST/ItaniumMangle.cpp

1017

In your paper, it is:

<module-subst> ::= W _ <digit> # substitutions 0-9
                             ::= W W <seq-id> _ # (n+10)th substitution

This looks inconsistency. Typo?

[urnathan]

thanks for pointing to that assert about SeqID. This mangling change is invalidating that assert. Will investigate.

In D118352#3313528, @ChuanqiXu wrote:

Although I failed to give a reduced example for the crash, I guess I found something.
The point here is that we shouldn't use SeqID and ModuleSubstitutions together. Without module, SeqID is used with Substitutions tightly. (SeqID would increase in addSubstitution which would increase Substitutions. And in extendSubstitutions, it would check SeqID and try to swap Substitutions. It doesn't matter about ModuleSubstitutions ). But now we use ModuleSubstitutions for modules instead of Substitutions. So SeqID is shared by both ModuleSubstitutions and Substitutions. And here is the question.
I made a simple experiment to create a new date member ModuleSeqID to replace the use of SeqID in mangleModuleNamePrefix and we could avoid the crash.

I believe you're referring to the old module mangling. Refer to 2.1.1:
A <module-name> is substitutable, using the existing <substitution> encoding:
<substitution> ::= S _

		::= S <seq-id> _

[urnathan]

In D118352#3313131, @ChuanqiXu wrote:

In D118352#3310654, @urnathan wrote:

In D118352#3309891, @ChuanqiXu wrote:

2. Confirmed the crash at extendSubstitutions if we use ModuleSubstitutions.insert({Name, SeqID++}); instead of ModuleSubstitutions.insert({Name, ModuleSubstitutions.size()});. Failed to get a reduced example. I am going to make it again. It looks like it would check the SeqID from other TU. It looks like:

Can you clarify what is crashing -- the compiler or something else? As I said, using ModuleSubstitutions.size() is wrong here, but it sounds like SeqID is not right either. I'm a little confused. We're mapping Name to an sequencing number that is common to the module names and the entity identities.

The compiler crashes at: https://github.com/llvm/llvm-project/blob/fa2d31e9e64ab92b5f06e3be8110f9907709ad0a/clang/lib/AST/ItaniumMangle.cpp#L6123.

Can you provide a stack traceback, or even better a testcase?

clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp

15

Why? That;s not relevant to what is being tested here.

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

1017

You are looking at the wrong document.

[urnathan]

• call mangleModule from mangleUnqualifiedName

• add Decomposition mangling tests -- and remove comment about decomp manglings never escaping

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

1038

I think the crash you're getting is solved by D119550, but as I mention in that patch, testcase would be helpful.

[ChuanqiXu]

clang/lib/AST/ItaniumMangle.cpp

1017

I looked at the 2.1.1 section at https://drive.google.com/file/d/1qQjqptzOFT_lfXH8L6-iD9nCRi34wjft/view. You mentioned it in the summary of the patch.

1038

Yeah, it is fixed after applying D119550. I would like to give a try to give a reduced test. But I've tried it last week and I am relatively busy. I couldn't promise I could give it finally.

clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp

15

I just feel like we lost a test about function name in implementation unit.

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

1017

2.1.1 Substitutions
A <module-name> is substitutable, using the existing <substitution> encoding:
<substitution> ::= S _
::= S <seq-id> _

1038

that is good to know. totally understand if it's too hard to extract a testcase.

[urnathan]

Added implementation unit test

[urnathan]

clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp

15

ah, it's mmore like there never was one to lose. I've added a codegen test.

[ChuanqiXu]

clang/lib/AST/ItaniumMangle.cpp

1017

Oh, sorry. I referred to 2.4.1:

<module-subst> ::= W _ <digit> # substitutions 0-9
::= W W <seq-id> _ # (n+10)th substitution

I mean the LHS of the comment. Its name should be <substitution> instead of <module-subst>, right?

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

1017

Section 2.4 discusses alternatives that were considered and rejected (it is very useful to have this kind of information so that future me knows we already thought about it).

by LHS of comment do you mean the BNF? This alternative is not a <substitution> element and would have a distinct name.

[urnathan]

Update after excise of IgnoreLinkageSpecDecls

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

1017

oh I see it now.

[ChuanqiXu]

clang/lib/AST/ItaniumMangle.cpp

1027–1035

For the partition part, I think we shouldn't mangle partition name. We should mangle about the name of the primary module (module-name) only. Since module linkage refers to that the entity could be referred by name for other units in the same module. So I think we could ignore partition part of the module simply.

[urnathan]

Add module initializer mangling entry point and fix partition mangling elswhere.

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

1027–1035

d'oh! right, there is one case where we do need to mangle the partition name (the initializer function), But mostly we should not.

[urnathan]

Looks like the partition stuff will need tweaking if/when D118586 lands.

[ChuanqiXu]

Looks like the partition stuff will need tweaking if/when D118586 lands.

I think it is worth to add D118586 as a related revision.

clang/lib/AST/ItaniumMangle.cpp

1027–1035

So why do we need the 'P' marker. I think we should treat names uniformly in partition unit and primary module interface unit.

1374

Unused variable?

6027–6029

It looks it is guarded at the line 6071.

6047–6048

What's the rationale to ban the case? I think it worths a comment . It looks like we don't want to mangle std name not in GMF. It looks like we would ban the case:

export module m;
namespace std {
template <class T>
class xxx { ... };
}

Do we need to ban this? Or if someone is trying to implement STL in named module (I know it wouldn't happen in recent future), the pattern looks not right.

[urnathan]

In D118352#3328101, @ChuanqiXu wrote:

Looks like the partition stuff will need tweaking if/when D118586 lands.

I think it is worth to add D118586 as a related revision.

I don't see the need. While both deal with modules, neither is dependent on the other and we're not going to forget to address the partition fixmes here.

clang/lib/AST/ItaniumMangle.cpp

1027–1035

The global initializer function. Partitions end up needing to emit them and they must be both externally reachable and distinct. Fortunately only the primary module interface unit needs to know about them.

1374

No, it is used later in code I have not changed.

6027–6029

No. that other check is checking the ownership of the template, this is checking the ownership of one of its template parameters. I.e. in

std::basic_string<char, std::type_traits<char>, std::allocator<char>>

the other check is for std::basic_string, this check is for std::type_traits and std::allocator.

6047–6048

We're not banning that, it would mangle as _ZStW1m3xxx. the special manglings for types in the std namespace are for when they are attached to the global module.

[urnathan]

Reminder to add to clang/docs/ReleaseNotes.rst, once this and the demangler patches are in.

[urnathan]

rebase on top of D116773

[ChuanqiXu]

Thanks for explanation. Now it looks good to me. Let's accept it formally after the series of partition landed and so that we could add test about partitions.

[urnathan]

In D118352#3331396, @ChuanqiXu wrote:

Thanks for explanation. Now it looks good to me. Let's accept it formally after the series of partition landed and so that we could add test about partitions.

thanks,

We're going to have to adjust a bit when partitions land, and the global initializer is a separate set of patches only tangentially impacting this. I don't think the project benefits from waiting.

[ChuanqiXu]

@urnathan FWIW, D114714 is landed~

[urnathan]

Updated to new partitions API

[ChuanqiXu]

If I don't misread the codes, it looks like mangleModuleInitializer is not called.

---

Now we could add test for partitions.

[urnathan]

In D118352#3358864, @ChuanqiXu wrote:

If I don't misread the codes, it looks like mangleModuleInitializer is not called.

Now we could add test for partitions.

Correct, it is not called as the global initializer pieces are not yet implemented. Let's not hold up this patch for that nor remove piece that will become necessary at that point.

[ChuanqiXu]

In D118352#3359626, @urnathan wrote:

In D118352#3358864, @ChuanqiXu wrote:

If I don't misread the codes, it looks like mangleModuleInitializer is not called.

Now we could add test for partitions.

Correct, it is not called as the global initializer pieces are not yet implemented. Let's not hold up this patch for that nor remove piece that will become necessary at that point.

It's odd to see unused functions. I just worry about that other people might delete these functions as cleaning up (maybe there wouldn't be).

---

Although the general declarations in partitions wouldn't be mangled specially, I think it would be better to add test case to show that.

[urnathan]

added partition tests

[urnathan]

In D118352#3362694, @ChuanqiXu wrote:

In D118352#3359626, @urnathan wrote:

Correct, it is not called as the global initializer pieces are not yet implemented. Let's not hold up this patch for that nor remove piece that will become necessary at that point.

It's odd to see unused functions. I just worry about that other people might delete these functions as cleaning up (maybe there wouldn't be).

That's a risk I'm ok with.

Although the general declarations in partitions wouldn't be mangled specially, I think it would be better to add test case to show that.

good idea, added.

[iains]

In D118352#3363909, @urnathan wrote:

In D118352#3362694, @ChuanqiXu wrote:

In D118352#3359626, @urnathan wrote:

Correct, it is not called as the global initializer pieces are not yet implemented. Let's not hold up this patch for that nor remove piece that will become necessary at that point.

It's odd to see unused functions. I just worry about that other people might delete these functions as cleaning up (maybe there wouldn't be).

That's a risk I'm ok with.

I agree - I think we already a fair amount of infrastructure in the compiler to handle module initialisers, ahead of a full implementation - so I do not see a reason for waiting longer to apply this because we cannot yet exercise it full.

Although the general declarations in partitions wouldn't be mangled specially, I think it would be better to add test case to show that.

good idea, added.

[ChuanqiXu]

LGTM, then. BTW, I see there is already no dependency with D119833 in code. But it shows dependency still in phab.

[thakis]

Looks like this breaks tests on macOS: http://45.33.8.238/macm1/29650/step_7.txt

Please take a look and revert for now if it takes a while to fix.

[thakis]

(also, please don't land unused code -- land those functions when you land their callers)

[phosek]

We're also seeing this issue on our Mac bots, is it possible to revert it?

[ChuanqiXu]

In D118352#3368919, @phosek wrote:

We're also seeing this issue on our Mac bots, is it possible to revert it?

I've reverted it. Since this is the new feature, I think it wouldn't so hurry to land it. BTW, I think it would be helpful to provide more information to fix it.

[steven_wu]

In D118352#3368922, @ChuanqiXu wrote:

In D118352#3368919, @phosek wrote:

We're also seeing this issue on our Mac bots, is it possible to revert it?

I've reverted it. Since this is the new feature, I think it wouldn't so hurry to land it. BTW, I think it would be helpful to provide more information to fix it.

It breaks the macOS bot for LLVM, you can click the test case for its output: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/27745/testReport/

You can see the error output. The problem might be that dso_local does not get codegen for macho targets.

[h-vetinari]

Something must have gone wrong... communication-wise... as @urnathan seems to have abandoned (resp. resigned from) all modules PRs. Hope any misunderstandings or grievances can be worked out!

[ChuanqiXu]

In D118352#3371425, @h-vetinari wrote:

Something must have gone wrong... communication-wise... as @urnathan seems to have abandoned (resp. resigned from) all modules PRs. Hope any misunderstandings or grievances can be worked out!

Oh, yeah... it is surprising...

clang/lib/AST/Decl.cpp

NamedDecl::ExplicitVisibilityKind EK = usesTypeVisibility(D)
                                              : NamedDecl::VisibilityForValue;
   LVComputationKind CK(EK);
   return getLVForDecl(D, D->getASTContext().getLangOpts().IgnoreXCOFFVisibility
                              ? CK.forLinkageOnly()
                              : CK);
 }
 
 Module *Decl::getOwningModuleForLinkage(bool IgnoreLinkage) const {
+  if (isa<NamespaceDecl>(this))
+    // Namespaces never have module linkage.  It is the entities within them
+    // that [may] do.
+    return nullptr;
+
   Module *M = getOwningModule();
   if (!M)
     return nullptr;
 
   switch (M->Kind) {
   case Module::ModuleMapModule:
     // Module map modules have no special linkage semantics.
     return nullptr;

clang/lib/AST/ItaniumMangle.cpp

void mangleUnresolvedName(NestedNameSpecifier *qualifier,
                             const TemplateArgumentLoc *TemplateArgs,
                             unsigned NumTemplateArgs,
                             unsigned KnownArity = UnknownArity);
 
   void mangleFunctionEncodingBareType(const FunctionDecl *FD);
 
   void mangleNameWithAbiTags(GlobalDecl GD,
                              const AbiTagList *AdditionalAbiTags);
-  void mangleModuleName(const Module *M);
+  void mangleModuleName(const NamedDecl *ND);
   void mangleModuleNamePrefix(StringRef Name);
   void mangleTemplateName(const TemplateDecl *TD,
                           const TemplateArgument *TemplateArgs,
                           unsigned NumTemplateArgs);
-  void mangleUnqualifiedName(GlobalDecl GD,
+  void mangleUnqualifiedName(GlobalDecl GD, const DeclContext *DC,
                              const AbiTagList *AdditionalAbiTags) {
-    mangleUnqualifiedName(GD, cast<NamedDecl>(GD.getDecl())->getDeclName(), UnknownArity,
-                          AdditionalAbiTags);
+    mangleUnqualifiedName(GD, cast<NamedDecl>(GD.getDecl())->getDeclName(), DC,
+                          UnknownArity, AdditionalAbiTags);
   }
   void mangleUnqualifiedName(GlobalDecl GD, DeclarationName Name,
-                             unsigned KnownArity,
+                             const DeclContext *DC, unsigned KnownArity,
                              const AbiTagList *AdditionalAbiTags);
-  void mangleUnscopedName(GlobalDecl GD,
+  void mangleUnscopedName(GlobalDecl GD, const DeclContext *DC,
                           const AbiTagList *AdditionalAbiTags);
-  void mangleUnscopedTemplateName(GlobalDecl GD,
+  void mangleUnscopedTemplateName(GlobalDecl GD, const DeclContext *DC,
                                   const AbiTagList *AdditionalAbiTags);
   void mangleSourceName(const IdentifierInfo *II);
   void mangleRegCallName(const IdentifierInfo *II);
   void mangleDeviceStubName(const IdentifierInfo *II);
   void mangleSourceNameWithAbiTags(
       const NamedDecl *ND, const AbiTagList *AdditionalAbiTags = nullptr);
   void mangleLocalName(GlobalDecl GD,
                        const AbiTagList *AdditionalAbiTags);
if (const auto *VD = dyn_cast<VarDecl>(D)) {
     // Decompositions are mangled.
     if (isa<DecompositionDecl>(VD))
       return true;
 
     // C variables are not mangled.
     if (VD->isExternC())
       return false;
 
-    // Variables at global scope with non-internal linkage are not mangled.
+    // Variables at global scope are not mangled unless they have internal
+    // linkage or are specializations or are attached to a named module.
     const DeclContext *DC = getEffectiveDeclContext(D);
     // Check for extern variable declared locally.
     if (DC->isFunctionOrMethod() && D->hasLinkage())
       while (!DC->isFileContext())
         DC = getEffectiveParentContext(DC);
     if (DC->isTranslationUnit() && D->getFormalLinkage() != InternalLinkage &&
         !CXXNameMangler::shouldHaveAbiTags(*this, VD) &&
-        !isa<VarTemplateSpecializationDecl>(VD))
+        !isa<VarTemplateSpecializationDecl>(VD) &&
+        !VD->getOwningModuleForLinkage())
       return false;
   }
 
   return true;
 }
 
 void CXXNameMangler::writeAbiTags(const NamedDecl *ND,
                                   const AbiTagList *AdditionalAbiTags) {
void CXXNameMangler::mangleNameWithAbiTags(GlobalDecl GD,
 
   assert(!isa<LinkageSpecDecl>(DC) && "context cannot be LinkageSpecDecl");
 
   if (isLocalContainerContext(DC)) {
     mangleLocalName(GD, AdditionalAbiTags);
     return;
   }
 
-  // Do not mangle the owning module for an external linkage declaration.
-  // This enables backwards-compatibility with non-modular code, and is
-  // a valid choice since conflicts are not permitted by C++ Modules TS
-  // [basic.def.odr]/6.2.
-  if (!ND->hasExternalFormalLinkage())
-    if (Module *M = ND->getOwningModuleForLinkage())
-      mangleModuleName(M);
-
   // Closures can require a nested-name mangling even if they're semantically
   // in the global namespace.
   if (const NamedDecl *PrefixND = getClosurePrefix(ND)) {
     mangleNestedNameWithClosurePrefix(GD, PrefixND, AdditionalAbiTags);
     return;
   }
 
   if (DC->isTranslationUnit() || isStdNamespace(DC)) {
     // Check if we have a template.
     const TemplateArgumentList *TemplateArgs = nullptr;
     if (GlobalDecl TD = isTemplate(GD, TemplateArgs)) {
-      mangleUnscopedTemplateName(TD, AdditionalAbiTags);
+      mangleUnscopedTemplateName(TD, DC, AdditionalAbiTags);
       mangleTemplateArgs(asTemplateName(TD), *TemplateArgs);
       return;
     }
 
-    mangleUnscopedName(GD, AdditionalAbiTags);
+    mangleUnscopedName(GD, DC, AdditionalAbiTags);
     return;
   }
 
   mangleNestedName(GD, DC, AdditionalAbiTags);
 }
 
-void CXXNameMangler::mangleModuleName(const Module *M) {
-  // Implement the C++ Modules TS name mangling proposal; see
-  //     https://gcc.gnu.org/wiki/cxx-modules?action=AttachFile
-  //
-  //   <module-name> ::= W <unscoped-name>+ E
-  //                 ::= W <module-subst> <unscoped-name>* E
-  Out << 'W';
+void CXXNameMangler::mangleModuleName(const NamedDecl *ND) {
+  if (ND->isExternallyVisible())
+    if (Module *M = ND->getOwningModuleForLinkage())
       mangleModuleNamePrefix(M->Name);
-  Out << 'E';
 }
 
+// <module-name> ::= <module-subname>
+//     ::= <module-name> <module-subname>
+//     ::= <substitution>
+// <module-subname> ::= W <source-name>
+//        ::= W P <source-name>
+// FIXME: Verify partitions are handled correctly once they are available.
 void CXXNameMangler::mangleModuleNamePrefix(StringRef Name) {
-  //  <module-subst> ::= _ <seq-id>          # 0 < seq-id < 10
-  //                 ::= W <seq-id - 10> _   # otherwise
+  //  <substitution> ::= S <seq-id> _
   auto It = ModuleSubstitutions.find(Name);
   if (It != ModuleSubstitutions.end()) {
-    if (It->second < 10)
-      Out << '_' << static_cast<char>('0' + It->second);
-    else
-      Out << 'W' << (It->second - 10) << '_';
+    Out << 'S';
+    mangleSeqID(It->second);
     return;
   }
 
   // FIXME: Preserve hierarchy in module names rather than flattening
   // them to strings; use Module*s as substitution keys.
-  auto Parts = Name.rsplit('.');
-  if (Parts.second.empty())
-    Parts.second = Parts.first;
-  else
-    mangleModuleNamePrefix(Parts.first);
+  bool IsPartition = false;
+  auto Sub = Name;
+  for (auto Split = Name.size(); Split--;)
+    if (Name[Split] == '.' || Name[Split] == ':') {
+      IsPartition = Name[Split] == ':';
+      mangleModuleNamePrefix(Name.take_front(Split));
+      Sub = Name.substr(Split + 1);
+      break;
+    }
 
-  Out << Parts.second.size() << Parts.second;
-  ModuleSubstitutions.insert({Name, ModuleSubstitutions.size()});
+  Out << 'W';
+  if (IsPartition)
+    Out << 'P';
+  Out << Sub.size() << Sub;
+  ModuleSubstitutions.insert({Name, SeqID++});
 }
 
 void CXXNameMangler::mangleTemplateName(const TemplateDecl *TD,
                                         const TemplateArgument *TemplateArgs,
                                         unsigned NumTemplateArgs) {
   const DeclContext *DC = getEffectiveDeclContext(TD);
 
   if (DC->isTranslationUnit() || isStdNamespace(DC)) {
-    mangleUnscopedTemplateName(TD, nullptr);
+    mangleUnscopedTemplateName(TD, DC, nullptr);
     mangleTemplateArgs(asTemplateName(TD), TemplateArgs, NumTemplateArgs);
   } else {
     mangleNestedName(TD, TemplateArgs, NumTemplateArgs);
   }
 }
 
-void CXXNameMangler::mangleUnscopedName(GlobalDecl GD,
+void CXXNameMangler::mangleUnscopedName(GlobalDecl GD, const DeclContext *DC,
                                         const AbiTagList *AdditionalAbiTags) {
-  const NamedDecl *ND = cast<NamedDecl>(GD.getDecl());
   //  <unscoped-name> ::= <unqualified-name>
   //                  ::= St <unqualified-name>   # ::std::
 
-  if (isStdNamespace(getEffectiveDeclContext(ND)))
+  assert(!isa<LinkageSpecDecl>(DC) && "unskipped LinkageSpecDecl");
+  if (isStdNamespace(DC))
     Out << "St";
 
-  mangleUnqualifiedName(GD, AdditionalAbiTags);
+  mangleUnqualifiedName(GD, DC, AdditionalAbiTags);
 }
 
 void CXXNameMangler::mangleUnscopedTemplateName(
-    GlobalDecl GD, const AbiTagList *AdditionalAbiTags) {
+    GlobalDecl GD, const DeclContext *DC, const AbiTagList *AdditionalAbiTags) {
   const TemplateDecl *ND = cast<TemplateDecl>(GD.getDecl());
   //     <unscoped-template-name> ::= <unscoped-name>
   //                              ::= <substitution>
   if (mangleSubstitution(ND))
     return;
 
   // <template-template-param> ::= <template-param>
   if (const auto *TTP = dyn_cast<TemplateTemplateParmDecl>(ND)) {
     assert(!AdditionalAbiTags &&
            "template template param cannot have abi tags");
     mangleTemplateParameter(TTP->getDepth(), TTP->getIndex());
   } else if (isa<BuiltinTemplateDecl>(ND) || isa<ConceptDecl>(ND)) {
-    mangleUnscopedName(GD, AdditionalAbiTags);
+    mangleUnscopedName(GD, DC, AdditionalAbiTags);
   } else {
-    mangleUnscopedName(GD.getWithDecl(ND->getTemplatedDecl()), AdditionalAbiTags);
+    mangleUnscopedName(GD.getWithDecl(ND->getTemplatedDecl()), DC,
+                       AdditionalAbiTags);
   }
 
   addSubstitution(ND);
 }
 
 void CXXNameMangler::mangleFloat(const llvm::APFloat &f) {
   // ABI:
   //   Floating-point literals are encoded using a fixed-length
void CXXNameMangler::mangleUnresolvedName(
   }
 
   // The <simple-id> and on <operator-name> productions end in an optional
   // <template-args>.
   if (TemplateArgs)
     mangleTemplateArgs(TemplateName(), TemplateArgs, NumTemplateArgs);
 }
 
-void CXXNameMangler::mangleUnqualifiedName(GlobalDecl GD,
-                                           DeclarationName Name,
-                                           unsigned KnownArity,
-                                           const AbiTagList *AdditionalAbiTags) {
+void CXXNameMangler::mangleUnqualifiedName(
+    GlobalDecl GD, DeclarationName Name, const DeclContext *DC,
+    unsigned KnownArity, const AbiTagList *AdditionalAbiTags) {
   const NamedDecl *ND = cast_or_null<NamedDecl>(GD.getDecl());
-  unsigned Arity = KnownArity;
-  //  <unqualified-name> ::= <operator-name>
+  //  <unqualified-name> ::= [<module-name>] <operator-name>
   //                     ::= <ctor-dtor-name>
-  //                     ::= <source-name>
+  //                     ::= [<module-name>] <source-name>
+  //                     ::= [<module-name>] DC <source-name>* E
+
+  if (ND && DC && DC->isFileContext())
+    mangleModuleName(ND);
+
+  unsigned Arity = KnownArity;
   switch (Name.getNameKind()) {
   case DeclarationName::Identifier: {
     const IdentifierInfo *II = Name.getAsIdentifierInfo();
 
     // We mangle decomposition declarations as the names of their bindings.
     if (auto *DD = dyn_cast<DecompositionDecl>(ND)) {
       // FIXME: Non-standard mangling for decomposition declarations:
       //
       //  <unqualified-name> ::= DC <source-name>* E
       //
-      // These can never be referenced across translation units, so we do
-      // not need a cross-vendor mangling for anything other than demanglers.
       // Proposed on cxx-abi-dev on 2016-08-12
       Out << "DC";
       for (auto *BD : DD->bindings())
         mangleSourceName(BD->getDeclName().getAsIdentifierInfo());
       Out << 'E';
       writeAbiTags(ND, AdditionalAbiTags);
       break;
     }
void CXXNameMangler::mangleNestedName(GlobalDecl GD,
 
   // Check if we have a template.
   const TemplateArgumentList *TemplateArgs = nullptr;
   if (GlobalDecl TD = isTemplate(GD, TemplateArgs)) {
     mangleTemplatePrefix(TD, NoFunction);
     mangleTemplateArgs(asTemplateName(TD), *TemplateArgs);
   } else {
     manglePrefix(DC, NoFunction);
-    mangleUnqualifiedName(GD, AdditionalAbiTags);
+    mangleUnqualifiedName(GD, DC, AdditionalAbiTags);
   }
 
   Out << 'E';
 }
 void CXXNameMangler::mangleNestedName(const TemplateDecl *TD,
                                       const TemplateArgument *TemplateArgs,
                                       unsigned NumTemplateArgs) {
   // <nested-name> ::= N [<CV-qualifiers>] <template-prefix> <template-args> E
void CXXNameMangler::mangleNestedNameWithClosurePrefix(
   // DeclContext, so needs special handling. In this case we're mangling a
   // limited form of <nested-name>:
   //
   // <nested-name> ::= N <closure-prefix> <closure-type-name> E
 
   Out << 'N';
 
   mangleClosurePrefix(PrefixND);
-  mangleUnqualifiedName(GD, AdditionalAbiTags);
+  mangleUnqualifiedName(GD, nullptr, AdditionalAbiTags);
 
   Out << 'E';
 }
 
 static GlobalDecl getParentOfLocalEntity(const DeclContext *DC) {
   GlobalDecl GD;
   // The Itanium spec says:
   // For entities in constructors and destructors, the mangling of the
if (CXXRD && CXXRD->isLambda()) {
           Out << '_';
         }
       }
     }
 
     // Mangle the name relative to the closest enclosing function.
     // equality ok because RD derived from ND above
     if (D == RD)  {
-      mangleUnqualifiedName(RD, AdditionalAbiTags);
+      mangleUnqualifiedName(RD, DC, AdditionalAbiTags);
     } else if (const BlockDecl *BD = dyn_cast<BlockDecl>(D)) {
       if (const NamedDecl *PrefixND = getClosurePrefix(BD))
         mangleClosurePrefix(PrefixND, true /*NoFunction*/);
       else
         manglePrefix(getEffectiveDeclContext(BD), true /*NoFunction*/);
       assert(!AdditionalAbiTags && "Block cannot have additional abi tags");
       mangleUnqualifiedBlock(BD);
     } else {
if (const ParmVarDecl *Parm
           mangleNumber(Num - 2);
         Out << '_';
       }
     }
 
     assert(!AdditionalAbiTags && "Block cannot have additional abi tags");
     mangleUnqualifiedBlock(BD);
   } else {
-    mangleUnqualifiedName(GD, AdditionalAbiTags);
+    mangleUnqualifiedName(GD, DC, AdditionalAbiTags);
   }
 
   if (const NamedDecl *ND = dyn_cast<NamedDecl>(RD ? RD : D)) {
     unsigned disc;
     if (Context.getNextDiscriminator(ND, disc)) {
       if (disc < 10)
         Out << '_' << disc;
       else
void CXXNameMangler::manglePrefix(const DeclContext *DC, bool NoFunction) {
 
   // Check if we have a template-prefix or a closure-prefix.
   const TemplateArgumentList *TemplateArgs = nullptr;
   if (GlobalDecl TD = isTemplate(ND, TemplateArgs)) {
     mangleTemplatePrefix(TD);
     mangleTemplateArgs(asTemplateName(TD), *TemplateArgs);
   } else if (const NamedDecl *PrefixND = getClosurePrefix(ND)) {
     mangleClosurePrefix(PrefixND, NoFunction);
-    mangleUnqualifiedName(ND, nullptr);
+    mangleUnqualifiedName(ND, nullptr, nullptr);
   } else {
-    manglePrefix(getEffectiveDeclContext(ND), NoFunction);
-    mangleUnqualifiedName(ND, nullptr);
+    const DeclContext *DC = getEffectiveDeclContext(ND);
+    manglePrefix(DC, NoFunction);
+    mangleUnqualifiedName(ND, DC, nullptr);
   }
 
   addSubstitution(ND);
 }
 
 void CXXNameMangler::mangleTemplatePrefix(TemplateName Template) {
   // <template-prefix> ::= <prefix> <template unqualified-name>
   //                   ::= <template-param>
void CXXNameMangler::mangleTemplatePrefix(GlobalDecl GD,
 
   if (mangleSubstitution(ND))
     return;
 
   // <template-template-param> ::= <template-param>
   if (const auto *TTP = dyn_cast<TemplateTemplateParmDecl>(ND)) {
     mangleTemplateParameter(TTP->getDepth(), TTP->getIndex());
   } else {
-    manglePrefix(getEffectiveDeclContext(ND), NoFunction);
+    const DeclContext *DC = getEffectiveDeclContext(ND);
+    manglePrefix(DC, NoFunction);
     if (isa<BuiltinTemplateDecl>(ND) || isa<ConceptDecl>(ND))
-      mangleUnqualifiedName(GD, nullptr);
+      mangleUnqualifiedName(GD, DC, nullptr);
     else
-      mangleUnqualifiedName(GD.getWithDecl(ND->getTemplatedDecl()), nullptr);
+      mangleUnqualifiedName(GD.getWithDecl(ND->getTemplatedDecl()), DC,
+                            nullptr);
   }
 
   addSubstitution(ND);
 }
 
 const NamedDecl *CXXNameMangler::getClosurePrefix(const Decl *ND) {
   if (getASTContext().getLangOpts().getClangABICompat() <=
       LangOptions::ClangABI::Ver12)
void CXXNameMangler::mangleClosurePrefix(const NamedDecl *ND, bool NoFunction) {
   if (mangleSubstitution(ND))
     return;
 
   const TemplateArgumentList *TemplateArgs = nullptr;
   if (GlobalDecl TD = isTemplate(ND, TemplateArgs)) {
     mangleTemplatePrefix(TD, NoFunction);
     mangleTemplateArgs(asTemplateName(TD), *TemplateArgs);
   } else {
-    manglePrefix(getEffectiveDeclContext(ND), NoFunction);
-    mangleUnqualifiedName(ND, nullptr);
+    const auto *DC = getEffectiveDeclContext(ND);
+    manglePrefix(DC, NoFunction);
+    mangleUnqualifiedName(ND, DC, nullptr);
   }
 
   Out << 'M';
 
   addSubstitution(ND);
 }
 
 /// Mangles a template name under the production <type>.  Required for
bool CXXNameMangler::mangleSubstitution(uintptr_t Ptr) {
   return true;
 }
 
 /// Returns whether S is a template specialization of std::Name with a single
 /// argument of type A.
 static bool isSpecializedAs(QualType S, llvm::StringRef Name, QualType A) {
   if (S.isNull())
     return false;
 
   const RecordType *RT = S->getAs<RecordType>();
   if (!RT)
     return false;
 
   const ClassTemplateSpecializationDecl *SD =
     dyn_cast<ClassTemplateSpecializationDecl>(RT->getDecl());
   if (!SD || !SD->getIdentifier()->isStr(Name))
     return false;
 
   if (!isStdNamespace(getEffectiveDeclContext(SD)))
     return false;
 
   const TemplateArgumentList &TemplateArgs = SD->getTemplateArgs();
   if (TemplateArgs.size() != 1)
     return false;
 
   if (TemplateArgs[0].getAsType() != A)
     return false;
 
+  if (SD->getSpecializedTemplate()->getOwningModuleForLinkage())
+    return false;
+
   return true;
 }
 
 /// Returns whether SD is a template specialization std::Name<char,
 /// std::char_traits<char> [, std::allocator<char>]>
 /// HasAllocator controls whether the 3rd template argument is needed.
 static bool isStdCharSpecialization(const ClassTemplateSpecializationDecl *SD,
                                     llvm::StringRef Name, bool HasAllocator) {
   if (!SD->getIdentifier()->isStr(Name))
     return false;
 
   const TemplateArgumentList &TemplateArgs = SD->getTemplateArgs();
   if (TemplateArgs.size() != (HasAllocator ? 3 : 2))
     return false;
 
   QualType A = TemplateArgs[0].getAsType();
   if (A.isNull())
     return false;
   // Plain 'char' is named Char_S or Char_U depending on the target ABI.
   if (!A->isSpecificBuiltinType(BuiltinType::Char_S) &&
       !A->isSpecificBuiltinType(BuiltinType::Char_U))
     return false;
 
   if (!isSpecializedAs(TemplateArgs[1].getAsType(), "char_traits", A))
     return false;
 
   if (HasAllocator &&
       !isSpecializedAs(TemplateArgs[2].getAsType(), "allocator", A))
     return false;
 
+  if (SD->getSpecializedTemplate()->getOwningModuleForLinkage())
+    return false;
+
   return true;
 }
 
 bool CXXNameMangler::mangleStandardSubstitution(const NamedDecl *ND) {
   // <substitution> ::= St # ::std::
   if (const NamespaceDecl *NS = dyn_cast<NamespaceDecl>(ND)) {
     if (isStd(NS)) {
       Out << "St";
       return true;
     }
     return false;
   }
 
   if (const ClassTemplateDecl *TD = dyn_cast<ClassTemplateDecl>(ND)) {
     if (!isStdNamespace(getEffectiveDeclContext(TD)))
       return false;
 
+    if (TD->getOwningModuleForLinkage())
+      return false;
+
     // <substitution> ::= Sa # ::std::allocator
     if (TD->getIdentifier()->isStr("allocator")) {
       Out << "Sa";
       return true;
     }
 
     // <<substitution> ::= Sb # ::std::basic_string
     if (TD->getIdentifier()->isStr("basic_string")) {
       Out << "Sb";
       return true;
     }
     return false;
   }
 
   if (const ClassTemplateSpecializationDecl *SD =
         dyn_cast<ClassTemplateSpecializationDecl>(ND)) {
     if (!isStdNamespace(getEffectiveDeclContext(SD)))
       return false;
 
+    if (SD->getSpecializedTemplate()->getOwningModuleForLinkage())
+      return false;
+
     //    <substitution> ::= Ss # ::std::basic_string<char,
     //                            ::std::char_traits<char>,
     //                            ::std::allocator<char> >
     if (isStdCharSpecialization(SD, "basic_string", /*HasAllocator=*/true)) {
       Out << "Ss";
       return true;
     }
 

clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp

 // 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
 
-// CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
-// CHECK-DAG: @const_var_exported = available_externally {{(dso_local )?}}constant i32 3,
+// CHECK-DAG: @_ZW6Module19extern_var_exported = external {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module19inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module18const_var_exported = available_externally {{(dso_local )?}}constant i32 3,
 //
-// CHECK-DAG: @_ZW6ModuleE25extern_var_module_linkage = external {{(dso_local )?}}global
-// CHECK-DAG: @_ZW6ModuleE25inline_var_module_linkage = linkonce_odr {{(dso_local )?}}global
-// CHECK-DAG: @_ZW6ModuleE25static_var_module_linkage = available_externally {{(dso_local )?}}global i32 0,
-// CHECK-DAG: @_ZW6ModuleE24const_var_module_linkage = available_externally {{(dso_local )?}}constant i32 3,
+// CHECK-DAG: @_ZW6Module25extern_var_module_linkage = external {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module25inline_var_module_linkage = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module25static_var_module_linkage = available_externally {{(dso_local )?}}global i32 0,
+// CHECK-DAG: @_ZW6Module24const_var_module_linkage = available_externally {{(dso_local )?}}constant i32 3,
 
 module Module;
 
 void use() {
-  // CHECK: define linkonce_odr {{.*}}@_Z20used_inline_exportedv
+  // CHECK: define linkonce_odr {{.*}}@_ZW6Module20used_inline_exportedv
   used_inline_exported();
-  // CHECK: declare {{.*}}@_Z18noninline_exportedv
+  // CHECK: declare {{.*}}@_ZW6Module18noninline_exportedv
   noninline_exported();
 
   (void)&extern_var_exported;
   (void)&inline_var_exported;
   (void)&const_var_exported;
 
   // FIXME: This symbol should not be visible here.
-  // CHECK: declare {{.*}}@_ZW6ModuleE26used_static_module_linkagev
+  // CHECK: declare {{.*}}@_ZW6Module26used_static_module_linkagev
   used_static_module_linkage();
 
-  // CHECK: define linkonce_odr {{.*}}@_ZW6ModuleE26used_inline_module_linkagev
+  // CHECK: define linkonce_odr {{.*}}@_ZW6Module26used_inline_module_linkagev
   used_inline_module_linkage();
 
-  // CHECK: declare {{.*}}@_ZW6ModuleE24noninline_module_linkagev
+  // CHECK: declare {{.*}}@_ZW6Module24noninline_module_linkagev
   noninline_module_linkage();
 
   (void)&extern_var_module_linkage;
   (void)&inline_var_module_linkage;
   (void)&static_var_module_linkage; // FIXME: Should not be visible here.
   (void)&const_var_module_linkage;
 }

clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm

 // RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s --implicit-check-not unused_inline --implicit-check-not unused_stastic_global_module
 
 // CHECK-DAG: @extern_var_global_module = external {{(dso_local )?}}global
 // CHECK-DAG: @inline_var_global_module = linkonce_odr {{(dso_local )?}}global
 // CHECK-DAG: @_ZL24static_var_global_module = internal global
 // CHECK-DAG: @_ZL23const_var_global_module = internal constant
 //
 // For ABI compatibility, these symbols do not include the module name.
-// CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module19extern_var_exported = external {{(dso_local )?}}global
 // FIXME: Should this be 'weak_odr global'? Presumably it must be, since we
 // can discard this global and its initializer (if any), and other TUs are not
 // permitted to run the initializer for this variable.
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
-// CHECK-DAG: @const_var_exported = {{(dso_local )?}}constant
+// CHECK-DAG: @_ZW6Module19inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module18const_var_exported = {{(dso_local )?}}constant
 //
-// CHECK-DAG: @_ZW6ModuleE25extern_var_module_linkage = external {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module25extern_var_module_linkage = external {{(dso_local )?}}global
 // FIXME: Should this be 'weak_odr global'? Presumably it must be, since we
 // can discard this global and its initializer (if any), and other TUs are not
 // permitted to run the initializer for this variable.
-// CHECK-DAG: @_ZW6ModuleE25inline_var_module_linkage = linkonce_odr {{(dso_local )?}}global
-// CHECK-DAG: @_ZW6ModuleE25static_var_module_linkage = {{(dso_local )?}}global
-// CHECK-DAG: @_ZW6ModuleE24const_var_module_linkage = {{(dso_local )?}}constant
+// CHECK-DAG: @_ZW6Module25inline_var_module_linkage = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module25static_var_module_linkage = {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module24const_var_module_linkage = {{(dso_local )?}}constant
 //
-// CHECK-DAG: @_ZW6ModuleE25unused_var_module_linkage = {{(dso_local )?}}global i32 4
-// CHECK-DAG: @_ZW6ModuleE32unused_static_var_module_linkage = {{(dso_local )?}}global i32 5
-// CHECK-DAG: @_ZW6ModuleE31unused_const_var_module_linkage = {{(dso_local )?}}constant i32 7
+// CHECK-DAG: @_ZW6Module25unused_var_module_linkage = {{(dso_local )?}}global i32 4
+// CHECK-DAG: @_ZW6Module32unused_static_var_module_linkage = {{(dso_local )?}}global i32 5
+// CHECK-DAG: @_ZW6Module31unused_const_var_module_linkage = {{(dso_local )?}}constant i32 7
 
 static void unused_static_global_module() {}
 static void used_static_global_module() {}
 
 inline void unused_inline_global_module() {}
 inline void used_inline_global_module() {}
 
 extern int extern_var_global_module;

 export {
   inline void unused_inline_exported() {}
   inline void used_inline_exported() {}
 
   extern int extern_var_exported;
   inline int inline_var_exported;
   const int const_var_exported = 3;
 
-  // CHECK: define {{(dso_local )?}}void {{.*}}@_Z18noninline_exportedv
+  // CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6Module18noninline_exportedv
   void noninline_exported() {
     (void)&extern_var_exported;
     (void)&inline_var_exported;
     (void)&const_var_exported;
   }
 }
 
 // FIXME: Ideally we wouldn't emit this as its name is not visible outside this
 // TU, but this module interface might contain a template that can use this
 // function so we conservatively emit it for now.
-// CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6ModuleE28unused_static_module_linkagev
+// CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6Module28unused_static_module_linkagev
 static void unused_static_module_linkage() {}
-// CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6ModuleE26used_static_module_linkagev
+// CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6Module26used_static_module_linkagev
 static void used_static_module_linkage() {}
 
 inline void unused_inline_module_linkage() {}
 inline void used_inline_module_linkage() {}
 
 extern int extern_var_module_linkage;
 inline int inline_var_module_linkage;
 static int static_var_module_linkage;
 const int const_var_module_linkage = 3;
 
-// CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6ModuleE24noninline_module_linkagev
+// CHECK: define {{(dso_local )?}}void {{.*}}@_ZW6Module24noninline_module_linkagev
 void noninline_module_linkage() {
   used_static_module_linkage();
-  // CHECK: define linkonce_odr {{.*}}@_ZW6ModuleE26used_inline_module_linkagev
+  // CHECK: define linkonce_odr {{.*}}@_ZW6Module26used_inline_module_linkagev
   used_inline_module_linkage();
 
   (void)&extern_var_module_linkage;
   (void)&inline_var_module_linkage;
   (void)&static_var_module_linkage;
   (void)&const_var_module_linkage;
 }
 
 int unused_var_module_linkage = 4;
 static int unused_static_var_module_linkage = 5;
 inline int unused_inline_var_module_linkage = 6;
 const int unused_const_var_module_linkage = 7;
 
 struct a {
   struct b {};
   struct c {};
 };
-// CHECK: define {{(dso_local )?}}void @_ZW6ModuleE1fW_0EN1a1bEW_0ENS_1cE(
+// CHECK: define {{(dso_local )?}}void @_ZW6Module1fNS_1a1bENS0_1cE(
 void f(a::b, a::c) {}

clang/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp

 // 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
 
-// CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
-// CHECK-DAG: @const_var_exported = available_externally {{(dso_local )?}}constant i32 3
+// CHECK-DAG: @_ZW6Module19extern_var_exported = external {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module19inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @_ZW6Module18const_var_exported = available_externally {{(dso_local )?}}constant i32 3
 
 import Module;
 
 void use() {
-  // CHECK: define linkonce_odr {{.*}}@_Z20used_inline_exportedv
+  // CHECK: define linkonce_odr {{.*}}@_ZW6Module20used_inline_exportedv
   used_inline_exported();
-  // CHECK: declare {{.*}}@_Z18noninline_exportedv
+  // CHECK: declare {{.*}}@_ZW6Module18noninline_exportedv
   noninline_exported();
 
   (void)&extern_var_exported;
   (void)&inline_var_exported;
   (void)&const_var_exported;
 
   // Module-linkage declarations are not visible here.
 }

clang/test/CXX/modules-ts/basic/basic.link/p3.cppm

 // RUN: %clang_cc1 -fmodules-ts -triple x86_64-linux %s -emit-module-interface -o %t
 // RUN: %clang_cc1 -fmodules-ts -triple x86_64-linux -x pcm %t -emit-llvm -o - | FileCheck %s
 
 export module M;
 
-// CHECK-DAG: @_ZW1ME1a ={{.*}} constant i32 1
+// CHECK-DAG: @_ZW1M1a ={{.*}} constant i32 1
 const int a = 1;
-// CHECK-DAG: @b ={{.*}} constant i32 2
+// CHECK-DAG: @_ZW1M1b ={{.*}} constant i32 2
 export const int b = 2;
 
 export int f() { return a + b; }

clang/test/CXX/modules-ts/codegen-basics.cppm

 // RUN: %clang_cc1 -fmodules-ts -std=c++1z -triple=x86_64-linux-gnu -fmodules-codegen -emit-module-interface %s -o %t.pcm
 // RUN: %clang_cc1 -fmodules-ts -std=c++1z -triple=x86_64-linux-gnu %t.pcm -emit-llvm -o - | FileCheck %s
 
 export module FooBar;
 
 export {
-  // CHECK-DAG: define{{.*}} i32 @_Z1fv(
+  // CHECK-DAG: define{{.*}} i32 @_ZW6FooBar1fv(
   int f() { return 0; }
 }
 
-// CHECK-DAG: define weak_odr void @_ZW6FooBarE2f2v(
+// CHECK-DAG: define weak_odr void @_ZW6FooBar2f2v(
 inline void f2() {}
 
-// CHECK-DAG: define{{.*}} void @_ZW6FooBarE2f3v(
+// CHECK-DAG: define{{.*}} void @_ZW6FooBar2f3v(
 static void f3() {}
 export void use_f3() { f3(); }
 
 // FIXME: Emit global variables and their initializers with this TU.
 // Emit an initialization function that other TUs can call, with guard variable?
 
 // FIXME: const-qualified variables don't have implicit internal linkage when owned by a module.

clang/test/CodeGenCXX/Inputs/cxx20-module-impl-1a.cpp

+export module Foo;
+export void Exported();
+void Module();

clang/test/CodeGenCXX/Inputs/cxx20-module-std-subst-2a.cpp

+module;
+# 5 __FILE__ 1
+namespace std {
+template <typename A> struct allocator {};
+template <typename C, typename T, typename A>
+class basic_string;
+} // namespace std
+# 12 "" 2
+export module RenameString;
+export template <typename C, typename T>
+using str = std::basic_string<C, T, std::allocator<C>>;

clang/test/CodeGenCXX/cxx20-module-decomp-1.cpp

+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s
+
+export module Foo;
+
+// CHECK-DAG: @_ZW3Foo3ary =
+int ary[2];
+
+// CHECK-DAG: @_ZW3FooDC1a1bE =
+export auto &[a, b] = ary;
+
+namespace N {
+// CHECK-DAG: @_ZN1NW3FooDC1a1bEE =
+export auto &[a, b] = ary;
+// CHECK-DAG: @_ZN1NW3FooDC1c1dEE =
+auto &[c, d] = ary;
+// FIXME: We mangle the module name here, as we give this ModuleInternalLinkage
+// That's no longer needed.
+// CHECK DAG: @_ZN1MDC1e1fEE =
+static auto &__attribute__((used))[e, f] = ary;
+} // namespace N

clang/test/CodeGenCXX/cxx20-module-extern-1.cppm

+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s
+
+// module-purview extern "C++" semantics not implemented
+// XFAIL: *
+
+export module FOO;
+extern "C++" export class A;
+export class B;
+
+// CHECK-DAG: void @_ZW3FOO3FooP1APNS_1B(
+export void Foo (A *, B*) {
+}
+
+extern "C++" {
+// CHECK-DAG: void @_Z3BarP1APW3FOO1B(
+export void Bar (A *, B*) {
+}
+}

clang/test/CodeGenCXX/cxx20-module-impl-1a.cpp

+// RUN: %clang_cc1 -std=c++20 %S/Inputs/cxx20-module-impl-1a.cpp -triple %itanium_abi_triple -emit-module-interface -o %t
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -fmodule-file=%t -emit-llvm -o - | FileCheck %s
+
+module Foo;
+
+// CHECK-DAG: @_ZW3Foo8Exportedv(
+void Exported() {
+}
+
+// CHECK-DAG: @_ZW3Foo6Modulev(
+void Module() {
+}
+
+// CHECK-DAG: @_ZW3Foo7Module2v(
+void Module2() {
+}
+

clang/test/CodeGenCXX/cxx20-module-nested-1.cppm

+// RUN: %clang_cc1 -std=c++20 -fmodules-ts %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s
+module;
+# 4 __FILE__ 1
+namespace Outer::Inner {
+class X;
+// CHECK-DAG: define dso_local void @_ZN5Outer5Inner3BarERNS0_1XE(
+void Bar (X &) {}
+} // namespace Outer::Inner
+# 10 "" 2
+export module FOO;  
+namespace Outer {
+class Y;
+namespace Inner {
+// CHECK-DAG: define dso_local void @_ZN5Outer5InnerW3FOO2FnERNS0_1XERNS_S1_1YE(
+void Fn (X &, Y &){}  // #1
+} // namespace Inner
+} // namespace Outer

clang/test/CodeGenCXX/cxx20-module-nested-2.cppm

+// RUN: %clang_cc1 -fmodules-ts %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s
+export module FOO;
+namespace Outer {
+class Y;
+class Inner {
+  class X;
+  void Fn (X &, Y &); // #2
+};
+// CHECK-DAG: define dso_local void @_ZN5OuterW3FOO5Inner2FnERNS1_1XERNS_S0_1YE(
+void Inner::Fn (X &, Y &) {}
+}
+

clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm

+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s
+
+module;
+# 5 __FILE__ 1
+class Pooh;
+class Piglet;
+# 8 "" 2
+
+export module std; // might happen, you can't say it won't!
+
+namespace std {
+export template<typename T> class allocator {
+// just for testing, not real!
+void M (T *);
+template <typename U> U *N (T *);
+};
+
+template<typename T> void allocator<T>::M (T *) {}
+template<typename T> template<typename U> U *allocator<T>::N (T *) {
+return nullptr;
+}
+
+// CHECK-DAG: void @_ZNStW3std9allocatorIiE1MEPi(
+template void allocator<int>::M (int *);
+// CHECK-DAG: @_ZNStW3std9allocatorIiE1NIfEEPT_Pi(
+template float *allocator<int>::N<float> (int *);
+}
+
+// CHECK-DAG: @_ZNStW3std9allocatorI4PoohE1MEPS1_(
+template void std::allocator<Pooh>::M (Pooh *);
+// CHECK-DAG: @_ZNStW3std9allocatorI4PoohE1NI6PigletEEPT_PS1_(
+template Piglet *std::allocator<Pooh>::N<Piglet> (Pooh *);

clang/test/CodeGenCXX/cxx20-module-std-subst-2b.cpp

+// RUN: %clang_cc1 -std=c++20 %S/Inputs/cxx20-module-std-subst-2a.cpp -triple %itanium_abi_triple -emit-module-interface -o %t
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -fmodule-file=%t -emit-llvm -o - | FileCheck %s
+
+export module Foo;
+import RenameString;
+
+namespace std {
+template <typename T> struct char_traits {};
+} // namespace std
+
+// use Sb mangling, not Ss as this is not global-module std::char_traits
+// std::char_traits.
+// CHECK-DAG: void @_ZW3Foo1fRSbIcStS_11char_traitsIcESaIcEE(
+void f(str<char, std::char_traits<char>> &s) {
+}

clang/test/CodeGenCXX/cxx20-module-std-subst-2c.cpp

+// RUN: %clang_cc1 -std=c++20 %S/Inputs/cxx20-module-std-subst-2a.cpp -triple %itanium_abi_triple -emit-module-interface -o %t
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -fmodule-file=%t -emit-llvm -o - | FileCheck %s
+module;
+# 5 __FILE__ 1
+namespace std {
+template <typename A> struct char_traits {};
+} // namespace std
+# 9 "" 2
+export module Bar;
+import RenameString;
+
+// Use Sb as this is global-module std::char_traits
+// CHECK-DAG: void @_ZW3Bar1gRSs(
+void g(str<char, std::char_traits<char>> &s) {
+}

clang/test/CodeGenCXX/cxx20-module-sub-1a.cppm

+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s
+
+export module FOO.BAR;
+export class A;
+namespace Bob {
+export class B;
+
+// CHECK-DAG: void @_ZN3BobW3FOOW3BAR3BarEPS1_1APNS_S1_1BE(
+export void Bar (A *, B*) {
+}
+}
+
+// CHECK-DAG: void @_ZW3FOOW3BAR3FooPS0_1APN3BobS0_1BE(
+export void Foo (A *, Bob::B*) {
+}

clang/test/CodeGenCXX/cxx20-module-sub-1b.cppm

+// RUN: %clang_cc1 -std=c++20 %S/cxx20-module-sub-1a.cppm -triple %itanium_abi_triple -emit-module-interface -o %t
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -fmodule-file=%t -emit-llvm -o - | FileCheck %s
+
+export module FOO.BAZ;
+import FOO.BAR;
+
+namespace Bob {
+
+// CHECK-DAG: void @_ZN3BobW3FOOW3BAZ3FooEPS0_W3BAR1APNS_S2_1BE(
+void Foo (A *, B*) {
+}
+}
+
+// CHECK-DAG: void @_ZW3FOOW3BAZ3BarPS_W3BAR1APN3BobS1_1BE(
+void Bar (A *, Bob::B*) {
+}

clang/test/CodeGenCXX/cxx20-module-tmpl-1.cppm

+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %s
+
+export module FOO;
+
+class One;
+class Two;
+
+export template<typename T> struct TPL
+{
+void M (T *);
+template<typename U> void N (T *, U*);
+};
+
+template<typename T>
+void TPL<T>::M (T *) {}
+
+template<typename T> template<typename U> void TPL<T>::N (T *, U*) {}
+
+// CHECK-DAG: void @_ZNW3FOO3TPLIS_3OneE1MEPS1_(
+template void TPL<One>::M (One *);
+// CHECK-DAG: void @_ZNW3FOO3TPLIS_3OneE1NIS_3TwoEEvPS1_PT_(
+template void TPL<One>::N<Two> (One *, Two *);
+
+namespace NMS {
+class One;
+class Two;
+
+export template<typename T> struct TPL
+{
+void M (T *);
+template<typename U> void N (T *, U*);
+};
+
+template<typename T>
+void TPL<T>::M (T *) {}
+
+template<typename T> template<typename U> void TPL<T>::N (T *, U*) {}
+
+// CHECK-DAG: void @_ZN3NMSW3FOO3TPLINS_S0_3OneEE1MEPS2_(
+template void TPL<One>::M (One *);
+// CHECK-DAG: void @_ZN3NMSW3FOO3TPLINS_S0_3OneEE1NINS_S0_3TwoEEEvPS2_PT_
+template void TPL<One>::N<Two> (One *, Two *);
+}