[llvm] r325454 - Revert: [llvm] r325448 - [ThinLTO] Add GraphTraits for FunctionSummaries

Simon Pilgrim via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 17 16:01:36 PST 2018


Author: rksimon
Date: Sat Feb 17 16:01:36 2018
New Revision: 325454

URL: http://llvm.org/viewvc/llvm-project?rev=325454&view=rev
Log:
Revert: [llvm] r325448 - [ThinLTO] Add GraphTraits for FunctionSummaries 

Add GraphTraits definitions to the FunctionSummary and ModuleSummaryIndex classes. These GraphTraits will be used to construct find SCC's in ThinLTO analysis passes.

Second attempt, since last patch caused stage2 build to fail (now using function_ref rather than std::function).

Reverted due to buildbot failures

Removed:
    llvm/trunk/test/ThinLTO/X86/module_summary_graph_traits.ll
Modified:
    llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
    llvm/trunk/lib/IR/ModuleSummaryIndex.cpp
    llvm/trunk/lib/LTO/LTO.cpp
    llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp

Modified: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h?rev=325454&r1=325453&r2=325454&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h (original)
+++ llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h Sat Feb 17 16:01:36 2018
@@ -162,24 +162,6 @@ struct ValueInfo {
   bool isDSOLocal() const;
 };
 
-inline bool operator==(const ValueInfo &A, const ValueInfo &B) {
-  assert(A.getRef() && B.getRef() &&
-         "Need ValueInfo with non-null Ref to compare GUIDs");
-  return A.getRef() == B.getRef();
-}
-
-inline bool operator!=(const ValueInfo &A, const ValueInfo &B) {
-  assert(A.getRef() && B.getRef() &&
-         "Need ValueInfo with non-null Ref to compare GUIDs");
-  return A.getGUID() != B.getGUID();
-}
-
-inline bool operator<(const ValueInfo &A, const ValueInfo &B) {
-  assert(A.getRef() && B.getRef() &&
-         "Need ValueInfo with non-null Ref to compare GUIDs");
-  return A.getGUID() < B.getGUID();
-}
-
 template <> struct DenseMapInfo<ValueInfo> {
   static inline ValueInfo getEmptyKey() {
     return ValueInfo(false, (GlobalValueSummaryMapTy::value_type *)-8);
@@ -415,25 +397,6 @@ public:
     unsigned ReturnDoesNotAlias : 1;
   };
 
-  /// Create an empty FunctionSummary (with specified call edges).
-  /// Used to represent external nodes and the dummy root node.
-  static FunctionSummary
-  makeDummyFunctionSummary(std::vector<FunctionSummary::EdgeTy> Edges) {
-    return FunctionSummary(
-        FunctionSummary::GVFlags(
-            GlobalValue::LinkageTypes::AvailableExternallyLinkage,
-            /*NotEligibleToImport=*/true, /*Live=*/true, /*IsLocal=*/false),
-        0, FunctionSummary::FFlags{}, std::vector<ValueInfo>(),
-        std::move(Edges), std::vector<GlobalValue::GUID>(),
-        std::vector<FunctionSummary::VFuncId>(),
-        std::vector<FunctionSummary::VFuncId>(),
-        std::vector<FunctionSummary::ConstVCall>(),
-        std::vector<FunctionSummary::ConstVCall>());
-  }
-
-  /// A dummy node to reference external functions that aren't in the index
-  static FunctionSummary ExternalNode;
-
 private:
   /// Number of instructions (ignoring debug instructions, e.g.) computed
   /// during the initial compile step when the summary index is first built.
@@ -553,8 +516,6 @@ public:
       TIdInfo = llvm::make_unique<TypeIdInfo>();
     TIdInfo->TypeTests.push_back(Guid);
   }
-
-  friend struct GraphTraits<ValueInfo>;
 };
 
 template <> struct DenseMapInfo<FunctionSummary::VFuncId> {
@@ -757,65 +718,6 @@ public:
   const_gvsummary_iterator end() const { return GlobalValueMap.end(); }
   size_t size() const { return GlobalValueMap.size(); }
 
-  // Calculate the callgraph root
-  FunctionSummary calculateCallGraphRoot() {
-    // Functions that have a parent will be marked in FunctionHasParent pair.
-    // Once we've marked all functions, the functions in the map that are false
-    // have no parent (so they're the roots)
-    std::map<ValueInfo, bool> FunctionHasParent;
-    function_ref<void(ValueInfo)> discoverNodes = [&](ValueInfo V) {
-      if (!V.getSummaryList().size())
-        return; // skip external functions that don't have summaries
-
-      // Mark discovered if we haven't yet
-      auto S = FunctionHasParent.emplace(V, false);
-
-      // Stop if we've already discovered this node
-      if (!S.second)
-        return;
-
-      FunctionSummary *F =
-          dyn_cast<FunctionSummary>(V.getSummaryList().front().get());
-      assert(F != nullptr && "Expected FunctionSummary node");
-
-      for (auto &C : F->calls()) {
-        // Insert node if necessary
-        auto S = FunctionHasParent.emplace(C.first, true);
-
-        // Skip nodes that we're sure have parents
-        if (!S.second && S.first->second)
-          continue;
-
-        if (S.second)
-          discoverNodes(C.first);
-        else
-          S.first->second = true;
-      }
-    };
-
-    for (auto &S : *this) {
-      // Skip external functions
-      if (!S.second.SummaryList.size() ||
-          !isa<FunctionSummary>(S.second.SummaryList.front().get()))
-        continue;
-      discoverNodes(ValueInfo(IsAnalysis, &S));
-    }
-
-    std::vector<FunctionSummary::EdgeTy> Edges;
-    // create edges to all roots in the Index
-    for (auto &P : FunctionHasParent) {
-      if (P.second)
-        continue; // skip over non-root nodes
-      Edges.push_back(std::make_pair(P.first, CalleeInfo{}));
-    }
-    if (Edges.empty()) {
-      // Failed to find root - return an empty node
-      return FunctionSummary::makeDummyFunctionSummary({});
-    }
-    auto CallGraphRoot = FunctionSummary::makeDummyFunctionSummary(Edges);
-    return CallGraphRoot;
-  }
-
   bool withGlobalValueDeadStripping() const {
     return WithGlobalValueDeadStripping;
   }
@@ -846,7 +748,7 @@ public:
     return ValueInfo(IsAnalysis, getOrInsertValuePtr(GUID));
   }
 
-  /// Return a ValueInfo for \p GUID setting value \p Name.
+  /// Return a ValueInfo for \p GUID setting value \p Name. 
   ValueInfo getOrInsertValueInfo(GlobalValue::GUID GUID, StringRef Name) {
     assert(!IsAnalysis);
     auto VP = getOrInsertValuePtr(GUID);
@@ -1021,56 +923,6 @@ public:
 
   /// Export summary to dot file for GraphViz.
   void exportToDot(raw_ostream& OS) const;
-
-  /// Print out strongly connected components for debugging.
-  void dumpSCCs(raw_ostream &OS);
-};
-
-/// GraphTraits definition to build SCC for the index
-template <> struct GraphTraits<ValueInfo> {
-  typedef ValueInfo NodeRef;
-
-  static NodeRef valueInfoFromEdge(FunctionSummary::EdgeTy &P) {
-    return P.first;
-  }
-  using ChildIteratorType =
-      mapped_iterator<std::vector<FunctionSummary::EdgeTy>::iterator,
-                      decltype(&valueInfoFromEdge)>;
-
-  static NodeRef getEntryNode(ValueInfo V) { return V; }
-
-  static ChildIteratorType child_begin(NodeRef N) {
-    if (!N.getSummaryList().size()) // handle external function
-      return ChildIteratorType(
-          FunctionSummary::ExternalNode.CallGraphEdgeList.begin(),
-          &valueInfoFromEdge);
-    FunctionSummary *F =
-        cast<FunctionSummary>(N.getSummaryList().front()->getBaseObject());
-    return ChildIteratorType(F->CallGraphEdgeList.begin(), &valueInfoFromEdge);
-  }
-
-  static ChildIteratorType child_end(NodeRef N) {
-    if (!N.getSummaryList().size()) // handle external function
-      return ChildIteratorType(
-          FunctionSummary::ExternalNode.CallGraphEdgeList.end(),
-          &valueInfoFromEdge);
-    FunctionSummary *F =
-        cast<FunctionSummary>(N.getSummaryList().front()->getBaseObject());
-    return ChildIteratorType(F->CallGraphEdgeList.end(), &valueInfoFromEdge);
-  }
-};
-
-template <>
-struct GraphTraits<ModuleSummaryIndex *> : public GraphTraits<ValueInfo> {
-  static NodeRef getEntryNode(ModuleSummaryIndex *I) {
-    std::unique_ptr<GlobalValueSummary> Root =
-        make_unique<FunctionSummary>(I->calculateCallGraphRoot());
-    GlobalValueSummaryInfo G(I->isPerformingAnalysis());
-    G.SummaryList.push_back(std::move(Root));
-    static auto P =
-        GlobalValueSummaryMapTy::value_type(GlobalValue::GUID(0), std::move(G));
-    return ValueInfo(I->isPerformingAnalysis(), &P);
-  }
 };
 
 } // end namespace llvm

Modified: llvm/trunk/lib/IR/ModuleSummaryIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/ModuleSummaryIndex.cpp?rev=325454&r1=325453&r2=325454&view=diff
==============================================================================
--- llvm/trunk/lib/IR/ModuleSummaryIndex.cpp (original)
+++ llvm/trunk/lib/IR/ModuleSummaryIndex.cpp Sat Feb 17 16:01:36 2018
@@ -13,14 +13,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/ModuleSummaryIndex.h"
-#include "llvm/ADT/SCCIterator.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
-FunctionSummary FunctionSummary::ExternalNode =
-    FunctionSummary::makeDummyFunctionSummary({});
 bool ValueInfo::isDSOLocal() const {
   // Need to check all summaries are local in case of hash collisions.
   return getSummaryList().size() &&
@@ -84,26 +80,6 @@ bool ModuleSummaryIndex::isGUIDLive(Glob
   return false;
 }
 
-// TODO: write a graphviz dumper for SCCs (see ModuleSummaryIndex::exportToDot)
-// then delete this function and update its tests
-LLVM_DUMP_METHOD
-void ModuleSummaryIndex::dumpSCCs(raw_ostream &O) {
-  for (scc_iterator<ModuleSummaryIndex *> I =
-           scc_begin<ModuleSummaryIndex *>(this);
-       !I.isAtEnd(); ++I) {
-    O << "SCC (" << utostr(I->size()) << " node" << (I->size() == 1 ? "" : "s")
-      << ") {\n";
-    for (const ValueInfo V : *I) {
-      FunctionSummary *F = nullptr;
-      if (V.getSummaryList().size())
-        F = cast<FunctionSummary>(V.getSummaryList().front().get());
-      O << " " << (F == nullptr ? "External" : "") << " " << utostr(V.getGUID())
-        << (I.hasLoop() ? " (has loop)" : "") << "\n";
-    }
-    O << "}\n";
-  }
-}
-
 namespace {
 struct Attributes {
   void add(const Twine &Name, const Twine &Value,

Modified: llvm/trunk/lib/LTO/LTO.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTO.cpp?rev=325454&r1=325453&r2=325454&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/LTO.cpp (original)
+++ llvm/trunk/lib/LTO/LTO.cpp Sat Feb 17 16:01:36 2018
@@ -50,10 +50,6 @@ using namespace object;
 
 #define DEBUG_TYPE "lto"
 
-static cl::opt<bool>
-    DumpThinCGSCCs("dump-thin-cg-sccs", cl::init(false), cl::Hidden,
-                   cl::desc("Dump the SCCs in the ThinLTO index's callgraph"));
-
 // The values are (type identifier, summary) pairs.
 typedef DenseMap<
     GlobalValue::GUID,
@@ -1145,9 +1141,6 @@ Error LTO::runThinLTO(AddStreamFn AddStr
       ThinLTO.ModuleMap.size());
   StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
 
-  if (DumpThinCGSCCs)
-    ThinLTO.CombinedIndex.dumpSCCs(outs());
-
   if (Conf.OptLevel > 0)
     ComputeCrossModuleImport(ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries,
                              ImportLists, ExportLists);

Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=325454&r1=325453&r2=325454&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Sat Feb 17 16:01:36 2018
@@ -18,8 +18,8 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/Constants.h"

Removed: llvm/trunk/test/ThinLTO/X86/module_summary_graph_traits.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/module_summary_graph_traits.ll?rev=325453&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/module_summary_graph_traits.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/module_summary_graph_traits.ll (removed)
@@ -1,58 +0,0 @@
-; RUN: opt -module-summary %s -o %t1.bc
-; RUN: llvm-lto2 run -print-summary-global-ids -dump-thin-cg-sccs %t1.bc -o %t.index.bc \
-; RUN:     -r %t1.bc,external,px -r %t1.bc,l2,pl -r %t1.bc,l1,pl \
-; RUN:     -r %t1.bc,simple,pl -r %t1.bc,root,pl 2>&1 | FileCheck %s
-
-; CHECK: 5224464028922159466{{.*}} is external
-; CHECK: 765152853862302398{{.*}} is l2
-; CHECK: 17000277804057984823{{.*}} is l1
-; CHECK: 15440740835768581517{{.*}} is simple
-; CHECK: 5800840261926955363{{.*}} is root
-
-; CHECK: SCC (2 nodes) {
-; CHECK-NEXT: {{^}} 17000277804057984823 (has loop)
-; CHECK-NEXT: {{^}} 765152853862302398 (has loop)
-; CHECK-NEXT: }
-
-; CHECK: SCC (1 node) {
-; CHECK-NEXT: {{^}} 15440740835768581517{{$}}
-; CHECK-NEXT: }
-
-; CHECK: SCC (1 node) {
-; CHECK-NEXT: External 5224464028922159466{{$}}
-; CHECK-NEXT: }
-
-; CHECK: SCC (1 node) {
-; CHECK-NEXT: {{^}} 5800840261926955363{{$}}
-; CHECK-NEXT: }
-
-; Dummy call graph root that points at all roots of the callgraph.
-; CHECK: SCC (1 node) {
-; CHECK-NEXT: {{^}} 0{{$}}
-; CHECK-NEXT: }
-
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-declare void @external()
-
-define void @l2() {
-  call void @l1()
-  ret void
-}
-
-define void @l1() {
-  call void @l2()
-  ret void
-}
-
-define i32 @simple() {
-  ret i32 23
-}
-
-define void @root() {
-  call void @l1()
-  call i32 @simple()
-  call void @external()
-  ret void
-}




More information about the llvm-commits mailing list