[PATCH] D31735: ThinLTOBitcodeWriter: delete comdats if their keys are renamed

[Bob Haarman via Phabricator via llvm-commits]

inglorion updated this revision to Diff 94437.
inglorion added a comment.

two-pass implementation


https://reviews.llvm.org/D31735

Files:
  lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  test/Transforms/ThinLTOBitcodeWriter/comdat.ll


Index: test/Transforms/ThinLTOBitcodeWriter/comdat.ll
===================================================================
--- /dev/null
+++ test/Transforms/ThinLTOBitcodeWriter/comdat.ll
@@ -0,0 +1,25 @@
+; RUN: opt -thinlto-bc -o %t %s
+; RUN: llvm-modextract -n 0 -o - %t | llvm-dis | FileCheck --check-prefix=CHECK0 %s
+; RUN: llvm-modextract -n 1 -o - %t | llvm-dis | FileCheck --check-prefix=CHECK1 %s
+
+; foo should have been renamed, and there should no longer be a comdat
+; with the original name.
+; CHECK0: {{@"?foo[^ ]+}} = external hidden global
+; CHECK1-NOT: $foo = comdat
+; CHECK1: {{@"?foo[^ ]+}} = hidden
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.0.24215"
+
+$foo = comdat any
+
+ at anon = private unnamed_addr constant { [1 x i8*] } { [1 x i8*] [i8* null] }, comdat($foo), !type !0
+
+ at foo = internal unnamed_addr alias i8*, getelementptr inbounds ({ [1 x i8*] }, { [1 x i8*] }* @anon, i32 0, i32 0, i32 1)
+
+define i8* @bar() {
+  %1 = load i8*, i8** @foo
+  ret i8* %1
+}
+
+!0 = !{i64 8, !"?AVA@@"}
Index: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
===================================================================
--- lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -68,16 +68,26 @@
 
 // Promote each local-linkage entity defined by ExportM and used by ImportM by
 // changing visibility and appending the given ModuleId.
-void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId) {
+void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId,
+                      std::set<const Comdat *> &ComdatsToRemove) {
   for (auto &ExportGV : ExportM.global_values()) {
     if (!ExportGV.hasLocalLinkage())
       continue;
 
-    GlobalValue *ImportGV = ImportM.getNamedValue(ExportGV.getName());
+    StringRef Name = ExportGV.getName();
+    GlobalValue *ImportGV = ImportM.getNamedValue(Name);
     if (!ImportGV || ImportGV->use_empty())
       continue;
 
-    std::string NewName = (ExportGV.getName() + ModuleId).str();
+    std::string NewName = (Name + ModuleId).str();
+
+    // COFF requires that every COMDAT contain a symbol with the same name
+    // as the COMDAT. If renaming this value would break that requirement,
+    // delete the COMDAT. This is safe, because this COMDAT is not going to
+    // be merged anyway.
+    if (const Comdat *C = ExportGV.getComdat())
+      if (C->getName() == Name)
+        ComdatsToRemove.insert(C);
 
     ExportGV.setName(NewName);
     ExportGV.setLinkage(GlobalValue::ExternalLinkage);
@@ -147,6 +157,17 @@
   }
 }
 
+void removeComdats(Module &M, const std::set<const Comdat *> &ComdatsToRemove) {
+  for (auto &GV : M.globals())
+    if (GV.hasComdat() && ComdatsToRemove.count(GV.getComdat()))
+      if (auto *GO = dyn_cast<GlobalObject>(&GV))
+        GO->setComdat(nullptr);
+
+  auto &Comdats = M.getComdatSymbolTable();
+  for (const Comdat *C : ComdatsToRemove)
+    Comdats.erase(C->getName());
+}
+
 // Drop unused globals, and drop type information from function declarations.
 // FIXME: If we made functions typeless then there would be no need to do this.
 void simplifyExternals(Module &M) {
@@ -317,8 +338,11 @@
     return true;
   });
 
-  promoteInternals(*MergedM, M, ModuleId);
-  promoteInternals(M, *MergedM, ModuleId);
+  std::set<const Comdat *> ComdatsToRemove;
+  promoteInternals(*MergedM, M, ModuleId, ComdatsToRemove);
+  promoteInternals(M, *MergedM, ModuleId, ComdatsToRemove);
+  removeComdats(*MergedM, ComdatsToRemove);
+  removeComdats(M, ComdatsToRemove);
 
   simplifyExternals(*MergedM);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31735.94437.patch
Type: text/x-patch
Size: 3656 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170406/a55b8b8f/attachment.bin>