Fix NAME.1 comdat name for replaced GlobalVariable

[yaron.keren]

Summary

In case of an existing GlobalVariable, the comdat is created using the name of the new GV (usually NAME.1) instead of the correct NAME of the old GV. Moving comdat creation after GV replacement solves this. Patch + testcase.

[rnk]

lgtm with slightly tighter test case.

tools/clang/test/CodeGenCXX/vtable-key-function-win-comdat.cpp

18

Can you also CHECK that the vtable definition has the right name, linkage, and uses this comdat?

[yaron.keren]

r243525 with expanded testcase.

tools/clang/lib/CodeGen/ItaniumCXXABI.cpp

 
   llvm::Constant *Init = llvm::ConstantStruct::getAnon(Fields);
 
   llvm::Module &M = CGM.getModule();
   llvm::GlobalVariable *GV =
       new llvm::GlobalVariable(M, Init->getType(),
                                /*Constant=*/true, Linkage, Init, Name);
 
-  if (CGM.supportsCOMDAT() && GV->isWeakForLinker())
-    GV->setComdat(M.getOrInsertComdat(GV->getName()));
-
   // If there's already an old global variable, replace it with the new one.
   if (OldGV) {
     GV->takeName(OldGV);
     llvm::Constant *NewPtr =
       llvm::ConstantExpr::getBitCast(GV, OldGV->getType());
     OldGV->replaceAllUsesWith(NewPtr);
     OldGV->eraseFromParent();
   }
 
+  if (CGM.supportsCOMDAT() && GV->isWeakForLinker())
+    GV->setComdat(M.getOrInsertComdat(GV->getName()));
+
   // The Itanium ABI specifies that type_info objects must be globally
   // unique, with one exception: if the type is an incomplete class
   // type or a (possibly indirect) pointer to one.  That exception
   // affects the general case of comparing type_info objects produced
   // by the typeid operator, which is why the comparison operators on
   // std::type_info generally use the type_info name pointers instead
   // of the object addresses.  However, the language's built-in uses
   // of RTTI generally require class types to be complete, even when

tools/clang/test/CodeGenCXX/vtable-key-function-win-comdat.cpp

+// RUN: %clang_cc1 %s -triple=x86_64-pc-windows-gnu -emit-llvm -o - | FileCheck %s
+
+namespace std { class type_info; }
+extern void use(const std::type_info &rtti);
+
+struct Test1a {
+  Test1a();
+  virtual void foo();
+  virtual void bar();
+};
+
+// V-table needs to be defined weakly.
+Test1a::Test1a() { use(typeid(Test1a)); }
+// This defines the key function.
+inline void Test1a::foo() {}
+
+// CHECK:     $_ZTS6Test1a = comdat any
+// CHECK-NOT: $_ZTI6Test1a.1 = comdat any