ThinLTOBitcodeWriter: Try harder to discard unused references to the merged module.

[pcc]

Summary

If the thin module has no references to an internal global in the
merged module, we need to make sure to preserve that property if the
global is a member of a comdat group, as otherwise promotion can end
up adding global symbols to the comdat, which is not allowed.

This situation can arise if the external global in the thin module
has dead constant users, which would cause use_empty() to return
false and would cause us to try to promote it. To prevent this from
happening, discard the dead constant users before asking whether a
global is empty.

[tejohnson]

LGTM - I assume the modified test fails without the patch? I can't tell from looking at it whether it anon would have dead constant users.

[pcc]

In D40593#941225, @tejohnson wrote:

LGTM - I assume the modified test fails without the patch?

It in fact passes because I made a mistake in the test. The declaration that appears in the thin module looks like this:

@"anon$ae77054242d4de6843fe9803b6a8ac9f" = external hidden unnamed_addr constant { [1 x i8*] }

I will adjust the test so that it would match this.

I can't tell from looking at it whether it anon would have dead constant users.

It would indeed, specifically the constant getelementptr inbounds ({ [1 x i8*] }, { [1 x i8*] }* @anon, i32 0, i32 0, i32 1 in @al's definition, which is left unreferenced when we drop @al from the thin module in filterModule.

llvm/trunk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

 void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId,
                       SetVector<GlobalValue *> &PromoteExtra) {
   DenseMap<const Comdat *, Comdat *> RenamedComdats;
   for (auto &ExportGV : ExportM.global_values()) {
     if (!ExportGV.hasLocalLinkage())
       continue;
 
     auto Name = ExportGV.getName();
-    GlobalValue *ImportGV = ImportM.getNamedValue(Name);
-    if ((!ImportGV || ImportGV->use_empty()) && !PromoteExtra.count(&ExportGV))
+    GlobalValue *ImportGV = nullptr;
+    if (!PromoteExtra.count(&ExportGV)) {
+      ImportGV = ImportM.getNamedValue(Name);
+      if (!ImportGV)
         continue;
+      ImportGV->removeDeadConstantUsers();
+      if (ImportGV->use_empty()) {
+        ImportGV->eraseFromParent();
+        continue;
+      }
+    }
 
     std::string NewName = (Name + ModuleId).str();
 
     if (const auto *C = ExportGV.getComdat())
       if (C->getName() == Name)
         RenamedComdats.try_emplace(C, ExportM.getOrInsertComdat(NewName));
 
     ExportGV.setName(NewName);

llvm/trunk/test/Transforms/ThinLTOBitcodeWriter/filter-alias.ll

 ; 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
+; CHECK0-NOT: @{{.*}}anon{{.*}}=
 ; CHECK0: @al = external global i8*
+; CHECK0-NOT: @{{.*}}anon{{.*}}=
 ; CHECK1: @al = unnamed_addr alias i8*,
 
 target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc19.0.24215"
 
 $al = comdat any
 
 @anon = private unnamed_addr constant { [1 x i8*] } { [1 x i8*] [i8* null] }, comdat($al), !type !0
 
 @al = external unnamed_addr alias i8*, getelementptr inbounds ({ [1 x i8*] }, { [1 x i8*] }* @anon, i32 0, i32 0, i32 1)
 
 @foo = global i32 1
 
 !0 = !{i64 8, !"?AVA@@"}