[llvm] r226028 - unique_ptrify the value in DominatorTreeBase::DomTreeNodes

Adrian Prantl aprantl at apple.com
Wed Jan 14 15:21:57 PST 2015


Could you add an
#include "llvm/ADT/SmallVector.h”
for llvm::make_unique()?

thanks!
-- adrian

> On Jan 14, 2015, at 11:55 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> Author: dblaikie
> Date: Wed Jan 14 13:55:27 2015
> New Revision: 226028
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=226028&view=rev
> Log:
> unique_ptrify the value in DominatorTreeBase::DomTreeNodes
> 
> (noticed the need for an explicit dtor in a recent commit & figured I'd
> tidy that up)
> 
> Modified:
>    llvm/trunk/include/llvm/Support/GenericDomTree.h
>    llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
> 
> Modified: llvm/trunk/include/llvm/Support/GenericDomTree.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTree.h?rev=226028&r1=226027&r2=226028&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/GenericDomTree.h (original)
> +++ llvm/trunk/include/llvm/Support/GenericDomTree.h Wed Jan 14 13:55:27 2015
> @@ -93,8 +93,9 @@ public:
>   DomTreeNodeBase(NodeT *BB, DomTreeNodeBase<NodeT> *iDom)
>       : TheBB(BB), IDom(iDom), DFSNumIn(-1), DFSNumOut(-1) {}
> 
> -  DomTreeNodeBase<NodeT> *addChild(DomTreeNodeBase<NodeT> *C) {
> -    Children.push_back(C);
> +  std::unique_ptr<DomTreeNodeBase<NodeT>>
> +  addChild(std::unique_ptr<DomTreeNodeBase<NodeT>> C) {
> +    Children.push_back(C.get());
>     return C;
>   }
> 
> @@ -210,7 +211,8 @@ template <class NodeT> class DominatorTr
>   }
> 
> protected:
> -  typedef DenseMap<NodeT *, DomTreeNodeBase<NodeT> *> DomTreeNodeMapType;
> +  typedef DenseMap<NodeT *, std::unique_ptr<DomTreeNodeBase<NodeT>>>
> +      DomTreeNodeMapType;
>   DomTreeNodeMapType DomTreeNodes;
>   DomTreeNodeBase<NodeT> *RootNode;
> 
> @@ -235,10 +237,6 @@ protected:
>   DenseMap<NodeT *, InfoRec> Info;
> 
>   void reset() {
> -    for (typename DomTreeNodeMapType::iterator I = this->DomTreeNodes.begin(),
> -                                               E = DomTreeNodes.end();
> -         I != E; ++I)
> -      delete I->second;
>     DomTreeNodes.clear();
>     IDoms.clear();
>     this->Roots.clear();
> @@ -314,7 +312,6 @@ protected:
> public:
>   explicit DominatorTreeBase(bool isPostDom)
>       : DominatorBase<NodeT>(isPostDom), DFSInfoValid(false), SlowQueries(0) {}
> -  ~DominatorTreeBase() { reset(); }
> 
>   DominatorTreeBase(DominatorTreeBase &&Arg)
>       : DominatorBase<NodeT>(
> @@ -358,10 +355,10 @@ public:
>       if (OI == OtherDomTreeNodes.end())
>         return true;
> 
> -      DomTreeNodeBase<NodeT> *MyNd = I->second;
> -      DomTreeNodeBase<NodeT> *OtherNd = OI->second;
> +      DomTreeNodeBase<NodeT> &MyNd = *I->second;
> +      DomTreeNodeBase<NodeT> &OtherNd = *OI->second;
> 
> -      if (MyNd->compare(OtherNd))
> +      if (MyNd.compare(&OtherNd))
>         return true;
>     }
> 
> @@ -374,7 +371,10 @@ public:
>   /// block.  This is the same as using operator[] on this class.
>   ///
>   DomTreeNodeBase<NodeT> *getNode(NodeT *BB) const {
> -    return DomTreeNodes.lookup(BB);
> +    auto I = DomTreeNodes.find(BB);
> +    if (I != DomTreeNodes.end())
> +      return I->second.get();
> +    return nullptr;
>   }
> 
>   DomTreeNodeBase<NodeT> *operator[](NodeT *BB) const { return getNode(BB); }
> @@ -555,8 +555,8 @@ public:
>     DomTreeNodeBase<NodeT> *IDomNode = getNode(DomBB);
>     assert(IDomNode && "Not immediate dominator specified for block!");
>     DFSInfoValid = false;
> -    return DomTreeNodes[BB] =
> -               IDomNode->addChild(new DomTreeNodeBase<NodeT>(BB, IDomNode));
> +    return (DomTreeNodes[BB] = IDomNode->addChild(
> +                llvm::make_unique<DomTreeNodeBase<NodeT>>(BB, IDomNode))).get();
>   }
> 
>   /// changeImmediateDominator - This method is used to update the dominator
> @@ -593,15 +593,6 @@ public:
>     }
> 
>     DomTreeNodes.erase(BB);
> -    delete Node;
> -  }
> -
> -  /// removeNode - Removes a node from the dominator tree.  Block must not
> -  /// dominate any other blocks.  Invalidates any node pointing to removed
> -  /// block.
> -  void removeNode(NodeT *BB) {
> -    assert(getNode(BB) && "Removing node that isn't in dominator tree.");
> -    DomTreeNodes.erase(BB);
>   }
> 
>   /// splitBlock - BB is split and now it has one successor. Update dominator
> @@ -703,8 +694,8 @@ protected:
> 
>     // Add a new tree node for this NodeT, and link it as a child of
>     // IDomNode
> -    DomTreeNodeBase<NodeT> *C = new DomTreeNodeBase<NodeT>(BB, IDomNode);
> -    return this->DomTreeNodes[BB] = IDomNode->addChild(C);
> +    return (this->DomTreeNodes[BB] = IDomNode->addChild(
> +                llvm::make_unique<DomTreeNodeBase<NodeT>>(BB, IDomNode))).get();
>   }
> 
>   NodeT *getIDom(NodeT *BB) const { return IDoms.lookup(BB); }
> 
> Modified: llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h?rev=226028&r1=226027&r2=226028&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h (original)
> +++ llvm/trunk/include/llvm/Support/GenericDomTreeConstruction.h Wed Jan 14 13:55:27 2015
> @@ -251,15 +251,17 @@ void Calculate(DominatorTreeBase<typenam
>   // an infinite loop.
>   typename GraphT::NodeType* Root = !MultipleRoots ? DT.Roots[0] : nullptr;
> 
> -  DT.DomTreeNodes[Root] = DT.RootNode =
> -                  new DomTreeNodeBase<typename GraphT::NodeType>(Root, nullptr);
> +  DT.RootNode =
> +      (DT.DomTreeNodes[Root] =
> +           llvm::make_unique<DomTreeNodeBase<typename GraphT::NodeType>>(
> +               Root, nullptr)).get();
> 
>   // Loop over all of the reachable blocks in the function...
>   for (unsigned i = 2; i <= N; ++i) {
>     typename GraphT::NodeType* W = DT.Vertex[i];
> 
> -    DomTreeNodeBase<typename GraphT::NodeType> *BBNode = DT.DomTreeNodes[W];
> -    if (BBNode) continue;  // Haven't calculated this node yet?
> +    if (DT.DomTreeNodes[W])
> +      continue; // Haven't calculated this node yet?
> 
>     typename GraphT::NodeType* ImmDom = DT.getIDom(W);
> 
> @@ -271,15 +273,16 @@ void Calculate(DominatorTreeBase<typenam
> 
>     // Add a new tree node for this BasicBlock, and link it as a child of
>     // IDomNode
> -    DomTreeNodeBase<typename GraphT::NodeType> *C =
> -                    new DomTreeNodeBase<typename GraphT::NodeType>(W, IDomNode);
> -    DT.DomTreeNodes[W] = IDomNode->addChild(C);
> +    DT.DomTreeNodes[W] = IDomNode->addChild(
> +        llvm::make_unique<DomTreeNodeBase<typename GraphT::NodeType>>(
> +            W, IDomNode));
>   }
> 
>   // Free temporary memory used to construct idom's
>   DT.IDoms.clear();
>   DT.Info.clear();
> -  std::vector<typename GraphT::NodeType*>().swap(DT.Vertex);
> +  DT.Vertex.clear();
> +  DT.Vertex.shrink_to_fit();
> 
>   DT.updateDFSNumbers();
> }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list