[llvm] r225966 - [PM] Make DominatorTrees (corectly) movable so that we can move them

[Mehdi Amini]

Hi Chandler,

> On Jan 14, 2015, at 2:07 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> 
> Author: chandlerc
> Date: Wed Jan 14 04:07:19 2015
> New Revision: 225966
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=225966&view=rev
> Log:
> [PM] Make DominatorTrees (corectly) movable so that we can move them
> into the new pass manager's analysis cache which stores results
> by-value.
> 
> Technically speaking, the dom trees were originally not movable but
> copyable! This, unsurprisingly, didn't work at all -- the copy was
> shallow and just resulted in rampant memory corruption. This change
> explicitly forbids copying (as it would need to be a deep copy) and
> makes them explicitly movable with the unsurprising boiler plate to
> member-wise move them because we can't rely on MSVC to generate this
> code for us. =/

Maybe a online comment can be relevant in general when you write unusual boilerplate for MSVC (or other)?
Just so that some who read the code later won’t think that this was just useless in the first place.
And if a new version of MSVC has a better support and we raise the version requirement for LLVM, it can help to grep MSVC in the code to cleanup.

--
Mehdi



> 
> Modified:
>    llvm/trunk/include/llvm/IR/Dominators.h
>    llvm/trunk/include/llvm/Support/GenericDomTree.h
> 
> Modified: llvm/trunk/include/llvm/IR/Dominators.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Dominators.h?rev=225966&r1=225965&r2=225966&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/Dominators.h (original)
> +++ llvm/trunk/include/llvm/IR/Dominators.h Wed Jan 14 04:07:19 2015
> @@ -69,6 +69,13 @@ public:
> 
>   DominatorTree() : DominatorTreeBase<BasicBlock>(false) {}
> 
> +  DominatorTree(DominatorTree &&Arg)
> +      : Base(std::move(static_cast<Base &>(Arg))) {}
> +  DominatorTree &operator=(DominatorTree &&RHS) {
> +    Base::operator=(std::move(static_cast<Base &>(RHS)));
> +    return *this;
> +  }
> +
>   /// \brief Returns *false* if the other dominator tree matches this dominator
>   /// tree.
>   inline bool compare(const DominatorTree &Other) const {
> 
> Modified: llvm/trunk/include/llvm/Support/GenericDomTree.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTree.h?rev=225966&r1=225965&r2=225966&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/GenericDomTree.h (original)
> +++ llvm/trunk/include/llvm/Support/GenericDomTree.h Wed Jan 14 04:07:19 2015
> @@ -34,9 +34,20 @@ namespace llvm {
> template <class NodeT> class DominatorBase {
> protected:
>   std::vector<NodeT *> Roots;
> -  const bool IsPostDominators;
> +  bool IsPostDominators;
>   explicit DominatorBase(bool isPostDom)
>       : Roots(), IsPostDominators(isPostDom) {}
> +  DominatorBase(DominatorBase &&Arg)
> +      : Roots(std::move(Arg.Roots)),
> +        IsPostDominators(std::move(Arg.IsPostDominators)) {
> +    Arg.Roots.clear();
> +  }
> +  DominatorBase &operator=(DominatorBase &&RHS) {
> +    Roots = std::move(RHS.Roots);
> +    IsPostDominators = std::move(RHS.IsPostDominators);
> +    RHS.Roots.clear();
> +    return *this;
> +  }
> 
> public:
>   /// getRoots - Return the root blocks of the current CFG.  This may include
> @@ -171,6 +182,9 @@ void Calculate(DominatorTreeBase<typenam
> /// This class is a generic template over graph nodes. It is instantiated for
> /// various graphs in the LLVM IR or in the code generator.
> template <class NodeT> class DominatorTreeBase : public DominatorBase<NodeT> {
> +  DominatorTreeBase(const DominatorTreeBase &) LLVM_DELETED_FUNCTION;
> +  DominatorTreeBase &operator=(const DominatorTreeBase &) LLVM_DELETED_FUNCTION;
> +
>   bool dominatedBySlowTreeWalk(const DomTreeNodeBase<NodeT> *A,
>                                const DomTreeNodeBase<NodeT> *B) const {
>     assert(A != B);
> @@ -183,6 +197,18 @@ template <class NodeT> class DominatorTr
>     return IDom != nullptr;
>   }
> 
> +  /// \brief Wipe this tree's state without releasing any resources.
> +  ///
> +  /// This is essentially a post-move helper only. It leaves the object in an
> +  /// assignable and destroyable state, but otherwise invalid.
> +  void wipe() {
> +    DomTreeNodes.clear();
> +    IDoms.clear();
> +    Vertex.clear();
> +    Info.clear();
> +    RootNode = nullptr;
> +  }
> +
> protected:
>   typedef DenseMap<NodeT *, DomTreeNodeBase<NodeT> *> DomTreeNodeMapType;
>   DomTreeNodeMapType DomTreeNodes;
> @@ -290,6 +316,30 @@ public:
>       : DominatorBase<NodeT>(isPostDom), DFSInfoValid(false), SlowQueries(0) {}
>   virtual ~DominatorTreeBase() { reset(); }
> 
> +  DominatorTreeBase(DominatorTreeBase &&Arg)
> +      : DominatorBase<NodeT>(
> +            std::move(static_cast<DominatorBase<NodeT> &>(Arg))),
> +        DomTreeNodes(std::move(Arg.DomTreeNodes)),
> +        RootNode(std::move(Arg.RootNode)),
> +        DFSInfoValid(std::move(Arg.DFSInfoValid)),
> +        SlowQueries(std::move(Arg.SlowQueries)), IDoms(std::move(Arg.IDoms)),
> +        Vertex(std::move(Arg.Vertex)), Info(std::move(Arg.Info)) {
> +    Arg.wipe();
> +  }
> +  DominatorTreeBase &operator=(DominatorTreeBase &&RHS) {
> +    DominatorBase<NodeT>::operator=(
> +        std::move(static_cast<DominatorBase<NodeT> &>(RHS)));
> +    DomTreeNodes = std::move(RHS.DomTreeNodes);
> +    RootNode = std::move(RHS.RootNode);
> +    DFSInfoValid = std::move(RHS.DFSInfoValid);
> +    SlowQueries = std::move(RHS.SlowQueries);
> +    IDoms = std::move(RHS.IDoms);
> +    Vertex = std::move(RHS.Vertex);
> +    Info = std::move(RHS.Info);
> +    RHS.wipe();
> +    return *this;
> +  }
> +
>   /// compare - Return false if the other dominator tree base matches this
>   /// dominator tree base. Otherwise return true.
>   bool compare(const DominatorTreeBase &Other) const {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits