[PATCH] Fix GraphTraits for "const CallGraphNode *" and "const CallGraph *"

Speziale Ettore speziale.ettore at gmail.com
Thu Nov 6 16:26:45 PST 2014


Hello,


>
> Is it possible to avoid the code duplication?
>>
>
> Not sure about that ... I'll try to do some experiments ...
>

I tried to factorize the const CallGraphNode * with CallGraphNode *
specialization and const CallGraph * with CallGraph * specialization. I had
to introduce some templates/traits to support that. It is working, but I am
not sure if it is the correct approach ... -- I'm not very expert with
templates ....


> Since this is just an API change, can you add a unittest?
>
>
Added some simple unit tests that iterate over all nodes in different call
graphs using GraphTraits iterators.

In the process, I added a couple of member functions to CallGraph -- i.e.
CallGraph::size() -- and CallGraph::empty() -- to implement sanity checks
in the unit tests.

To build the test call graphs I used an utility function I found in
LazyCallGraphTests.cpp. I moved that function to a separate file to be used
by both tests.

I modified the CMake files in order to build and link the new tests, but I
am not very expert of it. I tried to grep the codebase and I did not find
an header file being included by any add_llvm_unittest, so I did not add
CallGraphTestUtils.h there. Is that OK?

Thanks,
Ettore Speziale
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141106/9ac5b50d/attachment.html>
-------------- next part --------------
diff --git a/include/llvm/Analysis/CallGraph.h b/include/llvm/Analysis/CallGraph.h
index 24fa99f..8161a3b 100644
--- a/include/llvm/Analysis/CallGraph.h
+++ b/include/llvm/Analysis/CallGraph.h
@@ -119,6 +119,8 @@ public:
   inline iterator end() { return FunctionMap.end(); }
   inline const_iterator begin() const { return FunctionMap.begin(); }
   inline const_iterator end() const { return FunctionMap.end(); }
+  inline bool empty() const { return FunctionMap.empty(); }
+  inline unsigned size() const { return (unsigned)FunctionMap.size(); }
 
   /// \brief Returns the call graph node for the provided function.
   inline const CallGraphNode *operator[](const Function *F) const {
@@ -392,72 +394,87 @@ public:
 // graphs by the generic graph algorithms.
 //
 
-// Provide graph traits for tranversing call graphs using standard graph
-// traversals.
-template <> struct GraphTraits<CallGraphNode *> {
-  typedef CallGraphNode NodeType;
+template <typename Ty> struct CGIterTraits;
 
-  typedef CallGraphNode::CallRecord CGNPairTy;
-  typedef std::pointer_to_unary_function<CGNPairTy, CallGraphNode *>
-  CGNDerefFun;
+template <typename Ty> struct CGNodeGraphTraits {
+  typedef Ty NodeType;
+  typedef typename CGIterTraits<Ty>::iterator IterTy;
 
-  static NodeType *getEntryNode(CallGraphNode *CGN) { return CGN; }
+  typedef typename Ty::CallRecord CGNPairTy;
+  typedef std::pointer_to_unary_function<CGNPairTy, NodeType *> CGNDerefFun;
 
-  typedef mapped_iterator<NodeType::iterator, CGNDerefFun> ChildIteratorType;
+  static NodeType *getEntryNode(NodeType *CGN) { return CGN; }
 
-  static inline ChildIteratorType child_begin(NodeType *N) {
-    return map_iterator(N->begin(), CGNDerefFun(CGNDeref));
+  typedef mapped_iterator<IterTy, CGNDerefFun> ChildIteratorType;
+
+  static inline ChildIteratorType child_begin(NodeType *CGN) {
+    return map_iterator(CGN->begin(), CGNDerefFun(CGNDeref));
   }
-  static inline ChildIteratorType child_end(NodeType *N) {
-    return map_iterator(N->end(), CGNDerefFun(CGNDeref));
+
+  static inline ChildIteratorType child_end(NodeType *CGN) {
+    return map_iterator(CGN->end(), CGNDerefFun(CGNDeref));
   }
 
-  static CallGraphNode *CGNDeref(CGNPairTy P) { return P.second; }
+  static NodeType *CGNDeref(CGNPairTy P) { return P.second; }
 };
 
-template <> struct GraphTraits<const CallGraphNode *> {
-  typedef const CallGraphNode NodeType;
-  typedef NodeType::const_iterator ChildIteratorType;
-
-  static NodeType *getEntryNode(const CallGraphNode *CGN) { return CGN; }
-  static inline ChildIteratorType child_begin(NodeType *N) {
-    return N->begin();
-  }
-  static inline ChildIteratorType child_end(NodeType *N) { return N->end(); }
-};
+template <typename GraphTy,
+          typename NodeTy = typename CGIterTraits<GraphTy>::value_type>
+struct CGGraphTraits : public CGNodeGraphTraits<NodeTy> {
+  typedef typename CGIterTraits<GraphTy>::iterator IterTy;
 
-template <>
-struct GraphTraits<CallGraph *> : public GraphTraits<CallGraphNode *> {
-  static NodeType *getEntryNode(CallGraph *CGN) {
-    return CGN->getExternalCallingNode(); // Start at the external node!
+  static NodeTy *getEntryNode(GraphTy *CG) {
+    return CG->getExternalCallingNode(); // Start at the external node!
   }
-  typedef std::pair<const Function *, CallGraphNode *> PairTy;
-  typedef std::pointer_to_unary_function<PairTy, CallGraphNode &> DerefFun;
+  typedef std::pair<const Function *, NodeTy *> CGPairTy;
+  typedef std::pointer_to_unary_function<CGPairTy, NodeTy &> CGDerefFun;
 
   // nodes_iterator/begin/end - Allow iteration over all nodes in the graph
-  typedef mapped_iterator<CallGraph::iterator, DerefFun> nodes_iterator;
-  static nodes_iterator nodes_begin(CallGraph *CG) {
-    return map_iterator(CG->begin(), DerefFun(CGdereference));
+  typedef mapped_iterator<IterTy, CGDerefFun> nodes_iterator;
+  static nodes_iterator nodes_begin(GraphTy *CG) {
+    return map_iterator(CG->begin(), CGDerefFun(CGDeref));
   }
-  static nodes_iterator nodes_end(CallGraph *CG) {
-    return map_iterator(CG->end(), DerefFun(CGdereference));
+  static nodes_iterator nodes_end(GraphTy *CG) {
+    return map_iterator(CG->end(), CGDerefFun(CGDeref));
   }
 
-  static CallGraphNode &CGdereference(PairTy P) { return *P.second; }
+  static NodeTy &CGDeref(CGPairTy P) { return *P.second; }
 };
 
-template <>
-struct GraphTraits<const CallGraph *> : public GraphTraits<
-                                            const CallGraphNode *> {
-  static NodeType *getEntryNode(const CallGraph *CGN) {
-    return CGN->getExternalCallingNode();
-  }
-  // nodes_iterator/begin/end - Allow iteration over all nodes in the graph
-  typedef CallGraph::const_iterator nodes_iterator;
-  static nodes_iterator nodes_begin(const CallGraph *CG) { return CG->begin(); }
-  static nodes_iterator nodes_end(const CallGraph *CG) { return CG->end(); }
+// GraphTraits specializations for {CallGraphNode,CallGraph} *.
+
+template <> struct CGIterTraits<CallGraphNode> {
+  typedef CallGraphNode::iterator iterator;
+};
+
+template <> struct CGIterTraits<CallGraph> {
+  typedef CallGraph::iterator iterator;
+  typedef CallGraphNode value_type;
+};
+
+template <> struct GraphTraits<CallGraphNode *>
+  : public CGNodeGraphTraits<CallGraphNode> { };
+
+template <> struct GraphTraits<CallGraph *>
+  : public CGGraphTraits<CallGraph> { };
+
+// GraphTraits specializations for const {CallGraphNode,CallGraph} *.
+
+template <> struct CGIterTraits<const CallGraphNode> {
+  typedef CallGraphNode::const_iterator iterator;
+};
+
+template <> struct CGIterTraits<const CallGraph> {
+  typedef CallGraph::const_iterator iterator;
+  typedef const CallGraphNode value_type;
 };
 
+template <> struct GraphTraits<const CallGraphNode *>
+  : public CGNodeGraphTraits<const CallGraphNode> { };
+
+template <> struct GraphTraits<const CallGraph *>
+  : public CGGraphTraits<const CallGraph> { };
+
 } // End llvm namespace
 
 #endif
diff --git a/unittests/Analysis/CMakeLists.txt b/unittests/Analysis/CMakeLists.txt
index 8454860..96e1358 100644
--- a/unittests/Analysis/CMakeLists.txt
+++ b/unittests/Analysis/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  IPA
   Analysis
   AsmParser
   Core
@@ -6,6 +7,8 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_llvm_unittest(AnalysisTests
+  CallGraphTest.cpp
+  CallGraphTestUtils.cpp
   CFGTest.cpp
   LazyCallGraphTest.cpp
   ScalarEvolutionTest.cpp
diff --git a/unittests/Analysis/CallGraphTest.cpp b/unittests/Analysis/CallGraphTest.cpp
new file mode 100644
index 0000000..9822997
--- /dev/null
+++ b/unittests/Analysis/CallGraphTest.cpp
@@ -0,0 +1,243 @@
+//=======- CallGraphTest.cpp - Unit tests for the CG analysis -------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/DepthFirstIterator.h"
+#include "llvm/Analysis/CallGraph.h"
+#include "llvm/IR/Module.h"
+#include "gtest/gtest.h"
+
+#include "CallGraphTestUtils.h"
+
+using namespace llvm;
+
+namespace {
+
+template <typename Ty>
+bool GraphTraitsCanIterOverAllNodes(Ty G) {
+  typedef GraphTraits<Ty> TraitsTy;
+
+  std::set<typename TraitsTy::NodeType *> Visited, Expected;
+
+  // Need extract [const] CallGraph's nodes from the GraphTraits' nodes. The
+  // former are [const] CallGraphNode *, whilst the second are
+  // [const] CallGraphNode &.
+  for(auto I = TraitsTy::nodes_begin(G), E = TraitsTy::nodes_end(G);
+      I != E; ++I)
+    Visited.insert(&*I);
+
+  // Need to extract [const] CallGraph's nodes from the [const] CallGraph
+  // iterator since the second iterates over pairs which second element is the
+  // [const] CallGraphNode *.
+  for(auto I = std::begin(*G), E = std::end(*G);
+      I != E; ++I)
+    Expected.insert(I->second);
+
+  return Visited == Expected;
+}
+
+template <typename Ty>
+bool GraphTraitsCanChildIterOverAllNodes(Ty G) {
+  typedef typename GraphTraits<Ty>::NodeType *GraphTy;
+  typedef GraphTraits<GraphTy> TraitsTy;
+
+  GraphTy N = G->getExternalCallingNode();
+  std::set<typename TraitsTy::NodeType *> Visited, Expected;
+
+  // By using a depth-first iterator on the graph,
+  // GraphTraits::ChildIteratorType is exercised. All nodes of the call-graph
+  // are visited since its root -- i.e. the external calling node in these
+  // tests -- is passed to the depth-first iterator.
+  for(auto I = df_begin(N), E = df_end(N); I != E; ++I)
+    Visited.insert(*I);
+
+  // Need to extract [const] CallGraph's nodes from the [const] CallGraph
+  // iterator since the second iterates over pairs which second element is the
+  // [const] CallGraphNode *.
+  for(auto I = std::begin(*G), E = std::end(*G); I != E; ++I)
+    Expected.insert(I->second);
+
+  return Visited == Expected;
+}
+
+const char *EmptyCG =
+  "\n";
+
+const char *SingularCG =
+  "define void @a() {\n"
+  "entry:\n"
+  "  ret void\n"
+  "}\n";
+
+const char *SelfRecursiveCG =
+  "define void @a() {\n"
+  "entry:\n"
+  "  call void @a()\n"
+  "  ret void\n"
+  "}\n";
+
+const char *RecursiveCG =
+  "define void @a1() {\n"
+  "entry:\n"
+  "  call void @a2()\n"
+  "  ret void\n"
+  "}\n"
+  "define void @a2() {\n"
+  "entry:\n"
+  "  call void @a1()\n"
+  "  ret void\n"
+  "}\n"
+  "define void @a3() {\n"
+  "entry:\n"
+  "  call void @a1()\n"
+  "  ret void\n"
+  "}\n";
+
+const char *MultipleSCCsCG =
+  "define void @a1() {\n"
+  "entry:\n"
+  "  call void @a2()\n"
+  "  ret void\n"
+  "}\n"
+  "define void @a2() {\n"
+  "entry:\n"
+  "  call void @a1()\n"
+  "  ret void\n"
+  "}\n"
+  "define void @b1() {\n"
+  "entry:\n"
+  "  call void @b2()\n"
+  "  ret void\n"
+  "}\n"
+  "define void @b2() {\n"
+  "entry:\n"
+  "  call void @b1()\n"
+  "  ret void\n"
+  "}\n"
+  "define void @c() {\n"
+  "entry:\n"
+  "  call void @a1()\n"
+  "  call void @b1()\n"
+  "  ret void\n"
+  "}\n";
+
+} // End anonymous namespace.
+
+// Tests about CallGraphTraits::nodes_{begin,end}
+
+TEST(CallGraphTest, GraphTraitsCanIterOnEmpty) {
+  std::unique_ptr<Module> M = parseAssembly(EmptyCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CGC));
+}
+
+TEST(CallGraphTest, GraphTraitsCanIterOnSingular) {
+  std::unique_ptr<Module> M = parseAssembly(SingularCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CGC));
+}
+
+TEST(CallGraphTest, GraphTraitsCanIterOnSelfRecursive) {
+  std::unique_ptr<Module> M = parseAssembly(SelfRecursiveCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CGC));
+}
+
+TEST(CallGraphTest, GraphTraitsCanIterOnRecursive) {
+  std::unique_ptr<Module> M = parseAssembly(RecursiveCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CGC));
+}
+
+TEST(CallGraphTest, GraphTraitsCanIterOnMultipleSCCs) {
+  std::unique_ptr<Module> M = parseAssembly(MultipleSCCsCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanIterOverAllNodes(&CGC));
+}
+
+// Tests about CallGraphTraits::child_{begin,end}
+
+TEST(CallGraphTest, GraphTraitsCanChildIterOnEmpty) {
+  std::unique_ptr<Module> M = parseAssembly(EmptyCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CGC));
+}
+
+TEST(CallGraphTest, GraphTraitsCanChildIterOnSingular) {
+  std::unique_ptr<Module> M = parseAssembly(SingularCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CGC));
+}
+
+TEST(CallGraphTest, GraphTraitsCanChildIterOnSelfRecursive) {
+  std::unique_ptr<Module> M = parseAssembly(SelfRecursiveCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CGC));
+}
+
+TEST(CallGraphTest, GraphTraitsCanChildIterOnRecursive) {
+  std::unique_ptr<Module> M = parseAssembly(RecursiveCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CGC));
+}
+
+TEST(CallGraphTest, GraphTraitsCanChildIterOnMultipleSCCs) {
+  std::unique_ptr<Module> M = parseAssembly(MultipleSCCsCG);
+
+  CallGraph CG(*M);
+  const CallGraph &CGC = CG;
+
+  EXPECT_EQ(1 + M->size(), CG.size());
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CG));
+  EXPECT_TRUE(GraphTraitsCanChildIterOverAllNodes(&CGC));
+}
diff --git a/unittests/Analysis/CallGraphTestUtils.cpp b/unittests/Analysis/CallGraphTestUtils.cpp
new file mode 100644
index 0000000..54943b9
--- /dev/null
+++ b/unittests/Analysis/CallGraphTestUtils.cpp
@@ -0,0 +1,35 @@
+//=======- CallGraphTestUtils.cpp - Utils for CG tests --------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "CallGraphTestUtils.h"
+
+using namespace llvm;
+
+std::unique_ptr<Module> llvm::parseAssembly(const char *Assembly) {
+  SMDiagnostic Error;
+  std::unique_ptr<Module> M =
+      parseAssemblyString(Assembly, Error, getGlobalContext());
+
+  std::string ErrMsg;
+  raw_string_ostream OS(ErrMsg);
+  Error.print("", OS);
+
+  // A failure here means that the test itself is buggy.
+  if (!M)
+    report_fatal_error(OS.str().c_str());
+
+  return M;
+}
diff --git a/unittests/Analysis/CallGraphTestUtils.h b/unittests/Analysis/CallGraphTestUtils.h
new file mode 100644
index 0000000..abda82c
--- /dev/null
+++ b/unittests/Analysis/CallGraphTestUtils.h
@@ -0,0 +1,28 @@
+//=======- CallGraphTestUtils.h - Utils for CG tests ----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ANALYSIS_CALLGRAPHTESTUTILS_H
+#define LLVM_ANALYSIS_CALLGRAPHTESTUTILS_H
+
+#include <memory>
+
+namespace llvm {
+
+class Module;
+
+/// \brief Build the given LLVM IR assembly into a module.
+///
+/// This function allows to build a module starting from an LLVM IR assembly
+/// string. The module is created inside the global context. If an error
+/// occurs, execution is aborted.
+std::unique_ptr<Module> parseAssembly(const char *Assembly);
+
+}
+
+#endif // LLVM_ANALYSIS_CALLGRAPHTESTUTILS_H
diff --git a/unittests/Analysis/LazyCallGraphTest.cpp b/unittests/Analysis/LazyCallGraphTest.cpp
index 6caccb8..f58001d 100644
--- a/unittests/Analysis/LazyCallGraphTest.cpp
+++ b/unittests/Analysis/LazyCallGraphTest.cpp
@@ -8,35 +8,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/LazyCallGraph.h"
-#include "llvm/AsmParser/Parser.h"
-#include "llvm/IR/Function.h"
-#include "llvm/IR/LLVMContext.h"
-#include "llvm/IR/Module.h"
-#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
-#include <memory>
+
+#include "CallGraphTestUtils.h"
 
 using namespace llvm;
 
 namespace {
 
-std::unique_ptr<Module> parseAssembly(const char *Assembly) {
-  SMDiagnostic Error;
-  std::unique_ptr<Module> M =
-      parseAssemblyString(Assembly, Error, getGlobalContext());
-
-  std::string ErrMsg;
-  raw_string_ostream OS(ErrMsg);
-  Error.print("", OS);
-
-  // A failure here means that the test itself is buggy.
-  if (!M)
-    report_fatal_error(OS.str().c_str());
-
-  return M;
-}
-
 /*
    IR forming a call graph with a diamond of triangle-shaped SCCs:
 
diff --git a/unittests/Analysis/Makefile b/unittests/Analysis/Makefile
index 527f452..52296e7 100644
--- a/unittests/Analysis/Makefile
+++ b/unittests/Analysis/Makefile
@@ -9,7 +9,7 @@
 
 LEVEL = ../..
 TESTNAME = Analysis
-LINK_COMPONENTS := analysis asmparser
+LINK_COMPONENTS := ipa analysis asmparser
 
 include $(LEVEL)/Makefile.config
 include $(LLVM_SRC_ROOT)/unittests/Makefile.unittest


More information about the llvm-commits mailing list