Don't merge global constants with non-dbg metadata.

[eugenis]

Summary

!type metadata can not be dropped. An alternative to this is adding
!type metadata from the replaced globals to the replacement, but that
may weaken type tests and make them slower at the same time.

The merged global gets !dbg metadata from replaced globals, and can
end up with multiple debug locations

[pcc]

I think there is a pre-existing bug in this pass in that it can effectively *add* metadata to a global (by replacing references to a global without metadata with references to a global with metadata). Can you fix both issues at once by refusing to merge if either global has any metadata at all?

[pcc]

LGTM

[eugenis]

https://reviews.llvm.org/rL297327

lib/Transforms/IPO/ConstantMerge.cpp

if (!A.hasLocalLinkage() && B.hasLocalLinkage())
     return true;
 
   if (A.hasLocalLinkage() && !B.hasLocalLinkage())
     return false;
 
   return A.hasGlobalUnnamedAddr();
 }
 
+static bool hasMetadataOtherThanDebugLoc(const GlobalVariable *GV) {
+  SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
+  GV->getAllMetadata(MDs);
+  for (const auto &V : MDs)
+    if (V.first != LLVMContext::MD_dbg)
+      return true;
+  return false;
+}
+
+static void copyDebugLocMetadata(const GlobalVariable *From,
+                                 GlobalVariable *To) {
+  SmallVector<DIGlobalVariableExpression *, 1> MDs;
+  From->getDebugInfo(MDs);
+  for (auto MD : MDs)
+    To->addDebugInfo(MD);
+}
+
 static unsigned getAlignment(GlobalVariable *GV) {
   unsigned Align = GV->getAlignment();
   if (Align)
     return Align;
   return GV->getParent()->getDataLayout().getPreferredAlignment(GV);
 }
 
 static bool mergeConstants(Module &M) {
for (Module::global_iterator GVI = M.global_begin(), E = M.global_end();
 
       // This transformation is legal for weak ODR globals in the sense it
       // doesn't change semantics, but we really don't want to perform it
       // anyway; it's likely to pessimize code generation, and some tools
       // (like the Darwin linker in cases involving CFString) don't expect it.
       if (GV->isWeakForLinker())
         continue;
 
+      // Don't touch globals with metadata other then !dbg.
+      if (hasMetadataOtherThanDebugLoc(GV))
+        continue;
+
       Constant *Init = GV->getInitializer();
 
       // Check to see if the initializer is already known.
       GlobalVariable *&Slot = CMap[Init];
 
       // If this is the first constant we find or if the old one is local,
       // replace with the current one. If the current is externally visible
       // it cannot be replace, but can be the canonical constant we merge with.
for (Module::global_iterator GVI = M.global_begin(), E = M.global_end();
       GlobalVariable *Slot = CMap[Init];
 
       if (!Slot || Slot == GV)
         continue;
 
       if (!Slot->hasGlobalUnnamedAddr() && !GV->hasGlobalUnnamedAddr())
         continue;
 
+      if (hasMetadataOtherThanDebugLoc(GV))
+        continue;
+
       if (!GV->hasGlobalUnnamedAddr())
         Slot->setUnnamedAddr(GlobalValue::UnnamedAddr::None);
 
       // Make all uses of the duplicate constant use the canonical version.
       Replacements.push_back(std::make_pair(GV, Slot));
     }
 
     if (Replacements.empty())
       return MadeChange;
     CMap.clear();
 
     // Now that we have figured out which replacements must be made, do them all
     // now.  This avoid invalidating the pointers in CMap, which are unneeded
     // now.
     for (unsigned i = 0, e = Replacements.size(); i != e; ++i) {
       // Bump the alignment if necessary.
       if (Replacements[i].first->getAlignment() ||
           Replacements[i].second->getAlignment()) {
         Replacements[i].second->setAlignment(
             std::max(getAlignment(Replacements[i].first),
                      getAlignment(Replacements[i].second)));
       }
 
+      copyDebugLocMetadata(Replacements[i].first, Replacements[i].second);
+
       // Eliminate any uses of the dead global.
       Replacements[i].first->replaceAllUsesWith(Replacements[i].second);
 
       // Delete the global value from the module.
       assert(Replacements[i].first->hasLocalLinkage() &&
              "Refusing to delete an externally visible global variable.");
       Replacements[i].first->eraseFromParent();
     }

test/Transforms/ConstantMerge/dont-merge.ll

 @T3B = internal constant i32 0
 @llvm.used = appending global [2 x i32*] [i32* @T3A, i32* @T3B], section
 "llvm.metadata"
 
 define void @test3() {
   call void asm sideeffect "T3A, T3B",""() ; invisible use of T3A and T3B
   ret void
 }
+
+; Don't merge constants with !type annotations.
+
+@T4A1 = internal constant i32 2, !type !0
+@T4A2 = internal unnamed_addr constant i32 2, !type !1
+
+@T4B1 = internal constant i32 3, !type !0
+@T4B2 = internal unnamed_addr constant i32 3, !type !0
+
+@T4C1 = internal constant i32 4, !type !0
+@T4C2 = unnamed_addr constant i32 4
+
+@T4D1 = unnamed_addr constant i32 5, !type !0
+@T4D2 = internal constant i32 5
+
+!0 = !{i64 0, !"typeinfo name for A"}
+!1 = !{i64 0, !"typeinfo name for B"}
+
+; CHECK: @T4A1
+; CHECK: @T4A2
+; CHECK: @T4B1
+; CHECK: @T4B2
+; CHECK: @T4C1
+; CHECK: @T4C2
+; CHECK: @T4D1
+; CHECK: @T4D2
+
+define void @test4(i32** %P1, i32** %P2, i32** %P3, i32** %P4, i32** %P5, i32** %P6, i32** %P7, i32** %P8) {
+        store i32* @T4A1, i32** %P1
+        store i32* @T4A2, i32** %P2
+        store i32* @T4B1, i32** %P3
+        store i32* @T4B2, i32** %P4
+        store i32* @T4C1, i32** %P5
+        store i32* @T4C2, i32** %P6
+        store i32* @T4D1, i32** %P7
+        store i32* @T4D2, i32** %P8
+        ret void
+}

test/Transforms/ConstantMerge/merge-dbg.ll

+; RUN: opt < %s -constmerge -S | FileCheck %s
+
+; CHECK: = constant i32 1, !dbg [[A:![0-9]+]], !dbg [[B:![0-9]+]]
+@a = internal constant i32 1, !dbg !0
+@b = unnamed_addr constant i32 1, !dbg !9
+
+define void @test1(i32** %P1, i32** %P2) {
+  store i32* @a, i32** %P1
+  store i32* @b, i32** %P2
+  ret void
+}
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!7, !8}
+
+; CHECK: [[A]] = !DIGlobalVariableExpression(var: [[VA:![0-9]+]])
+; CHECK: [[VA]] = distinct !DIGlobalVariable(name: "y"
+; CHECK: [[B]] = !DIGlobalVariableExpression(var: [[VB:![0-9]+]])
+; CHECK: [[VB]] = distinct !DIGlobalVariable(name: "x"
+
+!0 = !DIGlobalVariableExpression(var: !1)
+!1 = distinct !DIGlobalVariable(name: "x", scope: !2, file: !3, line: 1, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 5.0.0 (trunk 297227) (llvm/trunk 297234)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "1.cc", directory: "/build")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!7 = !{i32 2, !"Dwarf Version", i32 4}
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+
+!9 = !DIGlobalVariableExpression(var: !10)
+!10 = distinct !DIGlobalVariable(name: "y", scope: !2, file: !3, line: 1, type: !6, isLocal: false, isDefinition: true)