[ThinLTO] Restructure AliasSummary to contain ValueInfo of Aliasee

[tejohnson]

Summary

The AliasSummary previously contained the AliaseeGUID, which was only
populated when reading the summary from bitcode. This patch changes it
to instead hold the ValueInfo of the aliasee, and always populates it.
This enables more efficient access to the ValueInfo (specifically in the
recent patch r352438 which needed to perform an index hash table lookup
using the aliasee GUID).

As noted in the comments in AliasSummary, we no longer technically need
to keep a pointer to the corresponding aliasee summary, since it could
be obtained by walking the list of summaries on the ValueInfo looking
for the summary in the same module. However, I am concerned that this
would be inefficient when walking through the index during the thin
link for various analyses. That can be reevaluated in the future.

By always populating this new field, we can remove the guard and special
handling for a 0 aliasee GUID when dumping the dot graph of the summary.

An additional improvement in this patch is when reading the summaries
from LLVM assembly we now set the AliaseeSummary field to the aliasee
summary in that same module, which makes it consistent with the behavior
when reading the summary from bitcode.

[tejohnson]

ping

[tejohnson]

ping

[evgeny777]

This should be rebased against r354850. Otherwise LGTM.

[tejohnson]

Rebase

include/llvm/IR/ModuleSummaryIndex.h

public:
   GlobalValueSummary *getBaseObject();
   const GlobalValueSummary *getBaseObject() const;
 
   friend class ModuleSummaryIndex;
 };
 
 /// Alias summary information.
 class AliasSummary : public GlobalValueSummary {
+  ValueInfo AliaseeValueInfo;
+
+  /// This is the Aliasee in the same module as alias (could get from VI, trades
+  /// memory for time). Note that this pointer may be null (and the value info
+  /// empty) when we have a distributed index where the alias is being imported
+  /// (as a copy of the aliasee), but the aliasee is not.
   GlobalValueSummary *AliaseeSummary;
-  // AliaseeGUID is only set and accessed when we are building a combined index
-  // via the BitcodeReader.
-  GlobalValue::GUID AliaseeGUID;
 
 public:
   AliasSummary(GVFlags Flags)
       : GlobalValueSummary(AliasKind, Flags, ArrayRef<ValueInfo>{}),
-        AliaseeSummary(nullptr), AliaseeGUID(0) {}
+        AliaseeSummary(nullptr) {}
 
   /// Check if this is an alias summary.
   static bool classof(const GlobalValueSummary *GVS) {
     return GVS->getSummaryKind() == AliasKind;
   }
 
-  void setAliasee(GlobalValueSummary *Aliasee) { AliaseeSummary = Aliasee; }
-  void setAliaseeGUID(GlobalValue::GUID GUID) { AliaseeGUID = GUID; }
+  void setAliasee(ValueInfo &AliaseeVI, GlobalValueSummary *Aliasee) {
+    AliaseeValueInfo = AliaseeVI;
+    AliaseeSummary = Aliasee;
+  }
 
-  bool hasAliasee() const { return !!AliaseeSummary; }
+  bool hasAliasee() const {
+    assert(!!AliaseeSummary == (AliaseeValueInfo &&
+                                !AliaseeValueInfo.getSummaryList().empty()) &&
+           "Expect to have both aliasee summary and summary list or neither");
+    return !!AliaseeSummary;
+  }
 
   const GlobalValueSummary &getAliasee() const {
     assert(AliaseeSummary && "Unexpected missing aliasee summary");
     return *AliaseeSummary;
   }
 
   GlobalValueSummary &getAliasee() {
     return const_cast<GlobalValueSummary &>(
                          static_cast<const AliasSummary *>(this)->getAliasee());
   }
-  bool hasAliaseeGUID() const { return AliaseeGUID != 0; }
-  const GlobalValue::GUID &getAliaseeGUID() const {
-    assert(AliaseeGUID && "Unexpected missing aliasee GUID");
-    return AliaseeGUID;
+  ValueInfo getAliaseeVI() const {
+    assert(AliaseeValueInfo && "Unexpected missing aliasee");
+    return AliaseeValueInfo;
+  }
+  GlobalValue::GUID getAliaseeGUID() const {
+    assert(AliaseeValueInfo && "Unexpected missing aliasee");
+    return AliaseeValueInfo.getGUID();
   }
 };
 
 const inline GlobalValueSummary *GlobalValueSummary::getBaseObject() const {
   if (auto *AS = dyn_cast<AliasSummary>(this))
     return &AS->getAliasee();
   return this;
 }
void addOriginalName(GlobalValue::GUID ValueGUID,
     if (OrigGUID == 0 || ValueGUID == OrigGUID)
       return;
     if (OidGuidMap.count(OrigGUID) && OidGuidMap[OrigGUID] != ValueGUID)
       OidGuidMap[OrigGUID] = 0;
     else
       OidGuidMap[OrigGUID] = ValueGUID;
   }
 
-  /// Find the summary for global \p GUID in module \p ModuleId, or nullptr if
+  /// Find the summary for ValueInfo \p VI in module \p ModuleId, or nullptr if
   /// not found.
-  GlobalValueSummary *findSummaryInModule(GlobalValue::GUID ValueGUID,
-                                          StringRef ModuleId) const {
-    auto CalleeInfo = getValueInfo(ValueGUID);
-    if (!CalleeInfo) {
-      return nullptr; // This function does not have a summary
-    }
+  GlobalValueSummary *findSummaryInModule(ValueInfo VI, StringRef ModuleId) const {
+    auto SummaryList = VI.getSummaryList();
     auto Summary =
-        llvm::find_if(CalleeInfo.getSummaryList(),
+        llvm::find_if(SummaryList,
                       [&](const std::unique_ptr<GlobalValueSummary> &Summary) {
                         return Summary->modulePath() == ModuleId;
                       });
-    if (Summary == CalleeInfo.getSummaryList().end())
+    if (Summary == SummaryList.end())
       return nullptr;
     return Summary->get();
   }
 
+  /// Find the summary for global \p GUID in module \p ModuleId, or nullptr if
+  /// not found.
+  GlobalValueSummary *findSummaryInModule(GlobalValue::GUID ValueGUID,
+                                          StringRef ModuleId) const {
+    auto CalleeInfo = getValueInfo(ValueGUID);
+    if (!CalleeInfo)
+      return nullptr; // This function does not have a summary
+    return findSummaryInModule(CalleeInfo, ModuleId);
+  }
+
   /// Returns the first GlobalValueSummary for \p GV, asserting that there
   /// is only one if \p PerModuleIndex.
   GlobalValueSummary *getGlobalValueSummary(const GlobalValue &GV,
                                             bool PerModuleIndex = true) const {
     assert(GV.hasName() && "Can't get GlobalValueSummary for GV with no name");
     return getGlobalValueSummary(GV.getGUID(), PerModuleIndex);
   }
 

lib/Analysis/ModuleSummaryAnalysis.cpp

 static void
 computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
                     DenseSet<GlobalValue::GUID> &CantBePromoted) {
   bool NonRenamableLocal = isNonRenamableLocal(A);
   GlobalValueSummary::GVFlags Flags(A.getLinkage(), NonRenamableLocal,
                                     /* Live = */ false, A.isDSOLocal());
   auto AS = llvm::make_unique<AliasSummary>(Flags);
   auto *Aliasee = A.getBaseObject();
-  auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee);
-  assert(AliaseeSummary && "Alias expects aliasee summary to be parsed");
-  AS->setAliasee(AliaseeSummary);
+  auto AliaseeVI = Index.getValueInfo(Aliasee->getGUID());
+  assert(AliaseeVI && "Alias expects aliasee summary to be available");
+  assert(AliaseeVI.getSummaryList().size() == 1 &&
+         "Expected a single entry per aliasee in per-module index");
+  AS->setAliasee(AliaseeVI, AliaseeVI.getSummaryList()[0].get());
   if (NonRenamableLocal)
     CantBePromoted.insert(A.getGUID());
   Index.addGlobalValueSummary(A, std::move(AS));
 }
 
 // Set LiveRoot flag on entries matching the given value name.
 static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
   if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))

lib/AsmParser/LLParser.cpp

if (M) {
           (!GlobalValue::isLocalLinkage(Linkage) || !SourceFileName.empty()) &&
           "Need a source_filename to compute GUID for local");
       GUID = GlobalValue::getGUID(
           GlobalValue::getGlobalIdentifier(Name, Linkage, SourceFileName));
       VI = Index->getOrInsertValueInfo(GUID, Index->saveString(Name));
     }
   }
 
-  // Add the summary if one was provided.
-  if (Summary)
-    Index->addGlobalValueSummary(VI, std::move(Summary));
-
   // Resolve forward references from calls/refs
   auto FwdRefVIs = ForwardRefValueInfos.find(ID);
   if (FwdRefVIs != ForwardRefValueInfos.end()) {
     for (auto VIRef : FwdRefVIs->second) {
       assert(VIRef.first->getRef() == FwdVIRef &&
              "Forward referenced ValueInfo expected to be empty");
       resolveFwdRef(VIRef.first, VI);
     }
     ForwardRefValueInfos.erase(FwdRefVIs);
   }
 
   // Resolve forward references from aliases
   auto FwdRefAliasees = ForwardRefAliasees.find(ID);
   if (FwdRefAliasees != ForwardRefAliasees.end()) {
     for (auto AliaseeRef : FwdRefAliasees->second) {
       assert(!AliaseeRef.first->hasAliasee() &&
              "Forward referencing alias already has aliasee");
-      AliaseeRef.first->setAliasee(VI.getSummaryList().front().get());
+      assert(Summary && "Aliasee must be a definition");
+      AliaseeRef.first->setAliasee(VI, Summary.get());
     }
     ForwardRefAliasees.erase(FwdRefAliasees);
   }
 
+  // Add the summary if one was provided.
+  if (Summary)
+    Index->addGlobalValueSummary(VI, std::move(Summary));
+
   // Save the associated ValueInfo for use in later references by ID.
   if (ID == NumberedValueInfos.size())
     NumberedValueInfos.push_back(VI);
   else {
     // Handle non-continuous numbers (to make test simplification easier).
     if (ID > NumberedValueInfos.size())
       NumberedValueInfos.resize(ID + 1);
     NumberedValueInfos[ID] = VI;
bool LLParser::ParseAliasSummary(std::string Name, GlobalValue::GUID GUID,
 
   AS->setModulePath(ModulePath);
 
   // Record forward reference if the aliasee is not parsed yet.
   if (AliaseeVI.getRef() == FwdVIRef) {
     auto FwdRef = ForwardRefAliasees.insert(
         std::make_pair(GVId, std::vector<std::pair<AliasSummary *, LocTy>>()));
     FwdRef.first->second.push_back(std::make_pair(AS.get(), Loc));
-  } else
-    AS->setAliasee(AliaseeVI.getSummaryList().front().get());
+  } else {
+    auto Summary = Index->findSummaryInModule(AliaseeVI, ModulePath);
+    assert(Summary && "Aliasee must be a definition");
+    AS->setAliasee(AliaseeVI, Summary);
+  }
 
   AddGlobalValueToIndex(Name, GUID, (GlobalValue::LinkageTypes)GVFlags.Linkage,
                         ID, std::move(AS));
 
   return false;
 }
 
 /// Flag

lib/Bitcode/Reader/BitcodeReader.cpp

case bitc::FS_ALIAS: {
       auto AS = llvm::make_unique<AliasSummary>(Flags);
       // The module path string ref set in the summary must be owned by the
       // index's module string table. Since we don't have a module path
       // string table section in the per-module index, we create a single
       // module path string table entry with an empty (0) ID to take
       // ownership.
       AS->setModulePath(getThisModule()->first());
 
-      GlobalValue::GUID AliaseeGUID =
-          getValueInfoFromValueId(AliaseeID).first.getGUID();
-      auto AliaseeInModule =
-          TheIndex.findSummaryInModule(AliaseeGUID, ModulePath);
+      auto AliaseeVI = getValueInfoFromValueId(AliaseeID).first;
+      auto AliaseeInModule = TheIndex.findSummaryInModule(AliaseeVI, ModulePath);
       if (!AliaseeInModule)
         return error("Alias expects aliasee summary to be parsed");
-      AS->setAliasee(AliaseeInModule);
-      AS->setAliaseeGUID(AliaseeGUID);
+      AS->setAliasee(AliaseeVI, AliaseeInModule);
 
       auto GUID = getValueInfoFromValueId(ValueID);
       AS->setOriginalName(GUID.second);
       TheIndex.addGlobalValueSummary(GUID.first, std::move(AS));
       break;
     }
     // FS_PERMODULE_GLOBALVAR_INIT_REFS: [valueid, flags, varflags, n x valueid]
     case bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS: {
case bitc::FS_COMBINED_ALIAS: {
       uint64_t ModuleId = Record[1];
       uint64_t RawFlags = Record[2];
       unsigned AliaseeValueId = Record[3];
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
       auto AS = llvm::make_unique<AliasSummary>(Flags);
       LastSeenSummary = AS.get();
       AS->setModulePath(ModuleIdMap[ModuleId]);
 
-      auto AliaseeGUID =
-          getValueInfoFromValueId(AliaseeValueId).first.getGUID();
-      auto AliaseeInModule =
-          TheIndex.findSummaryInModule(AliaseeGUID, AS->modulePath());
-      AS->setAliasee(AliaseeInModule);
-      AS->setAliaseeGUID(AliaseeGUID);
+      auto AliaseeVI = getValueInfoFromValueId(AliaseeValueId).first;
+      auto AliaseeInModule = TheIndex.findSummaryInModule(AliaseeVI, AS->modulePath());
+      AS->setAliasee(AliaseeVI, AliaseeInModule);
 
       ValueInfo VI = getValueInfoFromValueId(ValueID).first;
       LastSeenGUID = VI.getGUID();
       TheIndex.addGlobalValueSummary(VI, std::move(AS));
       break;
     }
     // FS_COMBINED_GLOBALVAR_INIT_REFS: [valueid, modid, flags, n x valueid]
     case bitc::FS_COMBINED_GLOBALVAR_INIT_REFS: {

lib/IR/ModuleSummaryIndex.cpp

for (auto &ModIt : ModuleToDefinedGVS) {
     OS << "    // Edges:\n";
 
     for (auto &SummaryIt : GVSMap) {
       auto *GVS = SummaryIt.second;
       for (auto &R : GVS->refs())
         Draw(SummaryIt.first, R.getGUID(), R.isReadOnly() ? -1 : -2);
 
       if (auto *AS = dyn_cast_or_null<AliasSummary>(SummaryIt.second)) {
-        GlobalValue::GUID AliaseeId;
-        if (AS->hasAliaseeGUID())
-          AliaseeId = AS->getAliaseeGUID();
-        else {
-          auto AliaseeOrigId = AS->getAliasee().getOriginalName();
-          AliaseeId = getGUIDFromOriginalID(AliaseeOrigId);
-          if (!AliaseeId)
-            AliaseeId = AliaseeOrigId;
-        }
-
-        Draw(SummaryIt.first, AliaseeId, -3);
+        Draw(SummaryIt.first, AS->getAliaseeGUID(), -3);
         continue;
       }
 
       if (auto *FS = dyn_cast_or_null<FunctionSummary>(SummaryIt.second))
         for (auto &CGEdge : FS->calls())
           Draw(SummaryIt.first, CGEdge.first.getGUID(),
                static_cast<int>(CGEdge.second.Hotness));
     }

lib/Transforms/IPO/FunctionImport.cpp

void llvm::computeDeadSymbols(
 
   while (!Worklist.empty()) {
     auto VI = Worklist.pop_back_val();
     for (auto &Summary : VI.getSummaryList()) {
       if (auto *AS = dyn_cast<AliasSummary>(Summary.get())) {
         // If this is an alias, visit the aliasee VI to ensure that all copies
         // are marked live and it is added to the worklist for further
         // processing of its references.
-        // FIXME: The aliasee GUID is only populated in the summary when we
-        // read them from bitcode, which is currently the only way we can
-        // get here (we don't yet support reading the summary index directly
-        // from LLVM assembly code in tools that can perform a thin link).
-        // If that ever changes, the below call to getAliaseGUID will assert.
-        visit(Index.getValueInfo(AS->getAliaseeGUID()));
+        visit(AS->getAliaseeVI());
         continue;
       }
 
       Summary->setLive(true);
       for (auto Ref : Summary->refs())
         visit(Ref);
       if (auto *FS = dyn_cast<FunctionSummary>(Summary.get()))
         for (auto Call : FS->calls())

test/ThinLTO/X86/dot-dumper.ll

 ; PERMODULE-NEXT:  subgraph cluster_0 {
 ; PERMODULE-NEXT:    style = filled;
 ; PERMODULE-NEXT:    color = lightgrey;
 ; PERMODULE-NEXT:    label = "";
 ; PERMODULE-NEXT:    node [style=filled,fillcolor=lightblue];
 ; PERMODULE-NEXT:    M0_[[MAIN_ALIAS:[0-9]+]] [style="dotted,filled",shape="box",label="main_alias",fillcolor="red"]; // alias, dead
 ; PERMODULE-NEXT:    M0_[[MAIN:[0-9]+]] [shape="record",label="main|extern (inst: 4, ffl: 00000)}",fillcolor="red"]; // function, dead
 ; PERMODULE-NEXT:    // Edges:
+; PERMODULE-NEXT:    M0_[[MAIN_ALIAS]] -> M0_[[MAIN]] [style=dotted]; // alias
 ; PERMODULE-NEXT:  }
 ; PERMODULE-NEXT:  // Cross-module edges:
-; PERMODULE-NEXT:  0 [label="@0"]; // defined externally
-; PERMODULE-NEXT:  M0_[[MAIN_ALIAS]] -> 0 [style=dotted]; // alias
 ; PERMODULE-NEXT:  [[A:[0-9]+]] [label="A"]; // defined externally
 ; PERMODULE-NEXT:  M0_[[MAIN]] -> [[A]] [style=dashed,color=forestgreen]; // const-ref
 ; PERMODULE-NEXT:  [[FOO:[0-9]+]] [label="foo"]; // defined externally
 ; PERMODULE-NEXT:  M0_[[MAIN]] -> [[FOO]] // call (hotness : Unknown)
 ; PERMODULE-NEXT: }
 
 ; COMBINED:     digraph Summary {
 ; COMBINED-NEXT:  // Module: {{.*}}dot-dumper{{.*}}1.bc