[llvm] r267304 - Store and emit original name in combined index

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 23 16:38:18 PDT 2016


Author: mehdi_amini
Date: Sat Apr 23 18:38:17 2016
New Revision: 267304

URL: http://llvm.org/viewvc/llvm-project?rev=267304&view=rev
Log:
Store and emit original name in combined index

Summary:
As discussed in D18298, some local globals can't
be renamed/promoted (because they have a section, or because
they are referenced from inline assembly).
To be able to detect naming collision, we need to keep around
the "GUID" using their original name without taking the linkage
into account.

Reviewers: tejohnson

Subscribers: joker.eph, llvm-commits

Differential Revision: http://reviews.llvm.org/D19454

From: Mehdi Amini <mehdi.amini at apple.com>

Added:
    llvm/trunk/test/Bitcode/thinlto-function-summary-originalnames.ll
Modified:
    llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h
    llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
    llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
    llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

Modified: llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h?rev=267304&r1=267303&r2=267304&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h (original)
+++ llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h Sat Apr 23 18:38:17 2016
@@ -217,6 +217,8 @@ enum GlobalValueSummarySymtabCodes {
   FS_ALIAS = 7,
   // COMBINED_ALIAS: [modid, linkage, offset]
   FS_COMBINED_ALIAS = 8,
+  // COMBINED_ORIGINAL_NAME: [original_name_hash]
+  FS_COMBINED_ORIGINAL_NAME = 9,
 };
 
 enum MetadataCodes {

Modified: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h?rev=267304&r1=267303&r2=267304&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h (original)
+++ llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h Sat Apr 23 18:38:17 2016
@@ -96,6 +96,11 @@ private:
   /// Kind of summary for use in dyn_cast<> et al.
   SummaryKind Kind;
 
+  /// This is the hash of the name of the symbol in the original file. It is
+  /// identical to the GUID for global symbols, but differs for local since the
+  /// GUID includes the module level id in the hash.
+  GlobalValue::GUID OriginalName;
+
   /// \brief Path of module IR containing value's definition, used to locate
   /// module during importing.
   ///
@@ -128,6 +133,13 @@ protected:
 public:
   virtual ~GlobalValueSummary() = default;
 
+  /// Returns the hash of the original name, it is identical to the GUID for
+  /// externally visible symbols, but not for local ones.
+  GlobalValue::GUID getOriginalName() { return OriginalName; }
+
+  /// Initialize the original name hash in this summary.
+  void setOriginalName(GlobalValue::GUID Name) { OriginalName = Name; }
+
   /// Which kind of summary subclass this is.
   SummaryKind getSummaryKind() const { return Kind; }
 

Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=267304&r1=267303&r2=267304&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Sat Apr 23 18:38:17 2016
@@ -487,7 +487,10 @@ class ModuleSummaryIndexBitcodeReader {
   // call graph edges read from the function summary from referencing
   // callees by their ValueId to using the GUID instead, which is how
   // they are recorded in the summary index being built.
-  DenseMap<unsigned, GlobalValue::GUID> ValueIdToCallGraphGUIDMap;
+  // We save a second GUID which is the same as the first one, but ignoring the
+  // linkage, i.e. for value other than local linkage they are identical.
+  DenseMap<unsigned, std::pair<GlobalValue::GUID, GlobalValue::GUID>>
+      ValueIdToCallGraphGUIDMap;
 
   /// Map to save the association between summary offset in the VST to the
   /// GlobalValueInfo object created when parsing it. Used to access the
@@ -543,7 +546,8 @@ private:
   std::error_code initStream(std::unique_ptr<DataStreamer> Streamer);
   std::error_code initStreamFromBuffer();
   std::error_code initLazyStream(std::unique_ptr<DataStreamer> Streamer);
-  GlobalValue::GUID getGUIDFromValueId(unsigned ValueId);
+  std::pair<GlobalValue::GUID, GlobalValue::GUID>
+  getGUIDFromValueId(unsigned ValueId);
   GlobalValueInfo *getInfoFromSummaryOffset(uint64_t Offset);
 };
 } // end anonymous namespace
@@ -5699,7 +5703,7 @@ void ModuleSummaryIndexBitcodeReader::fr
 
 void ModuleSummaryIndexBitcodeReader::releaseBuffer() { Buffer.release(); }
 
-GlobalValue::GUID
+std::pair<GlobalValue::GUID, GlobalValue::GUID>
 ModuleSummaryIndexBitcodeReader::getGUIDFromValueId(unsigned ValueId) {
   auto VGI = ValueIdToCallGraphGUIDMap.find(ValueId);
   assert(VGI != ValueIdToCallGraphGUIDMap.end());
@@ -5763,13 +5767,19 @@ std::error_code ModuleSummaryIndexBitcod
       auto VLI = ValueIdToLinkageMap.find(ValueID);
       assert(VLI != ValueIdToLinkageMap.end() &&
              "No linkage found for VST entry?");
-      std::string GlobalId = GlobalValue::getGlobalIdentifier(
-          ValueName, VLI->second, SourceFileName);
+      auto Linkage = VLI->second;
+      std::string GlobalId =
+          GlobalValue::getGlobalIdentifier(ValueName, Linkage, SourceFileName);
       auto ValueGUID = GlobalValue::getGUID(GlobalId);
+      auto OriginalNameID = ValueGUID;
+      if (GlobalValue::isLocalLinkage(Linkage))
+        OriginalNameID = GlobalValue::getGUID(ValueName);
       if (PrintSummaryGUIDs)
-        dbgs() << "GUID " << ValueGUID << " is " << ValueName << "\n";
+        dbgs() << "GUID " << ValueGUID << "(" << OriginalNameID << ") is "
+               << ValueName << "\n";
       TheIndex->addGlobalValueInfo(ValueGUID, std::move(GlobalValInfo));
-      ValueIdToCallGraphGUIDMap[ValueID] = ValueGUID;
+      ValueIdToCallGraphGUIDMap[ValueID] =
+          std::make_pair(ValueGUID, OriginalNameID);
       ValueName.clear();
       break;
     }
@@ -5785,13 +5795,19 @@ std::error_code ModuleSummaryIndexBitcod
       auto VLI = ValueIdToLinkageMap.find(ValueID);
       assert(VLI != ValueIdToLinkageMap.end() &&
              "No linkage found for VST entry?");
+      auto Linkage = VLI->second;
       std::string FunctionGlobalId = GlobalValue::getGlobalIdentifier(
           ValueName, VLI->second, SourceFileName);
       auto FunctionGUID = GlobalValue::getGUID(FunctionGlobalId);
+      auto OriginalNameID = FunctionGUID;
+      if (GlobalValue::isLocalLinkage(Linkage))
+        OriginalNameID = GlobalValue::getGUID(ValueName);
       if (PrintSummaryGUIDs)
-        dbgs() << "GUID " << FunctionGUID << " is " << ValueName << "\n";
+        dbgs() << "GUID " << FunctionGUID << "(" << OriginalNameID << ") is "
+               << ValueName << "\n";
       TheIndex->addGlobalValueInfo(FunctionGUID, std::move(FuncInfo));
-      ValueIdToCallGraphGUIDMap[ValueID] = FunctionGUID;
+      ValueIdToCallGraphGUIDMap[ValueID] =
+          std::make_pair(FunctionGUID, OriginalNameID);
 
       ValueName.clear();
       break;
@@ -5805,14 +5821,19 @@ std::error_code ModuleSummaryIndexBitcod
           llvm::make_unique<GlobalValueInfo>(GlobalValSummaryOffset);
       SummaryOffsetToInfoMap[GlobalValSummaryOffset] = GlobalValInfo.get();
       TheIndex->addGlobalValueInfo(GlobalValGUID, std::move(GlobalValInfo));
-      ValueIdToCallGraphGUIDMap[ValueID] = GlobalValGUID;
+      // The "original name", which is the second value of the pair will be
+      // overriden later by a FS_COMBINED_ORIGINAL_NAME in the combined index.
+      ValueIdToCallGraphGUIDMap[ValueID] =
+          std::make_pair(GlobalValGUID, GlobalValGUID);
       break;
     }
     case bitc::VST_CODE_COMBINED_ENTRY: {
       // VST_CODE_COMBINED_ENTRY: [valueid, refguid]
       unsigned ValueID = Record[0];
       GlobalValue::GUID RefGUID = Record[1];
-      ValueIdToCallGraphGUIDMap[ValueID] = RefGUID;
+      // The "original name", which is the second value of the pair will be
+      // overriden later by a FS_COMBINED_ORIGINAL_NAME in the combined index.
+      ValueIdToCallGraphGUIDMap[ValueID] = std::make_pair(RefGUID, RefGUID);
       break;
     }
     }
@@ -5976,7 +5997,9 @@ std::error_code ModuleSummaryIndexBitcod
     return error("Invalid record");
 
   SmallVector<uint64_t, 64> Record;
-
+  // Keep around the last seen summary to be used when we see an optional
+  // "OriginalName" attachement.
+  GlobalValueSummary *LastSeenSummary = nullptr;
   bool Combined = false;
   while (1) {
     BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
@@ -6041,7 +6064,7 @@ std::error_code ModuleSummaryIndexBitcod
              "Record size inconsistent with number of references");
       for (unsigned I = 4, E = CallGraphEdgeStartIndex; I != E; ++I) {
         unsigned RefValueId = Record[I];
-        GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId);
+        GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId).first;
         FS->addRefEdge(RefGUID);
       }
       bool HasProfile = (BitCode == bitc::FS_PERMODULE_PROFILE);
@@ -6050,12 +6073,13 @@ std::error_code ModuleSummaryIndexBitcod
         unsigned CalleeValueId = Record[I];
         unsigned CallsiteCount = Record[++I];
         uint64_t ProfileCount = HasProfile ? Record[++I] : 0;
-        GlobalValue::GUID CalleeGUID = getGUIDFromValueId(CalleeValueId);
+        GlobalValue::GUID CalleeGUID = getGUIDFromValueId(CalleeValueId).first;
         FS->addCallGraphEdge(CalleeGUID,
                              CalleeInfo(CallsiteCount, ProfileCount));
       }
-      GlobalValue::GUID GUID = getGUIDFromValueId(ValueID);
-      auto *Info = TheIndex->getGlobalValueInfo(GUID);
+      auto GUID = getGUIDFromValueId(ValueID);
+      FS->setOriginalName(GUID.second);
+      auto *Info = TheIndex->getGlobalValueInfo(GUID.first);
       assert(!Info->summary() && "Expected a single summary per VST entry");
       Info->setSummary(std::move(FS));
       break;
@@ -6077,14 +6101,15 @@ std::error_code ModuleSummaryIndexBitcod
       AS->setModulePath(
           TheIndex->addModulePath(Buffer->getBufferIdentifier(), 0)->first());
 
-      GlobalValue::GUID AliaseeGUID = getGUIDFromValueId(AliaseeID);
+      GlobalValue::GUID AliaseeGUID = getGUIDFromValueId(AliaseeID).first;
       auto *AliaseeInfo = TheIndex->getGlobalValueInfo(AliaseeGUID);
       if (!AliaseeInfo->summary())
         return error("Alias expects aliasee summary to be parsed");
       AS->setAliasee(AliaseeInfo->summary());
 
-      GlobalValue::GUID GUID = getGUIDFromValueId(ValueID);
-      auto *Info = TheIndex->getGlobalValueInfo(GUID);
+      auto GUID = getGUIDFromValueId(ValueID);
+      AS->setOriginalName(GUID.second);
+      auto *Info = TheIndex->getGlobalValueInfo(GUID.first);
       assert(!Info->summary() && "Expected a single summary per VST entry");
       Info->setSummary(std::move(AS));
       break;
@@ -6099,11 +6124,12 @@ std::error_code ModuleSummaryIndexBitcod
           TheIndex->addModulePath(Buffer->getBufferIdentifier(), 0)->first());
       for (unsigned I = 2, E = Record.size(); I != E; ++I) {
         unsigned RefValueId = Record[I];
-        GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId);
+        GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId).first;
         FS->addRefEdge(RefGUID);
       }
-      GlobalValue::GUID GUID = getGUIDFromValueId(ValueID);
-      auto *Info = TheIndex->getGlobalValueInfo(GUID);
+      auto GUID = getGUIDFromValueId(ValueID);
+      FS->setOriginalName(GUID.second);
+      auto *Info = TheIndex->getGlobalValueInfo(GUID.first);
       assert(!Info->summary() && "Expected a single summary per VST entry");
       Info->setSummary(std::move(FS));
       break;
@@ -6121,6 +6147,7 @@ std::error_code ModuleSummaryIndexBitcod
       unsigned NumRefs = Record[3];
       std::unique_ptr<FunctionSummary> FS = llvm::make_unique<FunctionSummary>(
           getDecodedLinkage(RawLinkage), InstCount);
+      LastSeenSummary = FS.get();
       FS->setModulePath(ModuleIdMap[ModuleId]);
       static int RefListStartIndex = 4;
       int CallGraphEdgeStartIndex = RefListStartIndex + NumRefs;
@@ -6128,7 +6155,7 @@ std::error_code ModuleSummaryIndexBitcod
              "Record size inconsistent with number of references");
       for (unsigned I = 4, E = CallGraphEdgeStartIndex; I != E; ++I) {
         unsigned RefValueId = Record[I];
-        GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId);
+        GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId).first;
         FS->addRefEdge(RefGUID);
       }
       bool HasProfile = (BitCode == bitc::FS_COMBINED_PROFILE);
@@ -6137,7 +6164,7 @@ std::error_code ModuleSummaryIndexBitcod
         unsigned CalleeValueId = Record[I];
         unsigned CallsiteCount = Record[++I];
         uint64_t ProfileCount = HasProfile ? Record[++I] : 0;
-        GlobalValue::GUID CalleeGUID = getGUIDFromValueId(CalleeValueId);
+        GlobalValue::GUID CalleeGUID = getGUIDFromValueId(CalleeValueId).first;
         FS->addCallGraphEdge(CalleeGUID,
                              CalleeInfo(CallsiteCount, ProfileCount));
       }
@@ -6156,6 +6183,7 @@ std::error_code ModuleSummaryIndexBitcod
       uint64_t AliaseeSummaryOffset = Record[2];
       std::unique_ptr<AliasSummary> AS =
           llvm::make_unique<AliasSummary>(getDecodedLinkage(RawLinkage));
+      LastSeenSummary = AS.get();
       AS->setModulePath(ModuleIdMap[ModuleId]);
 
       auto *AliaseeInfo = getInfoFromSummaryOffset(AliaseeSummaryOffset);
@@ -6175,10 +6203,11 @@ std::error_code ModuleSummaryIndexBitcod
       uint64_t RawLinkage = Record[1];
       std::unique_ptr<GlobalVarSummary> FS =
           llvm::make_unique<GlobalVarSummary>(getDecodedLinkage(RawLinkage));
+      LastSeenSummary = FS.get();
       FS->setModulePath(ModuleIdMap[ModuleId]);
       for (unsigned I = 2, E = Record.size(); I != E; ++I) {
         unsigned RefValueId = Record[I];
-        GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId);
+        GlobalValue::GUID RefGUID = getGUIDFromValueId(RefValueId).first;
         FS->addRefEdge(RefGUID);
       }
       auto *Info = getInfoFromSummaryOffset(CurRecordBit);
@@ -6187,6 +6216,15 @@ std::error_code ModuleSummaryIndexBitcod
       Combined = true;
       break;
     }
+    // FS_COMBINED_ORIGINAL_NAME: [original_name]
+    case bitc::FS_COMBINED_ORIGINAL_NAME: {
+      uint64_t OriginalName = Record[0];
+      if (!LastSeenSummary)
+        return error("Name attachment that does not follow a combined record");
+      LastSeenSummary->setOriginalName(OriginalName);
+      // Reset the LastSeenSummary
+      LastSeenSummary = nullptr;
+    }
     }
   }
   llvm_unreachable("Exit infinite loop");

Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=267304&r1=267303&r2=267304&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Sat Apr 23 18:38:17 2016
@@ -3211,6 +3211,17 @@ void IndexBitcodeWriter::writeCombinedGl
   DenseMap<const GlobalValueSummary *, uint64_t> SummaryToOffsetMap;
 
   SmallVector<uint64_t, 64> NameVals;
+
+  // For local linkage, we also emit the original name separately
+  // immediately after the record.
+  auto MaybeEmitOriginalName = [&](GlobalValueSummary &S) {
+    if (!GlobalValue::isLocalLinkage(S.linkage()))
+      return;
+    NameVals.push_back(S.getOriginalName());
+    Stream.EmitRecord(bitc::FS_COMBINED_ORIGINAL_NAME, NameVals);
+    NameVals.clear();
+  };
+
   for (const auto &FII : Index) {
     for (auto &FI : FII.second) {
       GlobalValueSummary *S = FI->summary();
@@ -3241,6 +3252,7 @@ void IndexBitcodeWriter::writeCombinedGl
         Stream.EmitRecord(bitc::FS_COMBINED_GLOBALVAR_INIT_REFS, NameVals,
                           FSModRefsAbbrev);
         NameVals.clear();
+        MaybeEmitOriginalName(*S);
         continue;
       }
 
@@ -3288,6 +3300,7 @@ void IndexBitcodeWriter::writeCombinedGl
       // Emit the finished record.
       Stream.EmitRecord(Code, NameVals, FSAbbrev);
       NameVals.clear();
+      MaybeEmitOriginalName(*S);
     }
   }
 
@@ -3307,6 +3320,7 @@ void IndexBitcodeWriter::writeCombinedGl
     // Emit the finished record.
     Stream.EmitRecord(bitc::FS_COMBINED_ALIAS, NameVals, FSAliasAbbrev);
     NameVals.clear();
+    MaybeEmitOriginalName(*AS);
   }
 
   Stream.ExitBlock();

Added: llvm/trunk/test/Bitcode/thinlto-function-summary-originalnames.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/thinlto-function-summary-originalnames.ll?rev=267304&view=auto
==============================================================================
--- llvm/trunk/test/Bitcode/thinlto-function-summary-originalnames.ll (added)
+++ llvm/trunk/test/Bitcode/thinlto-function-summary-originalnames.ll Sat Apr 23 18:38:17 2016
@@ -0,0 +1,24 @@
+; Test to check the callgraph in summary
+; RUN: opt -module-summary %s -o %t.o
+; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t.o
+; RUN: llvm-bcanalyzer -dump %t.index.bc | FileCheck %s --check-prefix=COMBINED
+
+; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
+; COMBINED-NEXT:    <COMBINED
+; COMBINED-NEXT:    <COMBINED_ORIGINAL_NAME op0=6699318081062747564/>
+; COMBINED-NEXT:    <COMBINED_GLOBALVAR_INIT_REFS
+; COMBINED-NEXT:    <COMBINED_ORIGINAL_NAME op0=-2012135647395072713/>
+; COMBINED-NEXT:    <COMBINED_ALIAS
+; COMBINED-NEXT:    <COMBINED_ORIGINAL_NAME op0=-4170563161550796836/>
+; COMBINED-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
+
+; ModuleID = 'thinlto-function-summary-callgraph.ll'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at bar = internal global i32 0
+ at fooalias = internal alias void (...), bitcast (void ()* @foo to void (...)*)
+
+define internal void @foo() {
+    ret void
+}

Modified: llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp?rev=267304&r1=267303&r2=267304&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp (original)
+++ llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp Sat Apr 23 18:38:17 2016
@@ -308,6 +308,7 @@ static const char *GetCodeName(unsigned
       STRINGIFY_CODE(FS, COMBINED_GLOBALVAR_INIT_REFS)
       STRINGIFY_CODE(FS, ALIAS)
       STRINGIFY_CODE(FS, COMBINED_ALIAS)
+      STRINGIFY_CODE(FS, COMBINED_ORIGINAL_NAME)
     }
   case bitc::METADATA_ATTACHMENT_ID:
     switch(CodeID) {




More information about the llvm-commits mailing list