Re: [PATCH] D23730: [GraphTraits] Replace all NodeType usage with NodeRef

[Tim Shen via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL279475: [GraphTraits] Replace all NodeType usage with 
NodeRef (authored by timshen).

Changed prior to commit:
  https://reviews.llvm.org/D23730?vs=68748=68912#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23730

Files:
  cfe/trunk/include/clang/AST/StmtGraphTraits.h
  cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
  cfe/trunk/include/clang/Analysis/CFG.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
  cfe/trunk/lib/Serialization/ModuleManager.cpp
  llvm/trunk/include/llvm/ADT/GraphTraits.h
  llvm/trunk/include/llvm/Analysis/CallGraph.h
  llvm/trunk/include/llvm/Analysis/Interval.h
  llvm/trunk/include/llvm/Analysis/LazyCallGraph.h
  llvm/trunk/include/llvm/Analysis/LoopInfo.h
  llvm/trunk/include/llvm/Analysis/PostDominators.h
  llvm/trunk/include/llvm/Analysis/RegionIterator.h
  llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/trunk/include/llvm/CodeGen/MachineDominators.h
  llvm/trunk/include/llvm/CodeGen/MachineFunction.h
  llvm/trunk/include/llvm/CodeGen/MachineLoopInfo.h
  llvm/trunk/include/llvm/CodeGen/MachineRegionInfo.h
  llvm/trunk/include/llvm/CodeGen/ScheduleDAG.h
  llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/trunk/include/llvm/IR/CFG.h
  llvm/trunk/include/llvm/IR/Dominators.h
  llvm/trunk/include/llvm/IR/Type.h
  llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h
  llvm/trunk/lib/Analysis/BlockFrequencyInfo.cpp
  llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp
  llvm/trunk/lib/CodeGen/MachineBlockFrequencyInfo.cpp
  llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/trunk/unittests/ADT/SCCIteratorTest.cpp
  llvm/trunk/unittests/Analysis/CallGraphTest.cpp

Index: llvm/trunk/include/llvm/IR/Type.h
===
--- llvm/trunk/include/llvm/IR/Type.h
+++ llvm/trunk/include/llvm/IR/Type.h
@@ -429,29 +429,27 @@
 // graph of sub types.
 
 template <> struct GraphTraits {
-  typedef Type NodeType;
   typedef Type *NodeRef;
   typedef Type::subtype_iterator ChildIteratorType;
 
-  static inline NodeType *getEntryNode(Type *T) { return T; }
-  static inline ChildIteratorType child_begin(NodeType *N) {
+  static inline NodeRef getEntryNode(Type *T) { return T; }
+  static inline ChildIteratorType child_begin(NodeRef N) {
 return N->subtype_begin();
   }
-  static inline ChildIteratorType child_end(NodeType *N) {
+  static inline ChildIteratorType child_end(NodeRef N) {
 return N->subtype_end();
   }
 };
 
 template <> struct GraphTraits {
-  typedef const Type NodeType;
   typedef const Type *NodeRef;
   typedef Type::subtype_iterator ChildIteratorType;
 
-  static inline NodeType *getEntryNode(NodeType *T) { return T; }
-  static inline ChildIteratorType child_begin(NodeType *N) {
+  static inline NodeRef getEntryNode(NodeRef T) { return T; }
+  static inline ChildIteratorType child_begin(NodeRef N) {
 return N->subtype_begin();
   }
-  static inline ChildIteratorType child_end(NodeType *N) {
+  static inline ChildIteratorType child_end(NodeRef N) {
 return N->subtype_end();
   }
 };
Index: llvm/trunk/include/llvm/IR/CFG.h
===
--- llvm/trunk/include/llvm/IR/CFG.h
+++ llvm/trunk/include/llvm/IR/CFG.h
@@ -154,65 +154,51 @@
 // graph of basic blocks...
 
 template <> struct GraphTraits {
-  typedef BasicBlock NodeType;
   typedef BasicBlock *NodeRef;
   typedef succ_iterator ChildIteratorType;
 
-  static NodeType *getEntryNode(BasicBlock *BB) { return BB; }
-  static inline ChildIteratorType child_begin(NodeType *N) {
+  static NodeRef getEntryNode(BasicBlock *BB) { return BB; }
+  static inline ChildIteratorType child_begin(NodeRef N) {
 return succ_begin(N);
   }
-  static inline ChildIteratorType child_end(NodeType *N) {
-return succ_end(N);
-  }
+  static inline ChildIteratorType child_end(NodeRef N) { return succ_end(N); }
 };
 
 template <> struct GraphTraits {
-  typedef const BasicBlock NodeType;
   typedef const BasicBlock *NodeRef;
   typedef succ_const_iterator ChildIteratorType;
 
-  static NodeType *getEntryNode(const BasicBlock *BB) { return BB; }
+  static NodeRef getEntryNode(const BasicBlock *BB) { return BB; }
 
-  static inline ChildIteratorType child_begin(NodeType *N) {
+  static inline ChildIteratorType child_begin(NodeRef N) {
 return succ_begin(N);
   }
-  static inline ChildIteratorType child_end(NodeType *N) {
-return succ_end(N);
-  }
+  static inline ChildIteratorType child_end(NodeRef N) { return succ_end(N); }
 };
 
 // Provide specializations of GraphTraits to be able to treat a function as a
 // graph of basic blocks... and to walk it in inverse order.  Inverse order for
 // a function is considered to be when traversing the predecessor edges of a BB
 // instead of the successor edges.
 //
 template <> struct 

Re: [PATCH] D23730: [GraphTraits] Replace all NodeType usage with NodeRef

[Tim Shen via cfe-commits]

timshen added a comment.

In https://reviews.llvm.org/D23730#522537, @dblaikie wrote:

> I'd sort of be inclined to remove them, then - but I leave that up to you.


It looks like people create some class, and add GraphTraits specialization for 
it, in the hope that others can use it, but didn't write a test. The right 
thing is to add the test, but I'm not going to be the one. :)

So I'll keep them.


https://reviews.llvm.org/D23730



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23730: [GraphTraits] Replace all NodeType usage with NodeRef

[David Blaikie via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Or it isn't approved (was mixing up reviews) - but now it is...


https://reviews.llvm.org/D23730



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23730: [GraphTraits] Replace all NodeType usage with NodeRef

[David Blaikie via cfe-commits]

dblaikie added a comment.

In https://reviews.llvm.org/D23730#522501, @timshen wrote:

> In https://reviews.llvm.org/D23730#522396, @dblaikie wrote:
>
> > Not all of these already had NodeRef implemented - that implies that some 
> > algorithms weren't using NodeRef before this change, or that these traits 
> > are unused? I thought the plan was to migrate each algorithm then just do a 
> > strict cleanup. Did that not pan out/some other aspects I'm forgetting?
>
>
> I think those traits are unused. Notice that by merely removing "NodeType *" 
> declarations, the build/tests doesn't break, so at least they are not covered 
> by the tests.


I'd sort of be inclined to remove them, then - but I leave that up to you.

Otherwise this review is already approved, so commit as you see fit.


https://reviews.llvm.org/D23730



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23730: [GraphTraits] Replace all NodeType usage with NodeRef

[Tim Shen via cfe-commits]

timshen added a comment.

In https://reviews.llvm.org/D23730#522396, @dblaikie wrote:

> Not all of these already had NodeRef implemented - that implies that some 
> algorithms weren't using NodeRef before this change, or that these traits are 
> unused? I thought the plan was to migrate each algorithm then just do a 
> strict cleanup. Did that not pan out/some other aspects I'm forgetting?


I think those traits are unused. Notice that by merely removing "NodeType *" 
declarations, the build/tests doesn't break, so at least they are not covered 
by the tests.


https://reviews.llvm.org/D23730



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23730: [GraphTraits] Replace all NodeType usage with NodeRef

[David Blaikie via cfe-commits]

dblaikie added a comment.

Not all of these already had NodeRef implemented - that implies that some 
algorithms weren't using NodeRef before this change, or that these traits are 
unused? I thought the plan was to migrate each algorithm then just do a strict 
cleanup. Did that not pan out/some other aspects I'm forgetting?



Comment at: llvm/trunk/include/llvm/CodeGen/ScheduleDAG.h:684-688
@@ -684,7 +683,7 @@
 typedef SUnitIterator ChildIteratorType;
-static inline NodeType *getEntryNode(SUnit *N) { return N; }
-static inline ChildIteratorType child_begin(NodeType *N) {
+static inline NodeRef getEntryNode(SUnit *N) { return N; }
+static inline ChildIteratorType child_begin(NodeRef N) {
   return SUnitIterator::begin(N);
 }
-static inline ChildIteratorType child_end(NodeType *N) {
+static inline ChildIteratorType child_end(NodeRef N) {
   return SUnitIterator::end(N);

If you like you could probably (separate commit) drop the 'inline' from these 
functions, they're implicitly inline anyway


Comment at: llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h:2060-2064
@@ -2060,7 +2059,7 @@
   typedef SDNodeIterator ChildIteratorType;
-  static inline NodeType *getEntryNode(SDNode *N) { return N; }
-  static inline ChildIteratorType child_begin(NodeType *N) {
+  static inline NodeRef getEntryNode(SDNode *N) { return N; }
+  static inline ChildIteratorType child_begin(NodeRef N) {
 return SDNodeIterator::begin(N);
   }
-  static inline ChildIteratorType child_end(NodeType *N) {
+  static inline ChildIteratorType child_end(NodeRef N) {
 return SDNodeIterator::end(N);

and here


Comment at: llvm/trunk/include/llvm/IR/CFG.h:161-164
@@ -163,6 +160,6 @@
+  static NodeRef getEntryNode(BasicBlock *BB) { return BB; }
+  static inline ChildIteratorType child_begin(NodeRef N) {
 return succ_begin(N);
   }
-  static inline ChildIteratorType child_end(NodeType *N) {
-return succ_end(N);
-  }
+  static inline ChildIteratorType child_end(NodeRef N) { return succ_end(N); }
 };

And here.. and so on.


https://reviews.llvm.org/D23730



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D23730: [GraphTraits] Replace all NodeType usage with NodeRef

[Tim Shen via cfe-commits]

timshen created this revision.
timshen added a reviewer: dblaikie.
timshen added subscribers: llvm-commits, cfe-commits.
Herald added subscribers: mzolotukhin, MatzeB.

This should finish the GraphTraits migration.

https://reviews.llvm.org/D23730

Files:
  cfe/trunk/include/clang/AST/StmtGraphTraits.h
  cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
  cfe/trunk/include/clang/Analysis/CFG.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
  cfe/trunk/lib/Serialization/ModuleManager.cpp
  llvm/trunk/include/llvm/ADT/GraphTraits.h
  llvm/trunk/include/llvm/Analysis/CallGraph.h
  llvm/trunk/include/llvm/Analysis/Interval.h
  llvm/trunk/include/llvm/Analysis/LazyCallGraph.h
  llvm/trunk/include/llvm/Analysis/LoopInfo.h
  llvm/trunk/include/llvm/Analysis/PostDominators.h
  llvm/trunk/include/llvm/Analysis/RegionIterator.h
  llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/trunk/include/llvm/CodeGen/MachineDominators.h
  llvm/trunk/include/llvm/CodeGen/MachineFunction.h
  llvm/trunk/include/llvm/CodeGen/MachineLoopInfo.h
  llvm/trunk/include/llvm/CodeGen/MachineRegionInfo.h
  llvm/trunk/include/llvm/CodeGen/ScheduleDAG.h
  llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/trunk/include/llvm/IR/CFG.h
  llvm/trunk/include/llvm/IR/Dominators.h
  llvm/trunk/include/llvm/IR/Type.h
  llvm/trunk/include/llvm/Transforms/Utils/MemorySSA.h
  llvm/trunk/lib/Analysis/BlockFrequencyInfo.cpp
  llvm/trunk/lib/Analysis/BlockFrequencyInfoImpl.cpp
  llvm/trunk/lib/CodeGen/MachineBlockFrequencyInfo.cpp
  llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/trunk/unittests/ADT/SCCIteratorTest.cpp
  llvm/trunk/unittests/Analysis/CallGraphTest.cpp

Index: llvm/trunk/unittests/Analysis/CallGraphTest.cpp
===
--- llvm/trunk/unittests/Analysis/CallGraphTest.cpp
+++ llvm/trunk/unittests/Analysis/CallGraphTest.cpp
@@ -17,29 +17,29 @@
 namespace {
 
 template  void canSpecializeGraphTraitsIterators(Ty *G) {
-  typedef typename GraphTraits::NodeType NodeTy;
+  typedef typename GraphTraits::NodeRef NodeRef;
 
   auto I = GraphTraits::nodes_begin(G);
   auto E = GraphTraits::nodes_end(G);
   auto X = ++I;
 
   // Should be able to iterate over all nodes of the graph.
-  static_assert(std::is_same::value,
+  static_assert(std::is_same::value,
 "Node type does not match");
-  static_assert(std::is_same::value,
+  static_assert(std::is_same::value,
 "Node type does not match");
-  static_assert(std::is_same::value,
+  static_assert(std::is_same::value,
 "Node type does not match");
 
-  NodeTy *N = GraphTraits::getEntryNode(G);
+  NodeRef N = GraphTraits::getEntryNode(G);
 
-  auto S = GraphTraits::child_begin(N);
-  auto F = GraphTraits::child_end(N);
+  auto S = GraphTraits::child_begin(N);
+  auto F = GraphTraits::child_end(N);
 
   // Should be able to iterate over immediate successors of a node.
-  static_assert(std::is_same::value,
+  static_assert(std::is_same::value,
 "Node type does not match");
-  static_assert(std::is_same::value,
+  static_assert(std::is_same::value,
 "Node type does not match");
 }
 
Index: llvm/trunk/unittests/ADT/SCCIteratorTest.cpp
===
--- llvm/trunk/unittests/ADT/SCCIteratorTest.cpp
+++ llvm/trunk/unittests/ADT/SCCIteratorTest.cpp
@@ -229,15 +229,16 @@
 
 template 
 struct GraphTraits {
-  typedef typename Graph::NodeType NodeType;
   typedef typename Graph::NodeType *NodeRef;
   typedef typename Graph::ChildIterator ChildIteratorType;
 
- static inline NodeType *getEntryNode(const Graph ) { return G.AccessNode(0); }
- static inline ChildIteratorType child_begin(NodeType *Node) {
-   return Graph::child_begin(Node);
+  static inline NodeRef getEntryNode(const Graph ) {
+return G.AccessNode(0);
+  }
+  static inline ChildIteratorType child_begin(NodeRef Node) {
+return Graph::child_begin(Node);
  }
- static inline ChildIteratorType child_end(NodeType *Node) {
+ static inline ChildIteratorType child_end(NodeRef Node) {
return Graph::child_end(Node);
  }
 };
Index: llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
===
--- llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
+++ llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -332,23 +332,18 @@
 
 namespace llvm {
 template <> struct GraphTraits {
-  typedef ArgumentGraphNode NodeType;
   typedef ArgumentGraphNode *NodeRef;
   typedef SmallVectorImpl::iterator ChildIteratorType;
 
-  static inline NodeType *getEntryNode(NodeType *A) { return A; }
-  static inline ChildIteratorType