[llvm] r304323 - [ThinLTO] Reduce unnecessary map lookups during combined summary write

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Wed May 31 11:58:11 PDT 2017


Author: tejohnson
Date: Wed May 31 13:58:11 2017
New Revision: 304323

URL: http://llvm.org/viewvc/llvm-project?rev=304323&view=rev
Log:
[ThinLTO] Reduce unnecessary map lookups during combined summary write

Summary:
Don't assign values to undefined references, simply don't emit those
reference edges as they are not useful (we were already not emitting
call edges to undefined refs).

Also, streamline the later lookup of value ids when writing the
summaries, by combining the check for value id existence with the access
of that value id.

Reviewers: pcc

Subscribers: Prazek, llvm-commits, inglorion

Differential Revision: https://reviews.llvm.org/D33634

Modified:
    llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
    llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph.ll

Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=304323&r1=304322&r2=304323&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Wed May 31 13:58:11 2017
@@ -378,18 +378,10 @@ private:
            ModuleToSummariesForIndex->count(ModulePath);
   }
 
-  bool hasValueId(GlobalValue::GUID ValGUID) {
-    const auto &VMI = GUIDToValueIdMap.find(ValGUID);
-    return VMI != GUIDToValueIdMap.end();
-  }
-  void assignValueId(GlobalValue::GUID ValGUID) {
-    unsigned &ValueId = GUIDToValueIdMap[ValGUID];
-    if (ValueId == 0)
-      ValueId = ++GlobalValueId;
-  }
-  unsigned getValueId(GlobalValue::GUID ValGUID) {
+  Optional<unsigned> getValueId(GlobalValue::GUID ValGUID) {
     auto VMI = GUIDToValueIdMap.find(ValGUID);
-    assert(VMI != GUIDToValueIdMap.end());
+    if (VMI == GUIDToValueIdMap.end())
+      return None;
     return VMI->second;
   }
   std::map<GlobalValue::GUID, unsigned> &valueIds() { return GUIDToValueIdMap; }
@@ -3413,12 +3405,6 @@ void IndexBitcodeWriter::writeCombinedGl
   Stream.EnterSubblock(bitc::GLOBALVAL_SUMMARY_BLOCK_ID, 3);
   Stream.EmitRecord(bitc::FS_VERSION, ArrayRef<uint64_t>{INDEX_VERSION});
 
-  // Create value IDs for undefined references.
-  forEachSummary([&](GVInfo I) {
-    for (auto &RI : I.second->refs())
-      assignValueId(RI.getGUID());
-  });
-
   for (const auto &GVI : valueIds()) {
     Stream.EmitRecord(bitc::FS_VALUE_GUID,
                       ArrayRef<uint64_t>{GVI.second, GVI.first});
@@ -3492,9 +3478,9 @@ void IndexBitcodeWriter::writeCombinedGl
     GlobalValueSummary *S = I.second;
     assert(S);
 
-    assert(hasValueId(I.first));
-    unsigned ValueId = getValueId(I.first);
-    SummaryToValueIdMap[S] = ValueId;
+    auto ValueId = getValueId(I.first);
+    assert(ValueId);
+    SummaryToValueIdMap[S] = *ValueId;
 
     if (auto *AS = dyn_cast<AliasSummary>(S)) {
       // Will process aliases as a post-pass because the reader wants all
@@ -3504,11 +3490,14 @@ void IndexBitcodeWriter::writeCombinedGl
     }
 
     if (auto *VS = dyn_cast<GlobalVarSummary>(S)) {
-      NameVals.push_back(ValueId);
+      NameVals.push_back(*ValueId);
       NameVals.push_back(Index.getModuleId(VS->modulePath()));
       NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
       for (auto &RI : VS->refs()) {
-        NameVals.push_back(getValueId(RI.getGUID()));
+        auto RefValueId = getValueId(RI.getGUID());
+        if (!RefValueId)
+          continue;
+        NameVals.push_back(*RefValueId);
       }
 
       // Emit the finished record.
@@ -3522,15 +3511,22 @@ void IndexBitcodeWriter::writeCombinedGl
     auto *FS = cast<FunctionSummary>(S);
     writeFunctionTypeMetadataRecords(Stream, FS);
 
-    NameVals.push_back(ValueId);
+    NameVals.push_back(*ValueId);
     NameVals.push_back(Index.getModuleId(FS->modulePath()));
     NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
     NameVals.push_back(FS->instCount());
-    NameVals.push_back(FS->refs().size());
+    // Fill in below
+    NameVals.push_back(0);
 
+    unsigned Count = 0;
     for (auto &RI : FS->refs()) {
-      NameVals.push_back(getValueId(RI.getGUID()));
+      auto RefValueId = getValueId(RI.getGUID());
+      if (!RefValueId)
+        continue;
+      NameVals.push_back(*RefValueId);
+      Count++;
     }
+    NameVals[4] = Count;
 
     bool HasProfileData = false;
     for (auto &EI : FS->calls()) {
@@ -3543,15 +3539,19 @@ void IndexBitcodeWriter::writeCombinedGl
       // If this GUID doesn't have a value id, it doesn't have a function
       // summary and we don't need to record any calls to it.
       GlobalValue::GUID GUID = EI.first.getGUID();
-      if (!hasValueId(GUID)) {
+      auto CallValueId = getValueId(GUID);
+      if (!CallValueId) {
         // For SamplePGO, the indirect call targets for local functions will
         // have its original name annotated in profile. We try to find the
         // corresponding PGOFuncName as the GUID.
         GUID = Index.getGUIDFromOriginalID(GUID);
-        if (GUID == 0 || !hasValueId(GUID))
+        if (GUID == 0)
+          continue;
+        CallValueId = getValueId(GUID);
+        if (!CallValueId)
           continue;
       }
-      NameVals.push_back(getValueId(GUID));
+      NameVals.push_back(*CallValueId);
       if (HasProfileData)
         NameVals.push_back(static_cast<uint8_t>(EI.second.Hotness));
     }

Modified: llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph.ll?rev=304323&r1=304322&r2=304323&view=diff
==============================================================================
--- llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph.ll (original)
+++ llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph.ll Wed May 31 13:58:11 2017
@@ -11,20 +11,23 @@
 ; RUN: llvm-lto -thinlto-index-stats %p/Inputs/thinlto-function-summary-callgraph-combined.1.bc  | FileCheck %s --check-prefix=OLD-COMBINED
 
 ; CHECK: <SOURCE_FILENAME
+; CHECK-NEXT: <GLOBALVAR
 ; CHECK-NEXT: <FUNCTION
 ; "func"
-; CHECK-NEXT: <FUNCTION op0=4 op1=4
+; CHECK-NEXT: <FUNCTION op0=17 op1=4
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; See if the call to func is registered.
-; CHECK-NEXT:    <PERMODULE {{.*}} op4=1/>
+; CHECK-NEXT:    <PERMODULE {{.*}} op3=1
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 ; CHECK: <STRTAB_BLOCK
-; CHECK-NEXT: blob data = 'mainfunc'
+; CHECK-NEXT: blob data = 'undefinedglobmainfunc'
 
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
+; Only 2 VALUE_GUID since reference to undefinedglob should not be included in
+; combined index.
 ; COMBINED-NEXT:    <VALUE_GUID op0=[[FUNCID:[0-9]+]] op1=7289175272376759421/>
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <COMBINED
@@ -40,10 +43,12 @@ target triple = "x86_64-unknown-linux-gn
 define i32 @main() #0 {
 entry:
     call void (...) @func()
-    ret i32 0
+    %u = load i32, i32* @undefinedglob
+    ret i32 %u
 }
 
 declare void @func(...) #1
+ at undefinedglob = external global i32
 
 ; OLD: Index {{.*}} contains 1 nodes (1 functions, 0 alias, 0 globals) and 1 edges (0 refs and 1 calls)
 ; OLD-COMBINED: Index {{.*}} contains 2 nodes (2 functions, 0 alias, 0 globals) and 1 edges (0 refs and 1 calls)




More information about the llvm-commits mailing list