[LTO] Fix cloning of llvm*.used when splitting module

[tejohnson]

Summary

Refines the fix in 3c4c205060c9398da705eb71b63ddd8a04999de9 to only
put globals whose defs were cloned into the split regular LTO module
on the cloned llvm*.used globals. This avoids an issue where one of the
attached values was a local that was promoted in the original module
after the module was cloned. We only need to have the values defined in
the new module on those globals.

Fixes PR49251.

[tejohnson]

llvm/lib/Transforms/Utils/ModuleUtils.cpp

79–85

I forgot to add in the description - this is needed because when we originally clone the module over, the llvm*.used globals will be declarations in the merged module. So when we go to add the new values to their used set, we need to handle deleting the old one which doesn't have an initializer.

[MaskRay]

LGTM.

llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

204

llvm.used -> llvm[.compiler].used

"First" here made me think whether there should be another comment about "then" :)

llvm/test/Transforms/ThinLTOBitcodeWriter/split-used.ll

16

Perhaps add ; M1-NOT: @g

or change the following ; M1: @llvm.used to ; M1-NEXT: @llvm.used, to check that @g3 or @g4 is not copied.

26

The canonical form has , section("llvm.metadata").

With this new patch, , section("llvm.metadata") will be appended for M1. I assume that this different does not matter.

[tejohnson]

llvm/test/Transforms/ThinLTOBitcodeWriter/split-used.ll

26

The compiler adds this when creating one of these special global values. I didn't bother putting it on the hand created globals here since it didn't matter for the test and keeps it more minimal.

[tejohnson]

Address comments

llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

void forEachVirtualFunction(Constant *C, function_ref<void(Function *)> Fn) {
   if (auto *F = dyn_cast<Function>(C))
     return Fn(F);
   if (isa<GlobalValue>(C))
     return;
   for (Value *Op : C->operands())
     forEachVirtualFunction(cast<Constant>(Op), Fn);
 }
 
+// Clone any @llvm[.compiler].used over to the new module and append
+// values whose defs were cloned into that module.
+static void cloneUsedGlobalVariables(const Module &SrcM, Module &DestM,
+                                     bool CompilerUsed) {
+  SmallPtrSet<GlobalValue *, 8> Used;
+  SmallPtrSet<GlobalValue *, 8> NewUsed;
+  // First collect those in the llvm[.compiler].used set.
+  collectUsedGlobalVariables(SrcM, Used, CompilerUsed);
+  // Next build a set of the equivalent values defined in DestM.
+  for (auto *V : Used) {
+    auto *GV = DestM.getNamedValue(V->getName());
+    if (GV && !GV->isDeclaration())
+      NewUsed.insert(GV);
+  }
+  // Finally, add them to a llvm[.compiler].used variable in DestM.
+  if (CompilerUsed)
+    appendToCompilerUsed(
+        DestM, std::vector<GlobalValue *>(NewUsed.begin(), NewUsed.end()));
+  else
+    appendToUsed(DestM,
+                 std::vector<GlobalValue *>(NewUsed.begin(), NewUsed.end()));
+}
+
 // If it's possible to split M into regular and thin LTO parts, do so and write
 // a multi-module bitcode file with the two parts to OS. Otherwise, write only a
 // regular LTO bitcode file to OS.
 void splitAndWriteThinLTOBitcode(
     raw_ostream &OS, raw_ostream *ThinLinkOS,
     function_ref<AAResults &(Function &)> AARGetter, Module &M) {
   std::string ModuleId = getUniqueModuleId(&M);
   if (ModuleId.empty()) {
if (HasTypeMetadata(&GV)) {
             computeFunctionBodyMemoryAccess(*F, AARGetter(*F)) == MAK_ReadNone)
           EligibleVirtualFns.insert(F);
       });
     }
 
   ValueToValueMapTy VMap;
   std::unique_ptr<Module> MergedM(
       CloneModule(M, VMap, [&](const GlobalValue *GV) -> bool {
-        // Clone any llvm.*used globals to ensure the included values are
-        // not deleted.
-        if (GV->getName() == "llvm.used" ||
-            GV->getName() == "llvm.compiler.used")
-          return true;
         if (const auto *C = GV->getComdat())
           if (MergedMComdats.count(C))
             return true;
         if (auto *F = dyn_cast<Function>(GV))
           return EligibleVirtualFns.count(F);
         if (auto *GVar = dyn_cast_or_null<GlobalVariable>(GV->getBaseObject()))
           return HasTypeMetadata(GVar);
         return false;
       }));
   StripDebugInfo(*MergedM);
   MergedM->setModuleInlineAsm("");
 
+  // Clone any llvm.*used globals to ensure the included values are
+  // not deleted.
+  cloneUsedGlobalVariables(M, *MergedM, /*CompilerUsed*/ false);
+  cloneUsedGlobalVariables(M, *MergedM, /*CompilerUsed*/ true);
+
   for (Function &F : *MergedM)
     if (!F.isDeclaration()) {
       // Reset the linkage of all functions eligible for virtual constant
       // propagation. The canonical definitions live in the thin LTO module so
       // that they can be imported.
       F.setLinkage(GlobalValue::AvailableExternallyLinkage);
       F.setComdat(nullptr);
     }

llvm/lib/Transforms/Utils/ModuleUtils.cpp

void llvm::appendToGlobalDtors(Module &M, Function *F, int Priority, Constant *Data) {
   appendToGlobalArray("llvm.global_dtors", M, F, Priority, Data);
 }
 
 static void appendToUsedList(Module &M, StringRef Name, ArrayRef<GlobalValue *> Values) {
   GlobalVariable *GV = M.getGlobalVariable(Name);
   SmallPtrSet<Constant *, 16> InitAsSet;
   SmallVector<Constant *, 16> Init;
   if (GV) {
+    if (GV->hasInitializer()) {
       auto *CA = cast<ConstantArray>(GV->getInitializer());
       for (auto &Op : CA->operands()) {
         Constant *C = cast_or_null<Constant>(Op);
         if (InitAsSet.insert(C).second)
           Init.push_back(C);
       }
+    }
     GV->eraseFromParent();
   }
 
   Type *Int8PtrTy = llvm::Type::getInt8PtrTy(M.getContext());
   for (auto *V : Values) {
     Constant *C = ConstantExpr::getBitCast(V, Int8PtrTy);
     if (InitAsSet.insert(C).second)
       Init.push_back(C);

llvm/test/Transforms/ThinLTOBitcodeWriter/split-used.ll

+; Test to ensure that @llvm[.compiler].used is cloned to the split module for
+; any globals whose defs were cloned to that module.
+
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t %s
+; RUN: llvm-modextract -b -n 0 -o %t0.bc %t
+; RUN: llvm-modextract -b -n 1 -o %t1.bc %t
+; RUN: llvm-dis -o - %t0.bc | FileCheck --check-prefix=M0 %s
+; RUN: llvm-dis -o - %t1.bc | FileCheck --check-prefix=M1 %s
+
+; M0: @g1 = external global i8
+; M0: @g2 = external global i8
+; M0: @g3 = global i8 42
+; M0: @g4 = global i8 42
+; M1: @g1 = global i8 42, !type !0
+; M1: @g2 = global i8 42, !type !0
+; M1-NOT: @g
+@g1 = global i8 42, !type !0
+@g2 = global i8 42, !type !0
+@g3 = global i8 42
+@g4 = global i8 42
+
+; M0: @llvm.used = appending global [2 x i8*] [i8* @g1, i8* @g3]
+; M0: @llvm.compiler.used = appending global [2 x i8*] [i8* @g2, i8* @g4]
+; M1: @llvm.used = appending global [1 x i8*] [i8* @g1]
+; M1: @llvm.compiler.used = appending global [1 x i8*] [i8* @g2]
[e-mail] = appending global [2 x i8*] [ i8* @g1, i8* @g3]
[e-mail] = appending global [2 x i8*] [ i8* @g2, i8* @g4]
+
+; M1: !0 = !{i32 0, !"typeid"}
+!0 = !{i32 0, !"typeid"}