[llvm] r222183 - Improve memory ownership/management in TableGen by unique_ptrifying TreePattern's Tree member.

David Blaikie dblaikie at gmail.com
Mon Nov 17 14:16:55 PST 2014


Author: dblaikie
Date: Mon Nov 17 16:16:55 2014
New Revision: 222183

URL: http://llvm.org/viewvc/llvm-project?rev=222183&view=rev
Log:
Improve memory ownership/management in TableGen by unique_ptrifying TreePattern's Tree member.

The next step is to actually use unique_ptr in TreePatternNode's
Children vector. That will be more intrusive, and may not work,
depending on exactly how these things are handled (I have a bad
suspicion things are shared more than they should be, making this more
DAG than tree - but if it's really a tree, unique_ptr should suffice)

Modified:
    llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp
    llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h

Modified: llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp?rev=222183&r1=222182&r2=222183&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp (original)
+++ llvm/trunk/utils/TableGen/CodeGenDAGPatterns.cpp Mon Nov 17 16:16:55 2014
@@ -1902,10 +1902,10 @@ TreePattern::TreePattern(Record *TheRec,
   Trees.push_back(ParseTreePattern(Pat, ""));
 }
 
-TreePattern::TreePattern(Record *TheRec, TreePatternNode *Pat, bool isInput,
+TreePattern::TreePattern(Record *TheRec, std::unique_ptr<TreePatternNode> Pat, bool isInput,
                          CodeGenDAGPatterns &cdp) : TheRecord(TheRec), CDP(cdp),
                          isInputPattern(isInput), HasError(false) {
-  Trees.push_back(Pat);
+  Trees.push_back(std::move(Pat));
 }
 
 void TreePattern::error(const Twine &Msg) {
@@ -1918,7 +1918,7 @@ void TreePattern::error(const Twine &Msg
 
 void TreePattern::ComputeNamedNodes() {
   for (unsigned i = 0, e = Trees.size(); i != e; ++i)
-    ComputeNamedNodes(Trees[i]);
+    ComputeNamedNodes(Trees[i].get());
 }
 
 void TreePattern::ComputeNamedNodes(TreePatternNode *N) {
@@ -1929,8 +1929,8 @@ void TreePattern::ComputeNamedNodes(Tree
     ComputeNamedNodes(N->getChild(i));
 }
 
-
-TreePatternNode *TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName){
+std::unique_ptr<TreePatternNode>
+TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName) {
   if (DefInit *DI = dyn_cast<DefInit>(TheInit)) {
     Record *R = DI->getDef();
 
@@ -1944,7 +1944,7 @@ TreePatternNode *TreePattern::ParseTreeP
         OpName);
 
     // Input argument?
-    TreePatternNode *Res = new TreePatternNode(DI, 1);
+    auto Res = llvm::make_unique<TreePatternNode>(DI, 1);
     if (R->getName() == "node" && !OpName.empty()) {
       if (OpName.empty())
         error("'node' argument requires a name to match with operand list");
@@ -1959,7 +1959,7 @@ TreePatternNode *TreePattern::ParseTreeP
   if (TheInit == UnsetInit::get()) {
     if (OpName.empty())
       error("'?' argument requires a name to match with operand list");
-    TreePatternNode *Res = new TreePatternNode(TheInit, 1);
+    auto Res = llvm::make_unique<TreePatternNode>(TheInit, 1);
     Args.push_back(OpName);
     Res->setName(OpName);
     return Res;
@@ -1968,7 +1968,7 @@ TreePatternNode *TreePattern::ParseTreeP
   if (IntInit *II = dyn_cast<IntInit>(TheInit)) {
     if (!OpName.empty())
       error("Constant int argument should not have a name!");
-    return new TreePatternNode(II, 1);
+    return llvm::make_unique<TreePatternNode>(II, 1);
   }
 
   if (BitsInit *BI = dyn_cast<BitsInit>(TheInit)) {
@@ -1994,7 +1994,7 @@ TreePatternNode *TreePattern::ParseTreeP
     if (Dag->getNumArgs() != 1)
       error("Type cast only takes one operand!");
 
-    TreePatternNode *New = ParseTreePattern(Dag->getArg(0), Dag->getArgName(0));
+    auto New = ParseTreePattern(Dag->getArg(0), Dag->getArgName(0));
 
     // Apply the type cast.
     assert(New->getNumTypes() == 1 && "FIXME: Unhandled");
@@ -2044,7 +2044,8 @@ TreePatternNode *TreePattern::ParseTreeP
 
   // Parse all the operands.
   for (unsigned i = 0, e = Dag->getNumArgs(); i != e; ++i)
-    Children.push_back(ParseTreePattern(Dag->getArg(i), Dag->getArgName(i)));
+    Children.push_back(
+        ParseTreePattern(Dag->getArg(i), Dag->getArgName(i)).release());
 
   // If the operator is an intrinsic, then this is just syntactic sugar for for
   // (intrinsic_* <number>, ..children..).  Pick the right intrinsic node, and
@@ -2089,7 +2090,7 @@ TreePatternNode *TreePattern::ParseTreeP
   }
 
   unsigned NumResults = GetNumNodeResults(Operator, CDP);
-  TreePatternNode *Result = new TreePatternNode(Operator, Children, NumResults);
+  auto Result = llvm::make_unique<TreePatternNode>(Operator, Children, NumResults);
   Result->setName(OpName);
 
   if (!Dag->getName().empty()) {
@@ -2105,7 +2106,7 @@ TreePatternNode *TreePattern::ParseTreeP
 /// more type generic things and have useless type casts fold away.
 ///
 /// This returns true if any change is made.
-static bool SimplifyTree(TreePatternNode *&N) {
+static bool SimplifyTree(std::unique_ptr<TreePatternNode> &N) {
   if (N->isLeaf())
     return false;
 
@@ -2115,7 +2116,7 @@ static bool SimplifyTree(TreePatternNode
       N->getExtType(0).isConcrete() &&
       N->getExtType(0) == N->getChild(0)->getExtType(0) &&
       N->getName().empty()) {
-    N = N->getChild(0);
+    N.reset(N->getChild(0));
     SimplifyTree(N);
     return true;
   }
@@ -2123,9 +2124,9 @@ static bool SimplifyTree(TreePatternNode
   // Walk all children.
   bool MadeChange = false;
   for (unsigned i = 0, e = N->getNumChildren(); i != e; ++i) {
-    TreePatternNode *Child = N->getChild(i);
+    std::unique_ptr<TreePatternNode> Child(N->getChild(i));
     MadeChange |= SimplifyTree(Child);
-    N->setChild(i, Child);
+    N->setChild(i, Child.release());
   }
   return MadeChange;
 }
@@ -2172,7 +2173,7 @@ InferAllTypes(const StringMap<SmallVecto
           // changing the type of the input register in this case.  This allows
           // us to match things like:
           //  def : Pat<(v1i64 (bitconvert(v2i32 DPR:$src))), (v1i64 DPR:$src)>;
-          if (Nodes[i] == Trees[0] && Nodes[i]->isLeaf()) {
+          if (Nodes[i] == Trees[0].get() && Nodes[i]->isLeaf()) {
             DefInit *DI = dyn_cast<DefInit>(Nodes[i]->getLeafValue());
             if (DI && (DI->getDef()->isSubClassOf("RegisterClass") ||
                        DI->getDef()->isSubClassOf("RegisterOperand")))
@@ -2924,26 +2925,27 @@ const DAGInstruction &CodeGenDAGPatterns
       I->error("Input operand $" + InstInputsCheck.begin()->first +
                " occurs in pattern but not in operands list!");
 
-    TreePatternNode *ResultPattern =
-      new TreePatternNode(I->getRecord(), ResultNodeOperands,
-                          GetNumNodeResults(I->getRecord(), *this));
+    auto ResultPattern = llvm::make_unique<TreePatternNode>(
+        I->getRecord(), ResultNodeOperands,
+        GetNumNodeResults(I->getRecord(), *this));
+
     // Copy fully inferred output node type to instruction result pattern.
     for (unsigned i = 0; i != NumResults; ++i)
       ResultPattern->setType(i, Res0Node->getExtType(i));
 
     // Create and insert the instruction.
     // FIXME: InstImpResults should not be part of DAGInstruction.
-    DAGInstruction TheInst(I, Results, Operands, InstImpResults);
-    DAGInsts.insert(std::make_pair(I->getRecord(), TheInst));
+    DAGInsts.insert(std::make_pair(
+        I->getRecord(), DAGInstruction(I, Results, Operands, InstImpResults)));
 
     // Use a temporary tree pattern to infer all types and make sure that the
     // constructed result is correct.  This depends on the instruction already
     // being inserted into the DAGInsts map.
-    TreePattern Temp(I->getRecord(), ResultPattern, false, *this);
+    TreePattern Temp(I->getRecord(), std::move(ResultPattern), false, *this);
     Temp.InferAllTypes(&I->getNamedNodesMap());
 
     DAGInstruction &TheInsertedInst = DAGInsts.find(I->getRecord())->second;
-    TheInsertedInst.setResultPattern(Temp.getOnlyTree());
+    TheInsertedInst.setResultPattern(std::move(Temp.getOnlyTree()));
 
     return TheInsertedInst;
   }
@@ -3375,7 +3377,7 @@ void CodeGenDAGPatterns::ParsePatterns()
                                   InstImpResults);
 
     // Promote the xform function to be an explicit node if set.
-    TreePatternNode *DstPattern = Result.getOnlyTree();
+    auto DstPattern = std::move(Result.getOnlyTree());
     std::vector<TreePatternNode*> ResultNodeOperands;
     for (unsigned ii = 0, ee = DstPattern->getNumChildren(); ii != ee; ++ii) {
       TreePatternNode *OpNode = DstPattern->getChild(ii);
@@ -3387,16 +3389,16 @@ void CodeGenDAGPatterns::ParsePatterns()
       }
       ResultNodeOperands.push_back(OpNode);
     }
-    DstPattern = Result.getOnlyTree();
-    if (!DstPattern->isLeaf())
-      DstPattern = new TreePatternNode(DstPattern->getOperator(),
-                                       ResultNodeOperands,
-                                       DstPattern->getNumTypes());
-
-    for (unsigned i = 0, e = Result.getOnlyTree()->getNumTypes(); i != e; ++i)
-      DstPattern->setType(i, Result.getOnlyTree()->getExtType(i));
+    if (!DstPattern->isLeaf()) {
+      auto NewPattern = llvm::make_unique<TreePatternNode>(
+          DstPattern->getOperator(), ResultNodeOperands,
+          DstPattern->getNumTypes());
+      for (unsigned i = 0, e = DstPattern->getNumTypes(); i != e; ++i)
+        NewPattern->setType(i, DstPattern->getExtType(i));
+      DstPattern = std::move(NewPattern);
+    }
 
-    TreePattern Temp(Result.getRecord(), DstPattern, false, *this);
+    TreePattern Temp(Result.getRecord(), std::move(DstPattern), false, *this);
     Temp.InferAllTypes();
 
 
@@ -3404,7 +3406,7 @@ void CodeGenDAGPatterns::ParsePatterns()
                     PatternToMatch(CurPattern,
                                    CurPattern->getValueAsListInit("Predicates"),
                                    Pattern->getTree(0),
-                                   Temp.getOnlyTree(), InstImpResults,
+                                   Temp.getOnlyTree().release(), InstImpResults,
                                    CurPattern->getValueAsInt("AddedComplexity"),
                                    CurPattern->getID()));
   }

Modified: llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h?rev=222183&r1=222182&r2=222183&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h (original)
+++ llvm/trunk/utils/TableGen/CodeGenDAGPatterns.h Mon Nov 17 16:16:55 2014
@@ -294,7 +294,7 @@ private:
   std::string getPredCode() const;
   std::string getImmCode() const;
 };
-  
+
 
 /// FIXME: TreePatternNode's can be shared in some cases (due to dag-shaped
 /// patterns), and as such should be ref counted.  We currently just leak all
@@ -508,7 +508,7 @@ class TreePattern {
   /// Trees - The list of pattern trees which corresponds to this pattern.
   /// Note that PatFrag's only have a single tree.
   ///
-  std::vector<TreePatternNode*> Trees;
+  std::vector<std::unique_ptr<TreePatternNode>> Trees;
 
   /// NamedNodes - This is all of the nodes that have names in the trees in this
   /// pattern.
@@ -548,15 +548,17 @@ public:
               CodeGenDAGPatterns &ise);
   TreePattern(Record *TheRec, DagInit *Pat, bool isInput,
               CodeGenDAGPatterns &ise);
-  TreePattern(Record *TheRec, TreePatternNode *Pat, bool isInput,
-              CodeGenDAGPatterns &ise);
+  TreePattern(Record *TheRec, std::unique_ptr<TreePatternNode> Pat,
+              bool isInput, CodeGenDAGPatterns &ise);
 
   /// getTrees - Return the tree patterns which corresponds to this pattern.
   ///
-  const std::vector<TreePatternNode*> &getTrees() const { return Trees; }
+  const std::vector<std::unique_ptr<TreePatternNode>> &getTrees() const {
+    return Trees;
+  }
   unsigned getNumTrees() const { return Trees.size(); }
-  TreePatternNode *getTree(unsigned i) const { return Trees[i]; }
-  TreePatternNode *getOnlyTree() const {
+  TreePatternNode *getTree(unsigned i) const { return Trees[i].get(); }
+  std::unique_ptr<TreePatternNode> &getOnlyTree() {
     assert(Trees.size() == 1 && "Doesn't have exactly one pattern!");
     return Trees[0];
   }
@@ -586,7 +588,8 @@ public:
   /// PatFrag references.
   void InlinePatternFragments() {
     for (unsigned i = 0, e = Trees.size(); i != e; ++i)
-      Trees[i] = Trees[i]->InlinePatternFragments(*this);
+      // Can leak, if InlinePatternFragments doesn't return 'this'
+      Trees[i].reset(Trees[i].release()->InlinePatternFragments(*this));
   }
 
   /// InferAllTypes - Infer/propagate as many types throughout the expression
@@ -609,7 +612,7 @@ public:
   void dump() const;
 
 private:
-  TreePatternNode *ParseTreePattern(Init *DI, StringRef OpName);
+  std::unique_ptr<TreePatternNode> ParseTreePattern(Init *DI, StringRef OpName);
   void ComputeNamedNodes();
   void ComputeNamedNodes(TreePatternNode *N);
 };
@@ -625,14 +628,15 @@ class DAGInstruction {
   std::vector<Record*> Results;
   std::vector<Record*> Operands;
   std::vector<Record*> ImpResults;
-  TreePatternNode *ResultPattern;
+  std::unique_ptr<TreePatternNode> ResultPattern;
+
 public:
   DAGInstruction(TreePattern *TP,
                  const std::vector<Record*> &results,
                  const std::vector<Record*> &operands,
                  const std::vector<Record*> &impresults)
     : Pattern(TP), Results(results), Operands(operands),
-      ImpResults(impresults), ResultPattern(nullptr) {}
+      ImpResults(impresults)  {}
 
   TreePattern *getPattern() const { return Pattern; }
   unsigned getNumResults() const { return Results.size(); }
@@ -640,7 +644,9 @@ public:
   unsigned getNumImpResults() const { return ImpResults.size(); }
   const std::vector<Record*>& getImpResults() const { return ImpResults; }
 
-  void setResultPattern(TreePatternNode *R) { ResultPattern = R; }
+  void setResultPattern(std::unique_ptr<TreePatternNode> R) {
+    ResultPattern = std::move(R);
+  }
 
   Record *getResult(unsigned RN) const {
     assert(RN < Results.size());
@@ -657,7 +663,7 @@ public:
     return ImpResults[RN];
   }
 
-  TreePatternNode *getResultPattern() const { return ResultPattern; }
+  TreePatternNode *getResultPattern() const { return ResultPattern.get(); }
 };
 
 /// PatternToMatch - Used by CodeGenDAGPatterns to keep tab of patterns





More information about the llvm-commits mailing list