Disable Unique Internal Linkage Names for internal global vars

[tmsriram]

Summary

Demanglers do not recognize clone suffixes for global variables, so disable unique linkage suffixes until demanglers can be fixed.

D96109 added support for unique internal linkage names for both internal linkage functions and global variables. There was a lot of discussion on how to get the demangling right for functions but I completely missed the point that demanglers do not support suffixes for global vars. For example:

$ c++filt _ZL3foo
foo
$ c++filt _ZL3foo.uniq.123
_ZL3foo.uniq.123

The demangling for functions works as expected.

I am not sure of the impact of this. I don't understand how debuggers and other tools depend on the correctness of global variable demangling so I am pre-emptively disabling it until we can get the demangling support added.

Importantly, uniquefying global variables is not needed right now as we do not do profile attribution to global vars based on sampling. It was added for completeness and so this feature is not exactly missed.

Sorry about missing this in the previous change, it was a blind spot.

[hoy]

Thanks for the fix. This should have no impact on AutoFDO.

[dblaikie]

clang/lib/AST/ItaniumMangle.cpp

637–646

/might/ be marginally better using an early exit

clang/lib/CodeGen/CodeGenModule.cpp

1183–1186

Perhaps this would be simpler to return the expression.

Or could be split out into a series of early exit:

if (empty module hash)
  return false;
if (!isa function)
  return false;
if (!= internal linkage)
  return false;
return true;

[tmsriram]

Address reviewer comments and refactor code as mentioned.

[dblaikie]

Looks good

clang/lib/AST/ItaniumMangle.cpp

static bool isInternalLinkageDecl(const NamedDecl *ND) {
   if (ND && ND->getFormalLinkage() == InternalLinkage &&
       !ND->isExternallyVisible() &&
       getEffectiveDeclContext(ND)->isFileContext() &&
       !ND->isInAnonymousNamespace())
     return true;
   return false;
 }
 
-// Check if this Decl needs a unique internal linkage name.
+// Check if this Function Decl needs a unique internal linkage name.
 bool ItaniumMangleContextImpl::isUniqueInternalLinkageDecl(
     const NamedDecl *ND) {
   if (!NeedsUniqueInternalLinkageNames || !ND)
     return false;
 
+  if (auto *FD = dyn_cast<FunctionDecl>(ND)) {
     // For C functions without prototypes, return false as their
     // names should not be mangled.
-  if (auto *FD = dyn_cast<FunctionDecl>(ND)) {
     if (!FD->getType()->getAs<FunctionProtoType>())
       return false;
-  }
-
     if (isInternalLinkageDecl(ND))
       return true;
+  }
+
   return false;
 }
 
 bool ItaniumMangleContextImpl::shouldMangleCXXName(const NamedDecl *D) {
   const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
   if (FD) {
     LanguageLinkage L = FD->getLanguageLinkage();
     // Overloadable functions need mangling.
     if (FD->hasAttr<OverloadableAttr>())

clang/lib/CodeGen/CodeGenModule.cpp

static void AppendTargetMangling(const CodeGenModule &CGM,
   for (StringRef Feat : Info.Features) {
     if (!IsFirst)
       Out << '_';
     IsFirst = false;
     Out << Feat.substr(1);
   }
 }
 
-// Returns true if GD is a function/var decl with internal linkage and
+// Returns true if GD is a function decl with internal linkage and
 // needs a unique suffix after the mangled name.
 static bool isUniqueInternalLinkageDecl(GlobalDecl GD,
                                         CodeGenModule &CGM) {
   const Decl *D = GD.getDecl();
-  if (!CGM.getModuleNameHash().empty() &&
-      ((isa<FunctionDecl>(D) &&
-        CGM.getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage) ||
-       (isa<VarDecl>(D) && CGM.getContext().GetGVALinkageForVariable(
-                               cast<VarDecl>(D)) == GVA_Internal)))
+  if (!CGM.getModuleNameHash().empty() && isa<FunctionDecl>(D) &&
+      CGM.getFunctionLinkage(GD) == llvm::GlobalValue::InternalLinkage)
     return true;
   return false;
 }
 
 static std::string getMangledNameImpl(CodeGenModule &CGM, GlobalDecl GD,
                                       const NamedDecl *ND,
                                       bool OmitMultiVersionMangling = false) {
   SmallString<256> Buffer;
   llvm::raw_svector_ostream Out(Buffer);
   MangleContext &MC = CGM.getCXXABI().getMangleContext();

clang/test/CodeGen/unique-internal-linkage-names-dwarf.c

 }
 
 // PLAIN: @glob = internal global i32
 // PLAIN: define internal i32 @foo()
 // PLAIN: distinct !DIGlobalVariable(name: "glob"{{.*}})
 // PLAIN: distinct !DISubprogram(name: "foo"{{.*}})
 // PLAIN-NOT: linkageName:
 //
-// UNIQUE: @_ZL4glob.[[MODHASH:__uniq.[0-9]+]] = internal global i32
-// UNIQUE: define internal i32 @_ZL3foov.[[MODHASH]]()
-// UNIQUE: distinct !DIGlobalVariable(name: "glob", linkageName: "_ZL4glob.[[MODHASH]]"{{.*}})
+// UNIQUE: @glob = internal global i32
+// UNIQUE: define internal i32 @_ZL3foov.[[MODHASH:__uniq.[0-9]+]]()
+// UNIQUE: distinct !DIGlobalVariable(name: "glob"{{.*}})
 // UNIQUE: distinct !DISubprogram(name: "foo", linkageName: "_ZL3foov.[[MODHASH]]"{{.*}})

clang/test/CodeGen/unique-internal-linkage-names-dwarf.cpp

 // PLAIN-DAG: define internal i32 @_ZN12_GLOBAL__N_13barEv()
 // PLAIN-DAG: distinct !DIGlobalVariable(name: "glob_bar", linkageName: "_ZN12_GLOBAL__N_18glob_barE"{{.*}})
 // PLAIN-DAG: distinct !DISubprogram(name: "bar", linkageName: "_ZN12_GLOBAL__N_13barEv"{{.*}})
 // PLAIN-DAG: @_ZL8glob_zip = internal global i32
 // PLAIN-DAG: define internal i32 @_ZL3zipv()
 // PLAIN-DAG: distinct !DIGlobalVariable(name: "glob_zip", linkageName: "_ZL8glob_zip"{{.*}})
 // PLAIN-DAG: distinct !DISubprogram(name: "zip", linkageName: "_ZL3zipv"{{.*}})
 
-// UNIQUE-DAG: @_ZL8glob_foo.[[MODHASH:__uniq\.[0-9]+]] = internal global i32
-// UNIQUE-DAG: define internal i32 @_ZL3foov.[[MODHASH]]()
-// UNIQUE-DAG: distinct !DIGlobalVariable(name: "glob_foo", linkageName: "_ZL8glob_foo.[[MODHASH]]"{{.*}})
+// UNIQUE-DAG: @_ZL8glob_foo = internal global i32
+// UNIQUE-DAG: define internal i32 @_ZL3foov.[[MODHASH:__uniq\.[0-9]+]]()
+// UNIQUE-DAG: distinct !DIGlobalVariable(name: "glob_foo", linkageName: "_ZL8glob_foo"{{.*}})
 // UNIQUE-DAG: distinct !DISubprogram(name: "foo", linkageName: "_ZL3foov.[[MODHASH]]"{{.*}})
-// UNIQUE-DAG: @_ZN12_GLOBAL__N_18glob_barE.[[MODHASH]] = internal global i32
+// UNIQUE-DAG: @_ZN12_GLOBAL__N_18glob_barE = internal global i32
 // UNIQUE-DAG: define internal i32 @_ZN12_GLOBAL__N_13barEv.[[MODHASH]]()
-// UNIQUE-DAG: distinct !DIGlobalVariable(name: "glob_bar", linkageName: "_ZN12_GLOBAL__N_18glob_barE.[[MODHASH]]"{{.*}})
+// UNIQUE-DAG: distinct !DIGlobalVariable(name: "glob_bar", linkageName: "_ZN12_GLOBAL__N_18glob_barE"{{.*}})
 // UNIQUE-DAG: distinct !DISubprogram(name: "bar", linkageName: "_ZN12_GLOBAL__N_13barEv.[[MODHASH]]"{{.*}})
-// UNIQUE-DAG: @_ZL8glob_zip.[[MODHASH]] = internal global i32
+// UNIQUE-DAG: @_ZL8glob_zip = internal global i32
 // UNIQUE-DAG: define internal i32 @_ZL3zipv.[[MODHASH]]()
-// UNIQUE-DAG: distinct !DIGlobalVariable(name: "glob_zip", linkageName: "_ZL8glob_zip.[[MODHASH]]"{{.*}})
+// UNIQUE-DAG: distinct !DIGlobalVariable(name: "glob_zip", linkageName: "_ZL8glob_zip"{{.*}})
 // UNIQUE-DAG: distinct !DISubprogram(name: "zip", linkageName: "_ZL3zipv.[[MODHASH]]"{{.*}})

clang/test/CodeGen/unique-internal-linkage-names.cpp

 // PLAIN: @_ZZ8retAnonMvE5fGlob = internal global
 // PLAIN: @_ZN12_GLOBAL__N_16anon_mE = internal global
 // PLAIN: define internal i32 @_ZL3foov()
 // PLAIN: define internal i32 @_ZN12_GLOBAL__N_14getMEv
 // PLAIN: define weak_odr i32 ()* @_ZL4mverv.resolver()
 // PLAIN: define internal i32 @_ZL4mverv()
 // PLAIN: define internal i32 @_ZL4mverv.sse4.2()
 // PLAIN-NOT: "sample-profile-suffix-elision-policy"
-// UNIQUE: @_ZL4glob.__uniq.{{[0-9]+}} = internal global
-// UNIQUE: @_ZZ8retAnonMvE5fGlob.__uniq.{{[0-9]+}} = internal global
-// UNIQUE: @_ZN12_GLOBAL__N_16anon_mE.__uniq.{{[0-9]+}} = internal global
-// UNIQUE: define internal i32 @_ZL3foov.__uniq.{{[0-9]+}}() #[[#ATTR:]] {
-// UNIQUE: define internal i32 @_ZN12_GLOBAL__N_14getMEv.__uniq.{{[0-9]+}}
-// UNIQUE: define weak_odr i32 ()* @_ZL4mverv.__uniq.{{[0-9]+}}.resolver()
-// UNIQUE: define internal i32 @_ZL4mverv.__uniq.{{[0-9]+}}()
-// UNIQUE: define internal i32 @_ZL4mverv.__uniq.{{[0-9]+}}.sse4.2
+// UNIQUE: @_ZL4glob = internal global
+// UNIQUE: @_ZZ8retAnonMvE5fGlob = internal global
+// UNIQUE: @_ZN12_GLOBAL__N_16anon_mE = internal global
+// UNIQUE: define internal i32 @_ZL3foov.[[MODHASH:__uniq.[0-9]+]]() #[[#ATTR:]] {
+// UNIQUE: define internal i32 @_ZN12_GLOBAL__N_14getMEv.[[MODHASH]]
+// UNIQUE: define weak_odr i32 ()* @_ZL4mverv.[[MODHASH]].resolver()
+// UNIQUE: define internal i32 @_ZL4mverv.[[MODHASH]]()
+// UNIQUE: define internal i32 @_ZL4mverv.[[MODHASH]].sse4.2
 // UNIQUE: attributes #[[#ATTR]] = { {{.*}}"sample-profile-suffix-elision-policy"{{.*}} }