[GraphWriter] Change GraphWriter to use NodeRef in GraphTraits

[timshen]

Summary

This is part of the "NodeType* -> NodeRef" migration. Notice that since
GraphWriter prints object address as identity, I added a static_assert on
NodeRef to be a pointer type.

[dblaikie]

what's the printer used for? Would it be best to generalize it (perhaps adding another extension point to the graph traits, so NodeRefs can be printed out - with a default implementation for pointers)?

[timshen]

In D23580#517447, @dblaikie wrote:

what's the printer used for? Would it be best to generalize it (perhaps adding another extension point to the graph traits, so NodeRefs can be printed out - with a default implementation for pointers)?

One usage is to print dot files.

I think it's more like that GraphWriter that requires the input nodes to be both fulfilling GraphTraits, and printable, not that GraphTraits has an optional printable requirement - these are two totally orthogonal traits.

[dblaikie]

Thanks for the explanation - I think that makes sense, that node printability is an orthogonal concept from the graph traits, hopefully. (the counterexample to this would be if two graphs have the same noderef type but need different printing schemes, but I thin kthat'd only come up if there was a concept of a "graph" distinct from nodes in the graph traits machinery (ie: if you didn't need fat pointers because you already had access to the graph... anyway))

[timshen]

Updated static_assert error message to illustrate the right thing to do.

llvm/trunk/include/llvm/CodeGen/ScheduleDAG.h

public:
     }
     const SDep &getSDep() const {
       return Node->Preds[Operand];
     }
   };
 
   template <> struct GraphTraits<SUnit*> {
     typedef SUnit NodeType;
+    typedef SUnit *NodeRef;
     typedef SUnitIterator ChildIteratorType;
     static inline NodeType *getEntryNode(SUnit *N) { return N; }
     static inline ChildIteratorType child_begin(NodeType *N) {
       return SUnitIterator::begin(N);
     }
     static inline ChildIteratorType child_end(NodeType *N) {
       return SUnitIterator::end(N);
     }

llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h

public:
   }
 
   unsigned getOperand() const { return Operand; }
   const SDNode *getNode() const { return Node; }
 };
 
 template <> struct GraphTraits<SDNode*> {
   typedef SDNode NodeType;
+  typedef SDNode *NodeRef;
   typedef SDNodeIterator ChildIteratorType;
   static inline NodeType *getEntryNode(SDNode *N) { return N; }
   static inline ChildIteratorType child_begin(NodeType *N) {
     return SDNodeIterator::begin(N);
   }
   static inline ChildIteratorType child_end(NodeType *N) {
     return SDNodeIterator::end(N);
   }

llvm/trunk/include/llvm/Support/GraphWriter.h

 
 template<typename GraphType>
 class GraphWriter {
   raw_ostream &O;
   const GraphType &G;
 
   typedef DOTGraphTraits<GraphType>           DOTTraits;
   typedef GraphTraits<GraphType>              GTraits;
-  typedef typename GTraits::NodeType          NodeType;
+  typedef typename GTraits::NodeRef           NodeRef;
   typedef typename GTraits::nodes_iterator    node_iterator;
   typedef typename GTraits::ChildIteratorType child_iterator;
   DOTTraits DTraits;
 
+  static_assert(std::is_pointer<NodeRef>::value,
+                "FIXME: Currently GraphWriter requires the NodeRef type to be "
+                "a pointer.\nThe pointer usage should be moved to "
+                "DOTGraphTraits, and removed from GraphWriter itself.");
+
   // Writes the edge labels of the node to O and returns true if there are any
   // edge labels not equal to the empty string "".
-  bool getEdgeSourceLabels(raw_ostream &O, NodeType *Node) {
+  bool getEdgeSourceLabels(raw_ostream &O, NodeRef Node) {
     child_iterator EI = GTraits::child_begin(Node);
     child_iterator EE = GTraits::child_end(Node);
     bool hasEdgeSourceLabels = false;
 
     for (unsigned i = 0; EI != EE && i != 64; ++EI, ++i) {
       std::string label = DTraits.getEdgeSourceLabel(Node, EI);
 
       if (label.empty())
void writeFooter() {
     // Finish off the graph
     O << "}\n";
   }
 
   void writeNodes() {
     // Loop over the graph, printing it out...
     for (node_iterator I = GTraits::nodes_begin(G), E = GTraits::nodes_end(G);
          I != E; ++I)
-      if (!isNodeHidden(*I))
-        writeNode(*I);
-  }
-
-  bool isNodeHidden(NodeType &Node) {
-    return isNodeHidden(&Node);
+      if (!isNodeHidden(&*I))
+        writeNode(&*I);
   }
 
-  bool isNodeHidden(NodeType *const *Node) {
+  bool isNodeHidden(NodeRef const *Node) {
     return isNodeHidden(*Node);
   }
 
-  bool isNodeHidden(NodeType *Node) {
+  bool isNodeHidden(NodeRef Node) {
     return DTraits.isNodeHidden(Node);
   }
 
-  void writeNode(NodeType& Node) {
-    writeNode(&Node);
-  }
-
-  void writeNode(NodeType *const *Node) {
+  void writeNode(NodeRef const *Node) {
     writeNode(*Node);
   }
 
-  void writeNode(NodeType *Node) {
+  void writeNode(NodeRef Node) {
     std::string NodeAttributes = DTraits.getNodeAttributes(Node, G);
 
     O << "\tNode" << static_cast<const void*>(Node) << " [shape=record,";
     if (!NodeAttributes.empty()) O << NodeAttributes << ",";
     O << "label=\"{";
 
     if (!DTraits.renderGraphFromBottomUp()) {
       O << DOT::EscapeString(DTraits.getNodeLabel(Node, G));
void writeNode(NodeRef Node) {
     for (unsigned i = 0; EI != EE && i != 64; ++EI, ++i)
       if (!DTraits.isNodeHidden(*EI))
         writeEdge(Node, i, EI);
     for (; EI != EE; ++EI)
       if (!DTraits.isNodeHidden(*EI))
         writeEdge(Node, 64, EI);
   }
 
-  void writeEdge(NodeType *Node, unsigned edgeidx, child_iterator EI) {
-    if (NodeType *TargetNode = *EI) {
+  void writeEdge(NodeRef Node, unsigned edgeidx, child_iterator EI) {
+    if (NodeRef TargetNode = *EI) {
       int DestPort = -1;
       if (DTraits.edgeTargetsEdgeSource(Node, EI)) {
         child_iterator TargetIt = DTraits.getEdgeTarget(Node, EI);
 
         // Figure out which edge this targets...
         unsigned Offset =
           (unsigned)std::distance(GTraits::child_begin(TargetNode), TargetIt);
         DestPort = static_cast<int>(Offset);

llvm/trunk/lib/Analysis/BlockFrequencyInfo.cpp

ViewHotFreqPercent("view-hot-freq-percent", cl::init(10), cl::Hidden,
                                 "is no less than the max frequency of the "
                                 "function multiplied by this percent."));
 
 namespace llvm {
 
 template <>
 struct GraphTraits<BlockFrequencyInfo *> {
   typedef const BasicBlock NodeType;
+  typedef const BasicBlock *NodeRef;
   typedef succ_const_iterator ChildIteratorType;
   typedef Function::const_iterator nodes_iterator;
 
   static inline const NodeType *getEntryNode(const BlockFrequencyInfo *G) {
     return &G->getFunction()->front();
   }
   static ChildIteratorType child_begin(const NodeType *N) {
     return succ_begin(N);

llvm/trunk/lib/CodeGen/MachineBlockFrequencyInfo.cpp

 
 extern cl::opt<std::string> ViewBlockFreqFuncName;
 extern cl::opt<unsigned> ViewHotFreqPercent;
 
 namespace llvm {
 
 template <> struct GraphTraits<MachineBlockFrequencyInfo *> {
   typedef const MachineBasicBlock NodeType;
+  typedef const MachineBasicBlock *NodeRef;
   typedef MachineBasicBlock::const_succ_iterator ChildIteratorType;
   typedef MachineFunction::const_iterator nodes_iterator;
 
   static inline const NodeType *
   getEntryNode(const MachineBlockFrequencyInfo *G) {
     return &G->getFunction()->front();
   }