[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