Eliminate the ability to add operands to an instruction, used in a narrow case

for SSA values in terminators, but easily worked around.  At the same time,
move the StmtOperand list in a OperationStmt to the end of its trailing
objects list so we can *reduce* the number of operands, without affecting
offsets to the other stuff in the allocation.

This is important because we want OperationStmts to be consequtive, including
their operands - we don't want to use an std::vector of operands like
Instructions have.

This is patch 4/n towards merging instructions and statements, NFC.

PiperOrigin-RevId: 226865727

mlir/include/mlir/IR/BuiltinOps.h

@@ -113,12 +113,6 @@ public:
   BasicBlock *getDest() const;
   void setDest(BasicBlock *block);
 
-  /// Add one value to the operand list.
-  void addOperand(SSAValue *value);
-
-  /// Add a list of values to the operand list.
-  void addOperands(ArrayRef<SSAValue *> values);
-
   /// Erase the operand at 'index' from the operand list.
   void eraseOperand(unsigned index);
 
@@ -153,7 +147,8 @@ public:
 
   static void build(Builder *builder, OperationState *result,
                     SSAValue *condition, BasicBlock *trueDest,
-                    BasicBlock *falseDest);
+                    ArrayRef<SSAValue *> trueOperands, BasicBlock *falseDest,
+                    ArrayRef<SSAValue *> falseOperands);
 
   // Hooks to customize behavior of this op.
   static bool parse(OpAsmParser *parser, OperationState *result);
@@ -201,12 +196,6 @@ public:
 
   unsigned getNumTrueOperands() const;
 
-  /// Add one value to the true operand list.
-  void addTrueOperand(SSAValue *value);
-
-  /// Add a list of values to the operand list.
-  void addTrueOperands(ArrayRef<SSAValue *> values);
-
   /// Erase the operand at 'index' from the true operand list.
   void eraseTrueOperand(unsigned index);
 
@@ -243,12 +232,6 @@ public:
 
   unsigned getNumFalseOperands() const;
 
-  /// Add one value to the false operand list.
-  void addFalseOperand(SSAValue *value);
-
-  /// Add a list of values to the false operand list.
-  void addFalseOperands(ArrayRef<SSAValue *> values);
-
   /// Erase the operand at 'index' from the false operand list.
   void eraseFalseOperand(unsigned index);
 

mlir/include/mlir/IR/Instructions.h

@@ -233,13 +233,6 @@ public:
   }
   void setSuccessor(BasicBlock *block, unsigned index);
 
-  /// Add one value to the operand list of the successor at the provided index.
-  void addSuccessorOperand(unsigned index, CFGValue *value);
-
-  /// Add a list of values to the operand list of the successor at the provided
-  /// index.
-  void addSuccessorOperands(unsigned index, ArrayRef<CFGValue *> values);
-
   /// Erase a specific operand from the operand list of the successor at
   /// 'index'.
   void eraseSuccessorOperand(unsigned succIndex, unsigned opIndex) {

mlir/include/mlir/IR/Operation.h

@@ -119,11 +119,6 @@ public:
     return const_cast<Operation *>(this)->getSuccessor(index);
   }
   void setSuccessor(BasicBlock *block, unsigned index);
-  void addSuccessorOperand(unsigned index, SSAValue *value);
-  void addSuccessorOperands(unsigned index, ArrayRef<SSAValue *> values) {
-    for (auto *value : values)
-      addSuccessorOperand(index, value);
-  }
   void eraseSuccessorOperand(unsigned succIndex, unsigned opIndex);
   llvm::iterator_range<const_operand_iterator>
   getSuccessorOperands(unsigned index) const;

mlir/include/mlir/IR/Statements.h

@@ -37,8 +37,8 @@ class OperationStmt;
 class OperationStmt final
     : public Operation,
       public Statement,
-      private llvm::TrailingObjects<OperationStmt, StmtOperand, StmtResult,
-                                    StmtBlockOperand, unsigned> {
+      private llvm::TrailingObjects<OperationStmt, StmtResult, StmtBlockOperand,
+                                    unsigned, StmtOperand> {
 public:
   /// Create a new OperationStmt with the specific fields.
   static OperationStmt *
@@ -260,8 +260,8 @@ private:
   ~OperationStmt();
 
   // This stuff is used by the TrailingObjects template.
-  friend llvm::TrailingObjects<OperationStmt, StmtOperand, StmtResult,
-                               StmtBlockOperand, unsigned>;
+  friend llvm::TrailingObjects<OperationStmt, StmtResult, StmtBlockOperand,
+                               unsigned, StmtOperand>;
   size_t numTrailingObjects(OverloadToken<StmtOperand>) const {
     return numOperands;
   }

mlir/lib/IR/BuiltinOps.cpp

@@ -203,14 +203,6 @@ void BranchOp::setDest(BasicBlock *block) {
   return getOperation()->setSuccessor(block, 0);
 }
 
-void BranchOp::addOperand(SSAValue *value) {
-  return getOperation()->addSuccessorOperand(0, value);
-}
-
-void BranchOp::addOperands(ArrayRef<SSAValue *> values) {
-  return getOperation()->addSuccessorOperands(0, values);
-}
-
 void BranchOp::eraseOperand(unsigned index) {
   getOperation()->eraseSuccessorOperand(0, index);
 }
@@ -221,10 +213,12 @@ void BranchOp::eraseOperand(unsigned index) {
 
 void CondBranchOp::build(Builder *builder, OperationState *result,
                          SSAValue *condition, BasicBlock *trueDest,
-                         BasicBlock *falseDest) {
+                         ArrayRef<SSAValue *> trueOperands,
+                         BasicBlock *falseDest,
+                         ArrayRef<SSAValue *> falseOperands) {
   result->addOperands(condition);
-  result->addSuccessor(trueDest, /*succOperands=*/{});
-  result->addSuccessor(falseDest, /*succOperands=*/{});
+  result->addSuccessor(trueDest, trueOperands);
+  result->addSuccessor(falseDest, falseOperands);
 }
 
 bool CondBranchOp::parse(OpAsmParser *parser, OperationState *result) {
@@ -286,14 +280,6 @@ unsigned CondBranchOp::getNumTrueOperands() const {
   return getOperation()->getNumSuccessorOperands(trueIndex);
 }
 
-void CondBranchOp::addTrueOperand(SSAValue *value) {
-  return getOperation()->addSuccessorOperand(trueIndex, value);
-}
-
-void CondBranchOp::addTrueOperands(ArrayRef<SSAValue *> values) {
-  return getOperation()->addSuccessorOperands(trueIndex, values);
-}
-
 void CondBranchOp::eraseTrueOperand(unsigned index) {
   getOperation()->eraseSuccessorOperand(trueIndex, index);
 }
@@ -302,14 +288,6 @@ unsigned CondBranchOp::getNumFalseOperands() const {
   return getOperation()->getNumSuccessorOperands(falseIndex);
 }
 
-void CondBranchOp::addFalseOperand(SSAValue *value) {
-  return getOperation()->addSuccessorOperand(falseIndex, value);
-}
-
-void CondBranchOp::addFalseOperands(ArrayRef<SSAValue *> values) {
-  return getOperation()->addSuccessorOperands(falseIndex, values);
-}
-
 void CondBranchOp::eraseFalseOperand(unsigned index) {
   getOperation()->eraseSuccessorOperand(falseIndex, index);
 }

mlir/lib/IR/Instructions.cpp

@@ -248,24 +248,6 @@ bool Instruction::emitError(const Twine &message) const {
   return getContext()->emitError(getLoc(), message);
 }
 
-void Instruction::addSuccessorOperand(unsigned index, CFGValue *value) {
-  assert(isTerminator() && "Only terminators have successors.");
-  assert(index < getNumSuccessors());
-  assert(std::accumulate(getTrailingObjects<unsigned>() + index + 1,
-                         getTrailingObjects<unsigned>() + numSuccs, 0u) == 0 &&
-         "All successor operands must be added before moving to the next.");
-
-  operands.push_back(InstOperand(this, value));
-  ++getTrailingObjects<unsigned>()[index];
-}
-
-void Instruction::addSuccessorOperands(unsigned index,
-                                       ArrayRef<CFGValue *> values) {
-  operands.reserve(operands.size() + values.size());
-  for (auto *value : values)
-    addSuccessorOperand(index, value);
-}
-
 void llvm::ilist_traits<::mlir::Instruction>::deleteNode(Instruction *inst) {
   inst->destroy();
 }

mlir/lib/IR/Operation.cpp

@@ -144,19 +144,19 @@ SSAValue *Operation::getResult(unsigned idx) {
 
 unsigned Operation::getNumSuccessors() const {
   assert(isTerminator() && "Only terminators have successors.");
-  if (llvm::isa<Instruction>(this))
-    return llvm::cast<Instruction>(this)->getNumSuccessors();
+  if (auto *inst = llvm::dyn_cast<Instruction>(this))
+    return inst->getNumSuccessors();
 
-  // OperationStmt currently only has a return terminator.
-  assert(llvm::cast<OperationStmt>(this)->isReturn() &&
-         "Unhandled OperationStmt terminator.");
-  return 0;
+  return llvm::cast<OperationStmt>(this)->getNumSuccessors();
 }
 
 unsigned Operation::getNumSuccessorOperands(unsigned index) const {
   assert(isTerminator() && "Only terminators have successors.");
-  assert(llvm::isa<Instruction>(this) && "Only instructions have successors.");
-  return llvm::cast<Instruction>(this)->getNumSuccessorOperands(index);
+
+  if (auto *inst = llvm::dyn_cast<Instruction>(this))
+    return inst->getNumSuccessorOperands(index);
+
+  return llvm::cast<OperationStmt>(this)->getNumSuccessorOperands(index);
 }
 BasicBlock *Operation::getSuccessor(unsigned index) {
   assert(isTerminator() && "Only terminators have successors.");
@@ -170,12 +170,7 @@ void Operation::setSuccessor(BasicBlock *block, unsigned index) {
          "Only instructions have basic block successors.");
   llvm::cast<Instruction>(this)->setSuccessor(block, index);
 }
-void Operation::addSuccessorOperand(unsigned index, SSAValue *value) {
-  assert(isTerminator() && "Only terminators have successors.");
-  assert(llvm::isa<Instruction>(this) && "Only instructions have successors.");
-  return llvm::cast<Instruction>(this)->addSuccessorOperand(
-      index, llvm::cast<CFGValue>(value));
-}
+
 void Operation::eraseSuccessorOperand(unsigned succIndex, unsigned opIndex) {
   assert(isTerminator() && "Only terminators have successors.");
   assert(llvm::isa<Instruction>(this) && "Only instructions have successors.");

mlir/lib/IR/Statement.cpp

@@ -259,8 +259,8 @@ OperationStmt *OperationStmt::create(Location location, OperationName name,
                                      MLIRContext *context) {
   unsigned numSuccessors = successors.size();
   auto byteSize =
-      totalSizeToAlloc<StmtOperand, StmtResult, StmtBlockOperand, unsigned>(
-          operands.size(), resultTypes.size(), numSuccessors, numSuccessors);
+      totalSizeToAlloc<StmtResult, StmtBlockOperand, unsigned, StmtOperand>(
+          resultTypes.size(), numSuccessors, numSuccessors, operands.size());
   void *rawMem = malloc(byteSize);
 
   // Initialize the OperationStmt part of the statement.

mlir/lib/Transforms/ConvertToCFG.cpp

@@ -285,7 +285,8 @@ void FunctionConverter::visitForStmt(ForStmt *forStmt) {
       forStmt->getLoc(), CmpIPredicate::SLT, iv, upperBound);
   auto comparisonResult = cast<CFGValue>(comparisonOp->getResult());
   builder.create<CondBranchOp>(builder.getUnknownLoc(), comparisonResult,
-                               loopBodyFirstBlock, postLoopBlock);
+                               loopBodyFirstBlock, ArrayRef<SSAValue *>(),
+                               postLoopBlock, ArrayRef<SSAValue *>());
 
   // Finally, make sure building can continue by setting the post-loop block
   // (end of loop SESE region) as the insertion point.
@@ -424,7 +425,9 @@ void FunctionConverter::visitIfStmt(IfStmt *ifStmt) {
         ifStmt->getLoc(), isEquality ? CmpIPredicate::EQ : CmpIPredicate::SGE,
         affResult, zeroConstant);
     builder.create<CondBranchOp>(ifStmt->getLoc(), comparisonOp->getResult(),
-                                 nextBlock, elseBlock);
+                                 nextBlock, /*trueArgs*/ ArrayRef<SSAValue *>(),
+                                 elseBlock,
+                                 /*falseArgs*/ ArrayRef<SSAValue *>());
     builder.setInsertionPoint(nextBlock);
   }
   ifConditionExtraBlocks.pop_back();