From 3941c30371853cdd9cd02c0d20fde90347289e6c Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Mon, 17 Nov 2014 22:16:55 +0000 Subject: [PATCH] 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) git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@222183 91177308-0d34-0410-b5e6-96231b3b80d8 --- utils/TableGen/CodeGenDAGPatterns.cpp | 70 ++++++++++++++------------- utils/TableGen/CodeGenDAGPatterns.h | 32 +++++++----- 2 files changed, 55 insertions(+), 47 deletions(-) diff --git a/utils/TableGen/CodeGenDAGPatterns.cpp b/utils/TableGen/CodeGenDAGPatterns.cpp index a750aa9f4ec..26f32d2026e 100644 --- a/utils/TableGen/CodeGenDAGPatterns.cpp +++ b/utils/TableGen/CodeGenDAGPatterns.cpp @@ -1902,10 +1902,10 @@ TreePattern::TreePattern(Record *TheRec, DagInit *Pat, bool isInput, Trees.push_back(ParseTreePattern(Pat, "")); } -TreePattern::TreePattern(Record *TheRec, TreePatternNode *Pat, bool isInput, +TreePattern::TreePattern(Record *TheRec, std::unique_ptr 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(TreePatternNode *N) { ComputeNamedNodes(N->getChild(i)); } - -TreePatternNode *TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName){ +std::unique_ptr +TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName) { if (DefInit *DI = dyn_cast(TheInit)) { Record *R = DI->getDef(); @@ -1944,7 +1944,7 @@ TreePatternNode *TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName){ OpName); // Input argument? - TreePatternNode *Res = new TreePatternNode(DI, 1); + auto Res = llvm::make_unique(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::ParseTreePattern(Init *TheInit, StringRef OpName){ 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(TheInit, 1); Args.push_back(OpName); Res->setName(OpName); return Res; @@ -1968,7 +1968,7 @@ TreePatternNode *TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName){ if (IntInit *II = dyn_cast(TheInit)) { if (!OpName.empty()) error("Constant int argument should not have a name!"); - return new TreePatternNode(II, 1); + return llvm::make_unique(II, 1); } if (BitsInit *BI = dyn_cast(TheInit)) { @@ -1994,7 +1994,7 @@ TreePatternNode *TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName){ 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::ParseTreePattern(Init *TheInit, StringRef OpName){ // 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_* , ..children..). Pick the right intrinsic node, and @@ -2089,7 +2090,7 @@ TreePatternNode *TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName){ } unsigned NumResults = GetNumNodeResults(Operator, CDP); - TreePatternNode *Result = new TreePatternNode(Operator, Children, NumResults); + auto Result = llvm::make_unique(Operator, Children, NumResults); Result->setName(OpName); if (!Dag->getName().empty()) { @@ -2105,7 +2106,7 @@ TreePatternNode *TreePattern::ParseTreePattern(Init *TheInit, StringRef OpName){ /// 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 &N) { if (N->isLeaf()) return false; @@ -2115,7 +2116,7 @@ static bool SimplifyTree(TreePatternNode *&N) { 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 *&N) { // Walk all children. bool MadeChange = false; for (unsigned i = 0, e = N->getNumChildren(); i != e; ++i) { - TreePatternNode *Child = N->getChild(i); + std::unique_ptr 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 > *InNamedTypes) { // 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(Nodes[i]->getLeafValue()); if (DI && (DI->getDef()->isSubClassOf("RegisterClass") || DI->getDef()->isSubClassOf("RegisterOperand"))) @@ -2924,26 +2925,27 @@ const DAGInstruction &CodeGenDAGPatterns::parseInstructionPattern( 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( + 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 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()); + if (!DstPattern->isLeaf()) { + auto NewPattern = llvm::make_unique( + 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); + } - for (unsigned i = 0, e = Result.getOnlyTree()->getNumTypes(); i != e; ++i) - DstPattern->setType(i, Result.getOnlyTree()->getExtType(i)); - - 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())); } diff --git a/utils/TableGen/CodeGenDAGPatterns.h b/utils/TableGen/CodeGenDAGPatterns.h index c0812cf0553..5e688cfb110 100644 --- a/utils/TableGen/CodeGenDAGPatterns.h +++ b/utils/TableGen/CodeGenDAGPatterns.h @@ -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 Trees; + std::vector> 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 Pat, + bool isInput, CodeGenDAGPatterns &ise); /// getTrees - Return the tree patterns which corresponds to this pattern. /// - const std::vector &getTrees() const { return Trees; } + const std::vector> &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 &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 ParseTreePattern(Init *DI, StringRef OpName); void ComputeNamedNodes(); void ComputeNamedNodes(TreePatternNode *N); }; @@ -625,14 +628,15 @@ class DAGInstruction { std::vector Results; std::vector Operands; std::vector ImpResults; - TreePatternNode *ResultPattern; + std::unique_ptr ResultPattern; + public: DAGInstruction(TreePattern *TP, const std::vector &results, const std::vector &operands, const std::vector &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& getImpResults() const { return ImpResults; } - void setResultPattern(TreePatternNode *R) { ResultPattern = R; } + void setResultPattern(std::unique_ptr 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