[PATCH] Add control dependence computation that computes control dependence equivalence classes.

Chandler Carruth chandlerc at gmail.com
Wed May 27 02:29:28 PDT 2015


================
Comment at: include/llvm/Analysis/ControlDependenceEquivalence.h:1-14
@@ +1,15 @@
+//===- ControlDependenceEquivalence.h - Compute Control Equivalence-------*- C++
+//-*---===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// \file
+// \brief Determines control dependence equivalence classes for basic
+// blocks. Any two blocks having the same set of control dependences land in one
+// class.
+// ===---------------------------------------------------------------------===//
+
----------------
This header block needs lots of fixes...

================
Comment at: include/llvm/Analysis/ControlDependenceEquivalence.h:32
@@ +31,3 @@
+/// \brief Compute control dependence equivalence classes for all nodes of the
+/// CFG.
+/// Note that this implementation actually uses cycle equivalence to establish
----------------
Need a line break here. You can omit the '\brief' now apparently, but I don't care either way.

================
Comment at: include/llvm/Analysis/ControlDependenceEquivalence.h:96-101
@@ +95,8 @@
+  class const_combined_iterator
+      : public std::iterator<std::forward_iterator_tag, const BasicBlock,
+                             ptrdiff_t, const BasicBlock *,
+                             const BasicBlock *> {
+    typedef std::iterator<std::forward_iterator_tag, const BasicBlock *,
+                          ptrdiff_t, const BasicBlock *,
+                          const BasicBlock *> super;
+
----------------
I suggest using include/llvm/ADT/iterator.h and iterator_facade_base to stub out much of the boiler plate here. Also, I would use 'BaseT' rather than 'super'.

================
Comment at: include/llvm/Analysis/ControlDependenceEquivalence.h:109
@@ +108,3 @@
+    // iterator too
+    inline const_combined_iterator(RealType Begin, RealType End,
+                                   FakeType FBegin, FakeType FEnd)
----------------
No need for the inline keyword.

================
Comment at: include/llvm/Analysis/ControlDependenceEquivalence.h:248
@@ +247,3 @@
+  unsigned ClassNumber;
+  SmallDenseMap<const BasicBlock *, BlockData, 8> BlockInfo;
+  bool Computed;
----------------
BlockData is *huge*. It doesn't belong in a densemap.

I think you want a DenseMap for the class numbers, and maybe some of the other *very* hot data, and a separate DenseMap<..., std::unique_ptr<BlockData>> for the rest.

================
Comment at: lib/Analysis/ControlDependenceEquivalence.cpp:11
@@ +10,3 @@
+/// classes.
+//===----------------------------------------------------------------------===//
+#include "llvm/Analysis/ControlDependenceEquivalence.h"
----------------
Missing line break.

================
Comment at: lib/Analysis/ControlDependenceEquivalence.cpp:327
@@ +326,3 @@
+  if (Info.Backedges.size() > 0) {
+    auto hi0iter = std::max_element(
+        Info.Backedges.begin(), Info.Backedges.end(),
----------------
'hi0iter'?

================
Comment at: lib/Analysis/ControlDependenceEquivalence.cpp:354-356
@@ +353,5 @@
+
+      // This is the original algorithm's version of this
+      // // Get the lowest two elements by DFS number, which in turn are the
+      // // highest two points in the tree.
+      // std::nth_element(Info.Children.begin(), Info.Children.begin() + 1,
----------------
Why is this interesting?

http://reviews.llvm.org/D8568

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list