[PATCH] D23625: [Analysis] Change several Analysis pieces to use NodeRef. NFC.

[David Blaikie via llvm-commits]

dblaikie added inline comments.

================
Comment at: include/llvm/Analysis/BlockFrequencyInfoImpl.h:1274
@@ -1274,3 +1273,3 @@
            I != E; ++I) {
-        NodeType &N = *I;
+        NodeRef N = &*I;
         MaxFrequency =
----------------
I'm confused by this '&'.

If GraphTraits is meant to be node handle agnostic, why is it producing a dereferenced node handle type here? (such that the address needs to be taken again)

================
Comment at: include/llvm/Analysis/RegionIterator.h:53
@@ -52,3 +52,3 @@
   // Use two bit to represent the mode iterator.
-  PointerIntPair<NodeType*, 2, ItMode> Node;
+  PointerIntPair<NodeRef, 2, ItMode> Node;
 
----------------
This would still be restricted to pointers - so it doesn't actually generalize over the new node handles you are introducing.

Does this need deeper changes?

================
Comment at: include/llvm/Analysis/RegionIterator.h:277-305
@@ -277,30 +276,31 @@
 
-#define RegionGraphTraits(RegionT, NodeT) \
-template<> struct GraphTraits<RegionT*> \
-  : public GraphTraits<NodeT*> { \
-  typedef df_iterator<NodeType*> nodes_iterator; \
-  static NodeType *getEntryNode(RegionT* R) { \
-    return R->getNode(R->getEntry()); \
-  } \
-  static nodes_iterator nodes_begin(RegionT* R) { \
-    return nodes_iterator::begin(getEntryNode(R)); \
-  } \
-  static nodes_iterator nodes_end(RegionT* R) { \
-    return nodes_iterator::end(getEntryNode(R)); \
-  } \
-}; \
-template<> struct GraphTraits<FlatIt<RegionT*> > \
-  : public GraphTraits<FlatIt<NodeT*> > { \
-  typedef df_iterator<NodeType*, SmallPtrSet<NodeType*, 8>, false, \
-  GraphTraits<FlatIt<NodeType*> > > nodes_iterator; \
-  static NodeType *getEntryNode(RegionT* R) { \
-    return R->getBBNode(R->getEntry()); \
-  } \
-  static nodes_iterator nodes_begin(RegionT* R) { \
-    return nodes_iterator::begin(getEntryNode(R)); \
-  } \
-  static nodes_iterator nodes_end(RegionT* R) { \
-    return nodes_iterator::end(getEntryNode(R)); \
-  } \
-}
+#define RegionGraphTraits(RegionT, NodeT)                                      \
+  template <> struct GraphTraits<RegionT *> : public GraphTraits<NodeT *> {    \
+    typedef df_iterator<NodeRef> nodes_iterator;                               \
+    static NodeRef getEntryNode(RegionT *R) {                                  \
+      return R->getNode(R->getEntry());                                        \
+    }                                                                          \
+    static nodes_iterator nodes_begin(RegionT *R) {                            \
+      return nodes_iterator::begin(getEntryNode(R));                           \
+    }                                                                          \
+    static nodes_iterator nodes_end(RegionT *R) {                              \
+      return nodes_iterator::end(getEntryNode(R));                             \
+    }                                                                          \
+  };                                                                           \
+  template <>                                                                  \
+  struct GraphTraits<FlatIt<RegionT *>>                                        \
+      : public GraphTraits<FlatIt<NodeT *>> {                                  \
+    typedef df_iterator<NodeRef, SmallPtrSet<NodeRef, 8>, false,               \
+                        GraphTraits<FlatIt<NodeRef>>>                          \
+        nodes_iterator;                                                        \
+    static NodeRef getEntryNode(RegionT *R) {                                  \
+      return R->getBBNode(R->getEntry());                                      \
+    }                                                                          \
+    static nodes_iterator nodes_begin(RegionT *R) {                            \
+      return nodes_iterator::begin(getEntryNode(R));                           \
+    }                                                                          \
+    static nodes_iterator nodes_end(RegionT *R) {                              \
+      return nodes_iterator::end(getEntryNode(R));                             \
+    }                                                                          \
+  }
 
----------------
Might be worth reformatting this in a separate preliminary commit (no review required, jsut mash clang-format on that, commit, then resolve the changes against this commit/code review)


https://reviews.llvm.org/D23625