<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 14, 2015 at 2:07 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: chandlerc<br>
Date: Wed Jan 14 04:07:19 2015<br>
New Revision: 225966<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225966&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225966&view=rev</a><br>
Log:<br>
[PM] Make DominatorTrees (corectly) movable so that we can move them<br>
into the new pass manager's analysis cache which stores results<br>
by-value.<br>
<br>
Technically speaking, the dom trees were originally not movable but<br>
copyable! This, unsurprisingly, didn't work at all -- the copy was<br>
shallow and just resulted in rampant memory corruption. This change<br>
explicitly forbids copying (as it would need to be a deep copy) and<br>
makes them explicitly movable with the unsurprising boiler plate to<br>
member-wise move them because we can't rely on MSVC to generate this<br>
code for us. =/<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/IR/Dominators.h<br>
llvm/trunk/include/llvm/Support/GenericDomTree.h<br>
<br>
Modified: llvm/trunk/include/llvm/IR/Dominators.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Dominators.h?rev=225966&r1=225965&r2=225966&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Dominators.h?rev=225966&r1=225965&r2=225966&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/IR/Dominators.h (original)<br>
+++ llvm/trunk/include/llvm/IR/Dominators.h Wed Jan 14 04:07:19 2015<br>
@@ -69,6 +69,13 @@ public:<br>
<br>
DominatorTree() : DominatorTreeBase<BasicBlock>(false) {}<br>
<br>
+ DominatorTree(DominatorTree &&Arg)<br>
+ : Base(std::move(static_cast<Base &>(Arg))) {}<br>
+ DominatorTree &operator=(DominatorTree &&RHS) {<br>
+ Base::operator=(std::move(static_cast<Base &>(RHS)));<br>
+ return *this;<br>
+ }<br>
+<br>
/// \brief Returns *false* if the other dominator tree matches this dominator<br>
/// tree.<br>
inline bool compare(const DominatorTree &Other) const {<br>
<br>
Modified: llvm/trunk/include/llvm/Support/GenericDomTree.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTree.h?rev=225966&r1=225965&r2=225966&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTree.h?rev=225966&r1=225965&r2=225966&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Support/GenericDomTree.h (original)<br>
+++ llvm/trunk/include/llvm/Support/GenericDomTree.h Wed Jan 14 04:07:19 2015<br>
@@ -34,9 +34,20 @@ namespace llvm {<br>
template <class NodeT> class DominatorBase {<br>
protected:<br>
std::vector<NodeT *> Roots;<br>
- const bool IsPostDominators;<br>
+ bool IsPostDominators;<br>
explicit DominatorBase(bool isPostDom)<br>
: Roots(), IsPostDominators(isPostDom) {}<br>
+ DominatorBase(DominatorBase &&Arg)<br>
+ : Roots(std::move(Arg.Roots)),<br>
+ IsPostDominators(std::move(Arg.IsPostDominators)) {<br>
+ Arg.Roots.clear();<br>
+ }<br>
+ DominatorBase &operator=(DominatorBase &&RHS) {<br>
+ Roots = std::move(RHS.Roots);<br>
+ IsPostDominators = std::move(RHS.IsPostDominators);<br>
+ RHS.Roots.clear();<br>
+ return *this;<br>
+ }<br>
<br>
public:<br>
/// getRoots - Return the root blocks of the current CFG. This may include<br>
@@ -171,6 +182,9 @@ void Calculate(DominatorTreeBase<typenam<br>
/// This class is a generic template over graph nodes. It is instantiated for<br>
/// various graphs in the LLVM IR or in the code generator.<br>
template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {<br>
+ DominatorTreeBase(const DominatorTreeBase &) LLVM_DELETED_FUNCTION;<br>
+ DominatorTreeBase &operator=(const DominatorTreeBase &) LLVM_DELETED_FUNCTION;<br></blockquote><div><br>This should be implicit, since it has a base that's non-copyable.<br><br>But, yes, since the non-copyability only comes from the need for explicit move operations which only comes from MSVC's limitations (& will hopefully be removed once we drop support for whatever versions of MSVC don't create implicit move ops)... - not sure if it makes more sense to put it here, or up in the base which owns the container of nodes (but the base doesn't know what kind of nodes they are/doesn't require those nodes to point to other nodes, etc, which is where the non-copyability comes in)<br><br>Sorry, kind of rambling, maybe chat about it over lunch or something.<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
bool dominatedBySlowTreeWalk(const DomTreeNodeBase<NodeT> *A,<br>
const DomTreeNodeBase<NodeT> *B) const {<br>
assert(A != B);<br>
@@ -183,6 +197,18 @@ template <class NodeT> class DominatorTr<br>
return IDom != nullptr;<br>
}<br>
<br>
+ /// \brief Wipe this tree's state without releasing any resources.<br>
+ ///<br>
+ /// This is essentially a post-move helper only. It leaves the object in an<br>
+ /// assignable and destroyable state, but otherwise invalid.<br>
+ void wipe() {<br>
+ DomTreeNodes.clear();<br>
+ IDoms.clear();<br>
+ Vertex.clear();<br>
+ Info.clear();<br>
+ RootNode = nullptr;<br>
+ }<br>
+<br>
protected:<br>
typedef DenseMap<NodeT *, DomTreeNodeBase<NodeT> *> DomTreeNodeMapType;<br>
DomTreeNodeMapType DomTreeNodes;<br>
@@ -290,6 +316,30 @@ public:<br>
: DominatorBase<NodeT>(isPostDom), DFSInfoValid(false), SlowQueries(0) {}<br>
virtual ~DominatorTreeBase() { reset(); }<br>
<br>
+ DominatorTreeBase(DominatorTreeBase &&Arg)<br>
+ : DominatorBase<NodeT>(<br>
+ std::move(static_cast<DominatorBase<NodeT> &>(Arg))),<br>
+ DomTreeNodes(std::move(Arg.DomTreeNodes)),<br>
+ RootNode(std::move(Arg.RootNode)),<br>
+ DFSInfoValid(std::move(Arg.DFSInfoValid)),<br>
+ SlowQueries(std::move(Arg.SlowQueries)), IDoms(std::move(Arg.IDoms)),<br>
+ Vertex(std::move(Arg.Vertex)), Info(std::move(Arg.Info)) {<br>
+ Arg.wipe();<br>
+ }<br>
+ DominatorTreeBase &operator=(DominatorTreeBase &&RHS) {<br>
+ DominatorBase<NodeT>::operator=(<br>
+ std::move(static_cast<DominatorBase<NodeT> &>(RHS)));<br>
+ DomTreeNodes = std::move(RHS.DomTreeNodes);<br>
+ RootNode = std::move(RHS.RootNode);<br>
+ DFSInfoValid = std::move(RHS.DFSInfoValid);<br>
+ SlowQueries = std::move(RHS.SlowQueries);<br>
+ IDoms = std::move(RHS.IDoms);<br>
+ Vertex = std::move(RHS.Vertex);<br>
+ Info = std::move(RHS.Info);<br>
+ RHS.wipe();<br>
+ return *this;<br>
+ }<br>
+<br>
/// compare - Return false if the other dominator tree base matches this<br>
/// dominator tree base. Otherwise return true.<br>
bool compare(const DominatorTreeBase &Other) const {<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>