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

Adrian Prantl aprantl at apple.com
Wed Jan 14 15:22:45 PST 2015


> On Jan 14, 2015, at 3:21 PM, Adrian Prantl <aprantl at apple.com> wrote:
> 
> Could you add an
> #include "llvm/ADT/SmallVector.h”
Copy&Pasto. I meant to write
#include “llvm/ADT/STLExtras.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