[CFI][ThinLTO] Remove the need for CFI calculating ThinLTO GUIDs (#201370)

CFI does name-based matching. ThinLTO uses a hash over the function name
(the "GUID"). As a result of this
[RFC](https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801)
- see also PR #184065 - GUID calculation should be treated as an
implementation detail, i.e. passes shouldn't need to re-do / reverse
engineer GUIDs.

The main reasons CFI is aware of GUIDs is because (1) the CFI functions
need to be communicated to ThinLink, as they need to be treated as
exports, in `LTO::runThinLTO`; and (2) because CFI needs to check the
liveliness of functions referenced in `cfi.functions` metadata, and this
check happens via the thinlto export summary
(`LowerTypeTestsModule::lower`).

To a lesser extent, the optimization in PR #130382 benefits from CFI
knowing about GUIDs; however, if this were the only reason, we could
make `ValueInfo`s available at that point, which carry names, and
perform name-based matching for CFI's needs.

This PR lets GUIDs be passed to CFI. The bulk of the change is moving
them around as metadata fields. The GUID calculation is hoisted out, but
kept the same as before this patch, following that once PR #184065 is
relanded it will be switched to the stable mechanism it (PR #184065)
introduces. The compile time effects are
[here](https://llvm-compile-time-tracker.com/compare.php?from=b29bf9fa25bdb906a61ec361fba68796020cc6b9&to=7bb1c07144e8ccb8a3332e91fc193303acd9439e&stat=instructions:u).

Author: mtrofin

llvm/include/llvm/IR/AutoUpgrade.h

@@ -63,6 +63,10 @@ namespace llvm {
   /// module is modified.
   LLVM_ABI bool UpgradeModuleFlags(Module &M);
 
+  /// Upgrade the cfi.functions metadata node by calculating and inserting
+  /// the GUID for each function entry if it's missing.
+  LLVM_ABI bool UpgradeCFIFunctionsMetadata(Module &M);
+
   /// Convert legacy nvvm.annotations metadata to appropriate function
   /// attributes.
   LLVM_ABI void UpgradeNVVMAnnotations(Module &M);

llvm/include/llvm/IR/ModuleSummaryIndex.h

@@ -1573,7 +1573,7 @@ class ModuleSummaryIndex {
   // in the way some record are interpreted, like flags for instance.
   // Note that incrementing this may require changes in both BitcodeReader.cpp
   // and BitcodeWriter.cpp.
-  static constexpr uint64_t BitcodeSummaryVersion = 13;
+  static constexpr uint64_t BitcodeSummaryVersion = 14;
 
   // Regular LTO module name for ASM writer
   static constexpr const char *getRegularLTOModuleName() {

llvm/lib/AsmParser/LLParser.cpp

@@ -476,6 +476,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
     llvm::UpgradeDebugInfo(*M);
 
   UpgradeModuleFlags(*M);
+  UpgradeCFIFunctionsMetadata(*M);
   UpgradeNVVMAnnotations(*M);
   UpgradeSectionAttributes(*M);
   copyModuleAttrToFunctions(*M);

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

@@ -3979,6 +3979,8 @@ Error BitcodeReader::materializeMetadata() {
     }
   }
 
+  UpgradeCFIFunctionsMetadata(*TheModule);
+
   DeferredMetadataInfo.clear();
   return Error::success();
 }
@@ -8149,24 +8151,42 @@ Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
 
     case bitc::FS_CFI_FUNCTION_DEFS: {
       auto &CfiFunctionDefs = TheIndex.cfiFunctionDefs();
-      for (unsigned I = 0; I != Record.size(); I += 2) {
-        StringRef Name(Strtab.data() + Record[I],
-                       static_cast<size_t>(Record[I + 1]));
-        GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
-            GlobalValue::dropLLVMManglingEscape(Name));
-        CfiFunctionDefs.addSymbolWithThinLTOGUID(Name, GUID);
+      if (Version < 14) {
+        for (unsigned I = 0; I != Record.size(); I += 2) {
+          StringRef Name(Strtab.data() + Record[I],
+                         static_cast<size_t>(Record[I + 1]));
+          GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+              GlobalValue::dropLLVMManglingEscape(Name));
+          CfiFunctionDefs.addSymbolWithThinLTOGUID(Name, GUID);
+        }
+      } else {
+        for (unsigned I = 0; I != Record.size(); I += 3) {
+          GlobalValue::GUID ThinLTOGUID = Record[I];
+          StringRef Name(Strtab.data() + Record[I + 1],
+                         static_cast<size_t>(Record[I + 2]));
+          CfiFunctionDefs.addSymbolWithThinLTOGUID(Name, ThinLTOGUID);
+        }
       }
       break;
     }
 
     case bitc::FS_CFI_FUNCTION_DECLS: {
       auto &CfiFunctionDecls = TheIndex.cfiFunctionDecls();
-      for (unsigned I = 0; I != Record.size(); I += 2) {
-        StringRef Name(Strtab.data() + Record[I],
-                       static_cast<size_t>(Record[I + 1]));
-        GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
-            GlobalValue::dropLLVMManglingEscape(Name));
-        CfiFunctionDecls.addSymbolWithThinLTOGUID(Name, GUID);
+      if (Version < 14) {
+        for (unsigned I = 0; I != Record.size(); I += 2) {
+          StringRef Name(Strtab.data() + Record[I],
+                         static_cast<size_t>(Record[I + 1]));
+          GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+              GlobalValue::dropLLVMManglingEscape(Name));
+          CfiFunctionDecls.addSymbolWithThinLTOGUID(Name, GUID);
+        }
+      } else {
+        for (unsigned I = 0; I != Record.size(); I += 3) {
+          GlobalValue::GUID ThinLTOGUID = Record[I];
+          StringRef Name(Strtab.data() + Record[I + 1],
+                         static_cast<size_t>(Record[I + 2]));
+          CfiFunctionDecls.addSymbolWithThinLTOGUID(Name, ThinLTOGUID);
+        }
       }
       break;
     }

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -5323,21 +5323,23 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
         getReferencedTypeIds(FS, ReferencedTypeIds);
   }
 
-  SmallVector<StringRef, 4> Functions;
+  SmallVector<std::pair<StringRef, GlobalValue::GUID>, 4> Functions;
   auto EmitCfiFunctions = [&](const CfiFunctionIndex &CfiIndex,
                               bitc::GlobalValueSummarySymtabCodes Code) {
     if (CfiIndex.empty())
       return;
     for (GlobalValue::GUID GUID : DefOrUseGUIDs) {
       auto Names = CfiIndex.getNamesForGUID(GUID);
-      llvm::append_range(Functions, Names);
+      for (StringRef Name : Names)
+        Functions.push_back({Name, GUID});
     }
     if (Functions.empty())
       return;
     llvm::sort(Functions);
-    for (const auto &S : Functions) {
-      NameVals.push_back(StrtabBuilder.add(S));
-      NameVals.push_back(S.size());
+    for (const auto &Record : Functions) {
+      NameVals.push_back(Record.second);
+      NameVals.push_back(StrtabBuilder.add(Record.first));
+      NameVals.push_back(Record.first.size());
     }
     Stream.EmitRecord(Code, NameVals);
     NameVals.clear();

llvm/lib/IR/AutoUpgrade.cpp

@@ -26,6 +26,7 @@
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/Instruction.h"
@@ -6359,6 +6360,50 @@ bool llvm::UpgradeModuleFlags(Module &M) {
   return Changed;
 }
 
+bool llvm::UpgradeCFIFunctionsMetadata(Module &M) {
+  NamedMDNode *CFIConsts = M.getNamedMetadata("cfi.functions");
+  // If this metadata has operands, we expect all of them to be either from
+  // before or from after the format change handled here, so we can bail out
+  // fast if the first (if any) operands is of the new format.
+  auto MatchesVersion = [](const MDNode *Op) {
+    return Op->getNumOperands() >= 3 &&
+           isa<ConstantAsMetadata>(Op->getOperand(2)) &&
+           cast<ConstantAsMetadata>(Op->getOperand(2))
+               ->getType()
+               ->isIntegerTy(64);
+  };
+
+  if (!CFIConsts || !CFIConsts->getNumOperands() ||
+      MatchesVersion(CFIConsts->getOperand(0)))
+    return false;
+
+  bool Changed = false;
+  for (unsigned I = 0, E = CFIConsts->getNumOperands(); I != E; ++I) {
+    MDNode *Op = CFIConsts->getOperand(I);
+    assert(!MatchesVersion(Op) && "Unexpected mix of CFIConstant formats");
+    assert(Op->getNumOperands() >= 2 &&
+           "Expected at least 2 operands - name and linkage type");
+    MDString *NameMD = dyn_cast<MDString>(Op->getOperand(0));
+    StringRef Name = NameMD->getString();
+    GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+        GlobalValue::dropLLVMManglingEscape(Name));
+
+    SmallVector<Metadata *, 4> Elts;
+    Elts.push_back(Op->getOperand(0));
+    Elts.push_back(Op->getOperand(1));
+    Elts.push_back(ConstantAsMetadata::get(
+        ConstantInt::get(Type::getInt64Ty(M.getContext()), GUID)));
+
+    for (unsigned J = 2, EJ = Op->getNumOperands(); J != EJ; ++J)
+      Elts.push_back(Op->getOperand(J));
+
+    CFIConsts->setOperand(I, MDNode::get(M.getContext(), Elts));
+    Changed = true;
+  }
+
+  return Changed;
+}
+
 void llvm::UpgradeSectionAttributes(Module &M) {
   auto TrimSpaces = [](StringRef Section) -> std::string {
     SmallVector<StringRef, 5> Components;

llvm/lib/Transforms/IPO/CrossDSOCFI.cpp

@@ -73,8 +73,9 @@ void CrossDSOCFI::buildCFICheck(Module &M) {
   NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions");
   if (CfiFunctionsMD) {
     for (auto *Func : CfiFunctionsMD->operands()) {
-      assert(Func->getNumOperands() >= 2);
-      for (unsigned I = 2; I < Func->getNumOperands(); ++I)
+      assert(Func->getNumOperands() >= 3);
+      assert(isa<ConstantAsMetadata>(Func->getOperand(2)));
+      for (unsigned I = 3; I < Func->getNumOperands(); ++I)
         if (ConstantInt *TypeId =
                 extractNumericTypeId(cast<MDNode>(Func->getOperand(I).get())))
           TypeIds.insert(TypeId->getZExtValue());

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

@@ -2213,8 +2213,10 @@ bool LowerTypeTestsModule::lower() {
                 ->getUniqueInteger()
                 .getZExtValue());
         const GlobalValue::GUID GUID =
-            GlobalValue::getGUIDAssumingExternalLinkage(
-                GlobalValue::dropLLVMManglingEscape(FunctionName));
+            cast<ConstantAsMetadata>(FuncMD->getOperand(2))
+                ->getValue()
+                ->getUniqueInteger()
+                .getZExtValue();
         // Do not emit jumptable entries for functions that are not-live and
         // have no live references (and are not exported with cross-DSO CFI.)
         if (!ExportSummary->isGUIDLive(GUID))
@@ -2285,7 +2287,7 @@ bool LowerTypeTestsModule::lower() {
             F->setLinkage(GlobalValue::ExternalWeakLinkage);
 
           F->eraseMetadata(LLVMContext::MD_type);
-          for (unsigned I = 2; I < FuncMD->getNumOperands(); ++I)
+          for (unsigned I = 3; I < FuncMD->getNumOperands(); ++I)
             F->addMetadata(LLVMContext::MD_type,
                            *cast<MDNode>(FuncMD->getOperand(I).get()));
         }

llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

@@ -436,6 +436,11 @@ void splitAndWriteThinLTOBitcode(
       Linkage = CFL_Declaration;
     Elts.push_back(ConstantAsMetadata::get(
         llvm::ConstantInt::get(Type::getInt8Ty(Ctx), Linkage)));
+    // TODO: use F->getGUID() once #184065 is relanded.
+    GlobalValue::GUID GUID = GlobalValue::getGUIDAssumingExternalLinkage(
+        GlobalValue::dropLLVMManglingEscape(V->getName()));
+    Elts.push_back(ConstantAsMetadata::get(
+        llvm::ConstantInt::get(Type::getInt64Ty(Ctx), GUID)));
     append_range(Elts, Types);
     CfiFunctionMDs.push_back(MDTuple::get(Ctx, Elts));
   }