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 d34125a.

Differential Revision: https://reviews.llvm.org/D154781

Author: tmatheson-arm

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,15 +2468,15 @@ 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);
       return MadeChange;
     }
 
     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);

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; }
 

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)];
   }