[llvm] 6d4a3e9 - Revert "[TableGen] Use heap allocated arrays instead of vectors for TreePatternNode::Types and ResultPerm. NFC"
Tomas Matheson via llvm-commits
llvm-commits at lists.llvm.org
Sun Jul 9 02:18:45 PDT 2023
Author: Tomas Matheson
Date: 2023-07-09T10:14:48+01:00
New Revision: 6d4a3e917c1e32b665e1177c92f6b90032817feb
URL: https://github.com/llvm/llvm-project/commit/6d4a3e917c1e32b665e1177c92f6b90032817feb
DIFF: https://github.com/llvm/llvm-project/commit/6d4a3e917c1e32b665e1177c92f6b90032817feb.diff
LOG: Revert "[TableGen] Use heap allocated arrays instead of vectors for TreePatternNode::Types and ResultPerm. NFC"
While working on DAGISelMatcherEmitter I've hit several runtime errors
caused by accessing TreePatternNode::Types out of bounds. These were
difficult to debug because the switch from std::vector to unique_ptr
removes bounds checking.
I don't think the slight reduction in class size is worth the extra
debugging and memory safety problems, so I suggest we revert this.
This reverts commit d34125a1a825208b592cfa8f5fc3566303d691a4.
Differential Revision: https://reviews.llvm.org/D154781
Added:
Modified:
llvm/utils/TableGen/CodeGenDAGPatterns.cpp
llvm/utils/TableGen/CodeGenDAGPatterns.h
llvm/utils/TableGen/DAGISelMatcherGen.cpp
Removed:
################################################################################
diff --git a/llvm/utils/TableGen/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
index 3260d025e82462..e481f7e38e6a5b 100644
--- a/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
@@ -1762,7 +1762,7 @@ bool TreePatternNode::UpdateNodeTypeFromInst(unsigned ResNo,
}
bool TreePatternNode::ContainsUnresolvedType(TreePattern &TP) const {
- for (unsigned i = 0, e = getNumTypes(); i != e; ++i)
+ for (unsigned i = 0, e = Types.size(); i != e; ++i)
if (!TP.getInfer().isConcrete(Types[i], true))
return true;
for (unsigned i = 0, e = getNumChildren(); i != e; ++i)
@@ -1772,7 +1772,7 @@ bool TreePatternNode::ContainsUnresolvedType(TreePattern &TP) const {
}
bool TreePatternNode::hasProperTypeByHwMode() const {
- for (const TypeSetByHwMode &S : getExtTypes())
+ for (const TypeSetByHwMode &S : Types)
if (!S.isSimple())
return true;
for (const TreePatternNodePtr &C : Children)
@@ -1782,7 +1782,7 @@ bool TreePatternNode::hasProperTypeByHwMode() const {
}
bool TreePatternNode::hasPossibleType() const {
- for (const TypeSetByHwMode &S : getExtTypes())
+ for (const TypeSetByHwMode &S : Types)
if (!S.isPossible())
return false;
for (const TreePatternNodePtr &C : Children)
@@ -1792,7 +1792,7 @@ bool TreePatternNode::hasPossibleType() const {
}
bool TreePatternNode::setDefaultMode(unsigned Mode) {
- for (TypeSetByHwMode &S : getExtTypes()) {
+ for (TypeSetByHwMode &S : Types) {
S.makeSimple(Mode);
// Check if the selected mode had a type conflict.
if (S.get(DefaultMode).empty())
@@ -1932,7 +1932,7 @@ void TreePatternNode::print(raw_ostream &OS) const {
else
OS << '(' << getOperator()->getName();
- for (unsigned i = 0, e = getNumTypes(); i != e; ++i) {
+ for (unsigned i = 0, e = Types.size(); i != e; ++i) {
OS << ':';
getExtType(i).writeToStream(OS);
}
@@ -2024,7 +2024,7 @@ TreePatternNodePtr TreePatternNode::clone() const {
}
New->setName(getName());
New->setNamesAsPredicateArg(getNamesAsPredicateArg());
- llvm::copy(getExtTypes(), New->getExtTypes().begin());
+ New->Types = Types;
New->setPredicateCalls(getPredicateCalls());
New->setGISelFlagsRecord(getGISelFlagsRecord());
New->setTransformFn(getTransformFn());
@@ -2034,7 +2034,7 @@ TreePatternNodePtr TreePatternNode::clone() const {
/// RemoveAllTypes - Recursively strip all the types of this tree.
void TreePatternNode::RemoveAllTypes() {
// Reset to unknown type.
- std::fill(getExtTypes().begin(), getExtTypes().end(), TypeSetByHwMode());
+ std::fill(Types.begin(), Types.end(), TypeSetByHwMode());
if (isLeaf()) return;
for (unsigned i = 0, e = getNumChildren(); i != e; ++i)
getChild(i)->RemoveAllTypes();
@@ -2130,10 +2130,10 @@ void TreePatternNode::InlinePatternFragments(
R->setPredicateCalls(getPredicateCalls());
R->setGISelFlagsRecord(getGISelFlagsRecord());
R->setTransformFn(getTransformFn());
- for (unsigned i = 0, e = getNumTypes(); i != e; ++i) {
+ for (unsigned i = 0, e = getNumTypes(); i != e; ++i)
R->setType(i, getExtType(i));
+ for (unsigned i = 0, e = getNumResults(); i != e; ++i)
R->setResultIndex(i, getResultIndex(i));
- }
// Register alternative.
OutAlternatives.push_back(R);
@@ -2468,7 +2468,7 @@ bool TreePatternNode::ApplyTypeConstraints(TreePattern &TP, bool NotRegisters) {
if (DefInit *DI = dyn_cast<DefInit>(getLeafValue())) {
// If it's a regclass or something else known, include the type.
bool MadeChange = false;
- for (unsigned i = 0, e = getNumTypes(); i != e; ++i)
+ for (unsigned i = 0, e = Types.size(); i != e; ++i)
MadeChange |= UpdateNodeType(i, getImplicitType(DI->getDef(), i,
NotRegisters,
!hasName(), TP), TP);
@@ -2476,7 +2476,7 @@ bool TreePatternNode::ApplyTypeConstraints(TreePattern &TP, bool NotRegisters) {
}
if (IntInit *II = dyn_cast<IntInit>(getLeafValue())) {
- assert(getNumTypes() == 1 && "Invalid IntInit");
+ assert(Types.size() == 1 && "Invalid IntInit");
// Int inits are always integers. :)
bool MadeChange = TP.getInfer().EnforceInteger(Types[0]);
@@ -2713,7 +2713,7 @@ bool TreePatternNode::ApplyTypeConstraints(TreePattern &TP, bool NotRegisters) {
bool MadeChange = false;
if (!NotRegisters) {
- assert(getNumTypes() == 1 && "ComplexPatterns only produce one result!");
+ assert(Types.size() == 1 && "ComplexPatterns only produce one result!");
Record *T = CDP.getComplexPattern(getOperator()).getValueType();
const CodeGenHwModes &CGH = CDP.getTargetInfo().getHwModes();
const ValueTypeByHwMode VVT = getValueTypeByHwMode(T, CGH);
diff --git a/llvm/utils/TableGen/CodeGenDAGPatterns.h b/llvm/utils/TableGen/CodeGenDAGPatterns.h
index 1579409dbb67b5..2611fe06f55ca5 100644
--- a/llvm/utils/TableGen/CodeGenDAGPatterns.h
+++ b/llvm/utils/TableGen/CodeGenDAGPatterns.h
@@ -626,16 +626,13 @@ struct TreePredicateCall {
};
class TreePatternNode : public RefCountedBase<TreePatternNode> {
- /// Number of results for this node.
- unsigned NumResults;
-
/// The type of each node result. Before and during type inference, each
/// result may be a set of possible types. After (successful) type inference,
/// each is a single concrete type.
- std::unique_ptr<TypeSetByHwMode[]> Types;
+ std::vector<TypeSetByHwMode> Types;
/// The index of each result in results of the pattern.
- std::unique_ptr<unsigned[]> ResultPerm;
+ std::vector<unsigned> ResultPerm;
/// OperatorOrVal - The Record for the operator if this is an interior node
/// (not a leaf) or the init value (e.g. the "GPRC" record, or "7") for a
@@ -654,7 +651,7 @@ class TreePatternNode : public RefCountedBase<TreePatternNode> {
/// TransformFn - The transformation function to execute on this node before
/// it can be substituted into the resulting instruction on a pattern match.
- Record *TransformFn = nullptr;
+ Record *TransformFn;
std::vector<TreePatternNodePtr> Children;
@@ -664,16 +661,17 @@ class TreePatternNode : public RefCountedBase<TreePatternNode> {
public:
TreePatternNode(Record *Op, std::vector<TreePatternNodePtr> Ch,
- unsigned numResults)
- : NumResults(numResults), Types(new TypeSetByHwMode[numResults]),
- ResultPerm(new unsigned[numResults]), OperatorOrVal(Op),
- Children(std::move(Ch)) {
- std::iota(ResultPerm.get(), ResultPerm.get() + numResults, 0);
+ unsigned NumResults)
+ : OperatorOrVal(Op), TransformFn(nullptr), Children(std::move(Ch)) {
+ Types.resize(NumResults);
+ ResultPerm.resize(NumResults);
+ std::iota(ResultPerm.begin(), ResultPerm.end(), 0);
}
- TreePatternNode(Init *val, unsigned numResults) // leaf ctor
- : NumResults(numResults), Types(new TypeSetByHwMode[numResults]),
- ResultPerm(new unsigned[numResults]), OperatorOrVal(val) {
- std::iota(ResultPerm.get(), ResultPerm.get() + numResults, 0);
+ TreePatternNode(Init *val, unsigned NumResults) // leaf ctor
+ : OperatorOrVal(val), TransformFn(nullptr) {
+ Types.resize(NumResults);
+ ResultPerm.resize(NumResults);
+ std::iota(ResultPerm.begin(), ResultPerm.end(), 0);
}
bool hasName() const { return !Name.empty(); }
@@ -693,16 +691,11 @@ class TreePatternNode : public RefCountedBase<TreePatternNode> {
bool isLeaf() const { return isa<Init *>(OperatorOrVal); }
// Type accessors.
- unsigned getNumTypes() const { return NumResults; }
+ unsigned getNumTypes() const { return Types.size(); }
ValueTypeByHwMode getType(unsigned ResNo) const {
return Types[ResNo].getValueTypeByHwMode();
}
- ArrayRef<TypeSetByHwMode> getExtTypes() const {
- return ArrayRef(Types.get(), NumResults);
- }
- MutableArrayRef<TypeSetByHwMode> getExtTypes() {
- return MutableArrayRef(Types.get(), NumResults);
- }
+ const std::vector<TypeSetByHwMode> &getExtTypes() const { return Types; }
const TypeSetByHwMode &getExtType(unsigned ResNo) const {
return Types[ResNo];
}
@@ -719,6 +712,7 @@ class TreePatternNode : public RefCountedBase<TreePatternNode> {
return Types[ResNo].empty();
}
+ unsigned getNumResults() const { return ResultPerm.size(); }
unsigned getResultIndex(unsigned ResNo) const { return ResultPerm[ResNo]; }
void setResultIndex(unsigned ResNo, unsigned RI) { ResultPerm[ResNo] = RI; }
diff --git a/llvm/utils/TableGen/DAGISelMatcherGen.cpp b/llvm/utils/TableGen/DAGISelMatcherGen.cpp
index 33ec3e7099f90a..f773f7c77a7713 100644
--- a/llvm/utils/TableGen/DAGISelMatcherGen.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherGen.cpp
@@ -1067,7 +1067,7 @@ void MatcherGen::EmitResultCode() {
SmallVector<unsigned, 8> Results(Ops);
// Apply result permutation.
- for (unsigned ResNo = 0; ResNo < Pattern.getDstPattern()->getNumTypes();
+ for (unsigned ResNo = 0; ResNo < Pattern.getDstPattern()->getNumResults();
++ResNo) {
Results[ResNo] = Ops[Pattern.getDstPattern()->getResultIndex(ResNo)];
}
More information about the llvm-commits
mailing list