[LTO] Use names from more GlobalValue types to generate the ModuleID

[ormris]

Summary

In many situations the current version of getUniqueModuleId returns no hash. This causes problems for ThinLTOBitcodeWriter pass, and it will fall back to generating a regular LTO module in these cases. Since it's preferable to generate a ThinLTO module for performance reasons, this patch allows getUniqueModuleId to use more types of GlobalValues when generating the ModuleId hash. This mix has been fairly reliable for us.

This change is part of the Unified LTO RFC: https://discourse.llvm.org/t/rfc-a-unified-lto-bitcode-frontend/61774

[ormris]

Update patch with fixes for some PowerPC tests.

[pcc]

I think this will introduce a correctness issue as it will become possible for two modules that can be linked together to have the same unique module ID.

[ormris]

This patch causes several test failures. Currently working on resolving those.

[ormris]

I think this will introduce a correctness issue as it will become possible for two modules that can be linked together to have the same unique module ID.

Our goal with this change was to reduce the hash collisions we were finding in internal testing of the unified LTO pipeline. The previous method wasn't quite broad enough for our internal tests. While we've certainly constructed test cases that cause a collision when using the changes in this patch, we've never seen a collision when working with real code.

[ormris]

Changelog:

• Changes are now implemented as a fallback, reducing necessary test changes.
• Add option to allow testing cases using empty module IDs.
• Reduce complexity of symbol filter

[ormris]

Update to remove unnecessary -unified-lto flag from tests.

[ormris]

clang-format

[ormris]

gentle ping

[ormris]

ping

llvm/lib/Transforms/Utils/ModuleUtils.cpp

 
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/xxhash.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "moduleutils"
 
+static cl::opt<bool> EmptyModuleId("empty-moduleid", cl::init(false),
+                                   cl::Hidden,
+                                   cl::desc("Always emit an empty ModuleID"));
+
 static void appendToGlobalArray(StringRef ArrayName, Module &M, Function *F,
                                 int Priority, Constant *Data) {
   IRBuilder<> IRB(M.getContext());
   FunctionType *FnTy = FunctionType::get(IRB.getVoidTy(), false);
 
   // Get the current set of static global constructors and add the new ctor
   // to the list.
   SmallVector<Constant *, 16> CurrentCtors;
void llvm::filterDeadComdatFunctions(
   // Only keep functions which have no comdat or a dead comdat.
   erase_if(DeadComdatFunctions, [&](Function *F) {
     Comdat *C = F->getComdat();
     return C && !DeadComdats.contains(C);
   });
 }
 
 std::string llvm::getUniqueModuleId(Module *M) {
+  if (EmptyModuleId)
+    return "";
+
   MD5 Md5;
   bool ExportsSymbols = false;
   auto AddGlobal = [&](GlobalValue &GV) {
     if (GV.isDeclaration() || GV.getName().startswith("llvm.") ||
         !GV.hasExternalLinkage() || GV.hasComdat())
       return;
     ExportsSymbols = true;
     Md5.update(GV.getName());
     Md5.update(ArrayRef<uint8_t>{0});
   };
 
   for (auto &F : *M)
     AddGlobal(F);
   for (auto &GV : M->globals())
     AddGlobal(GV);
   for (auto &GA : M->aliases())
     AddGlobal(GA);
   for (auto &IF : M->ifuncs())
     AddGlobal(IF);
 
+  if (!ExportsSymbols) {
+    Md5.update(M->getModuleIdentifier());
+    Md5.update(ArrayRef<uint8_t>{0});
+
+    auto AddWeakerGlobal = [&](const GlobalValue &GV) {
+      if (GV.isDeclaration() || GV.getName().startswith("llvm.") ||
+          GV.hasWeakLinkage()) {
+        ExportsSymbols = true;
+        Md5.update(GV.getName());
+        Md5.update(ArrayRef<uint8_t>{0});
+      }
+    };
+
+    // When UnifiedLTO is enabled, we always need to generate split modules
+    // which requires a ModuleID. Using more types of metadata to generate this
+    // hash helps to ensure this.
+    auto AddArrayElt =
+        [&](const char *array,
+            std::function<const GlobalValue *(const Value *)> fun) {
+          if (auto *Used = M->getNamedGlobal(array))
+            if (const auto *A = dyn_cast<ConstantArray>(Used->getInitializer()))
+              for (const Value *Op : A->operands())
+                if (const GlobalValue *GV = fun(Op))
+                  AddWeakerGlobal(*GV);
+        };
+    AddArrayElt("llvm.used", [&](const Value *Op) -> const GlobalValue * {
+      return dyn_cast<GlobalValue>(Op->stripPointerCasts());
+    });
+    AddArrayElt("llvm.global_ctors",
+                [&](const Value *Op) -> const GlobalValue * {
+                  if (const auto *CS = dyn_cast<ConstantStruct>(Op))
+                    if (const auto *GV =
+                            dyn_cast<GlobalValue>(CS->getAggregateElement(1)))
+                      return GV;
+                  return nullptr;
+                });
+
+    for (auto &GV : M->globals())
+      AddWeakerGlobal(GV);
+  }
+
   if (!ExportsSymbols)
     return "";
 
   MD5::MD5Result R;
   Md5.final(R);
 
   SmallString<32> Str;
   MD5::stringifyResult(R, Str);
+
+  LLVM_DEBUG(dbgs() << "Generated ModuleID: " << Str.str() << "\n");
+
   return ("." + Str).str();
 }
 
 void VFABI::setVectorVariantNames(CallInst *CI,
                                   ArrayRef<std::string> VariantMappings) {
   if (VariantMappings.empty())
     return;
 

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

-; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
-; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff -empty-moduleid < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff -empty-moduleid < %s | FileCheck %s
 
 @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @foo, ptr null }]
 
 define internal void @foo() {
   ret void
 }
 
 ; FIXME: Adjust the comment after we use source file full path to generate unique

llvm/test/LTO/Resolution/X86/llvm-used-moduleid.ll

+; RUN: opt <%s -thinlto-bc -thinlto-split-lto-unit -o %t0
+; RUN: llvm-bcanalyzer %t0 | FileCheck %s
+
+target triple="x86_64-pc-linux-gnu"
+
[e-mail] = appending global [1 x i8*] [i8* bitcast (void ()* @a to i8*)], !type !0
+
+declare i1 @llvm.type.test(i8*, metadata)
+declare void @llvm.assume(i1)
+
+; CHECK: GLOBALVAL_SUMMARY_BLOCK
+; CHECK: FULL_LTO_GLOBALVAL_SUMMARY_BLOCK
+define internal void @a() {
+  %1 = call i1 @llvm.type.test(i8* undef, metadata !"a")
+  call void @llvm.assume(i1 %1)
+  ret void
+}
+
+!0 = !{i64 16, !"a"}

llvm/test/LTO/Resolution/X86/weak-linkage-moduleid.ll

+; RUN: opt <%s -thinlto-bc -thinlto-split-lto-unit -o %t0
+; RUN: llvm-bcanalyzer %t0 | FileCheck %s
+
+target triple="x86_64-pc-linux-gnu"
+
+@p = weak global void ()* @a, !type !0
+
+declare i1 @llvm.type.test(i8*, metadata)
+declare void @llvm.assume(i1)
+
+; CHECK: GLOBALVAL_SUMMARY_BLOCK
+; CHECK: FULL_LTO_GLOBALVAL_SUMMARY_BLOCK
+define internal void @a() {
+  %1 = call i1 @llvm.type.test(i8* undef, metadata !"a")
+  call void @llvm.assume(i1 %1)
+  ret void
+}
+
+!0 = !{i64 16, !"a"}

llvm/test/LTO/X86/path-moduleid.ll

+; REQUIRES: asserts
+; Test that chaning the path to the module doesn't effect the ModuleID. This
+; file and Inputs/path-moduleid.ll have the same content, and thus should have
+; the same ModuleID.
+; RUN: opt -o /dev/null -thinlto-bc -thinlto-split-lto-unit=1 --debug-only=moduleutils %s 2>%t0
+; RUN: mkdir -p %t; cp %s %t/path-moduleid.ll
+; RUN: opt -o /dev/null -thinlto-bc -thinlto-split-lto-unit=1 --debug-only=moduleutils %t/path-moduleid.ll 2>%t1
+; RUN: FileCheck %s <%t0
+; RUN: FileCheck %s <%t1
+; RUN: not diff %t0 %t1
+
+; CHECK: Generated ModuleID:
+target triple="x86_64-pc-linux-gnu"
+
[e-mail] = appending global [1 x i8*] [i8* bitcast (void ()* @a to i8*)], !type !0
+
+declare i1 @llvm.type.test(i8*, metadata)
+declare void @llvm.assume(i1)
+
+define internal void @a() {
+  %1 = call i1 @llvm.type.test(i8* undef, metadata !"a")
+  call void @llvm.assume(i1 %1)
+  ret void
+}
+
+
+!0 = !{i64 16, !"a"}

llvm/test/ThinLTO/X86/cfi-unsat.ll

 ; REQUIRES: x86-registered-target
 
 ; Test CFI devirtualization through the thin link and backend when
 ; a type id is Unsat (not used on any global's type metadata).
 ;
 ; In this test case, the first module is split and will import a resolution
 ; for its type test. The resolution would be exported by the second
 ; module, which is set up so that it does not get split (treated as regular
 ; LTO because it does not have any external globals from which to create
 ; a unique module ID). We should not actually get any resolution for the
 ; type id in this case, since no globals include it in their type metadata,
 ; so the resolution is Unsat and the type.checked.load instructions are
 ; converted to type tests that evaluate to false.
 
-; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
-; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t1.o %p/Inputs/cfi-unsat.ll
+; RUN: opt -thinlto-bc -empty-moduleid -thinlto-split-lto-unit -o %t.o %s
+; RUN: opt -thinlto-bc -empty-moduleid -thinlto-split-lto-unit -o %t1.o %p/Inputs/cfi-unsat.ll
 
 ; RUN: llvm-lto2 run %t.o %t1.o -save-temps -pass-remarks=. \
 ; RUN:   -whole-program-visibility \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t.o,test2,px \
 ; RUN:   -r=%t1.o,_ZTV1B,px \
 ; RUN:   -r=%t1.o,test,px \
 ; RUN:   -r=%t1.o,testb,px

llvm/test/ThinLTO/X86/nodevirt-nonpromoted-typeid.ll

 ; REQUIRES: x86-registered-target
 
 ; Test that index-only devirtualization handles and ignores any
 ; type metadata that could not be summarized (because it was internal
 ; and could not be promoted due to the fact that the module has
 ; no external symbols and therefore could not be assigned a unique
 ; identifier). In this case we should simply not get the type
 ; metadata summary entries, and no promotion will occur.
 
 ; Generate unsplit module with summary for ThinLTO index-based WPD.
-; RUN: opt -thinlto-bc -thinlto-split-lto-unit=false -o %t2.o %s
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit=false -empty-moduleid -o %t2.o %s
 
 ; Check that we don't have module flag when splitting not enabled for ThinLTO,
 ; and that we generate summary information needed for index-based WPD.
 ; RUN: llvm-dis -o - %t2.o | FileCheck %s --check-prefix=DIS
 ; DIS-NOT: typeIdInfo
 ; DIS-NOT: typeidMetadata
 
 ; Index based WPD

llvm/test/Transforms/ThinLTOBitcodeWriter/unsplittable.ll

-; RUN: opt -thinlto-bc -thin-link-bitcode-file=%t2 -thinlto-split-lto-unit -o %t %s
+; RUN: opt -thinlto-bc -thin-link-bitcode-file=%t2 -thinlto-split-lto-unit -empty-moduleid -o %t %s
 ; RUN: llvm-dis -o - %t | FileCheck %s
 ; RUN: llvm-bcanalyzer -dump %t | FileCheck --check-prefix=BCA %s
 ; When not splitting the module, the thin link bitcode file should simply be a
 ; copy of the regular module.
 ; RUN: diff %t %t2
 
 ; BCA: <FULL_LTO_GLOBALVAL_SUMMARY_BLOCK
 ; BCA-NOT: <GLOBALVAL_SUMMARY_BLOCK

llvm/test/Transforms/Util/generate-moduleid.ll

+; REQUIRES: asserts
+; RUN: opt --thinlto-split-lto-unit=1 --thinlto-bc -o %t0 -debug-only=moduleutils <%s 2>&1 | FileCheck %s
+; CHECK: Generated ModuleID
+
+target triple="x86_64-pc-linux-gnu"
+
+declare i1 @llvm.type.test(i8*, metadata)
+
+define internal void @foo() !type !1{
+entry:
+  %0 = tail call i1 @llvm.type.test(i8* null, metadata !"")
+  ret void
+}
+
[e-mail]_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 103, void ()* @foo, i8* null }]
+
+!1 = !{i64 0, !"foo"}