[mlir] Merge the successor operand count into BlockOperand.

[rriddle]

Summary

The successor operand counts are directly tied to block operands anyways, and this simplifies the trailing objects of Operation(i.e. one less computation to perform).

[merge_guards_bot]

Unit tests: pass. 61127 tests passed, 0 failed and 728 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

mlir/include/mlir/IR/BlockSupport.h

 #include "mlir/IR/Value.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/ilist.h"
 #include "llvm/ADT/ilist_node.h"
 
 namespace mlir {
 class Block;
 
-using BlockOperand = IROperandImpl<Block>;
-
 //===----------------------------------------------------------------------===//
 // Predecessors
 //===----------------------------------------------------------------------===//
 
 /// Implement a predecessor iterator for blocks. This works by walking the use
 /// lists of the blocks. The entries on this list are the BlockOperands that
 /// are embedded into terminator operations. From the operand, we can get the
 /// terminator that contains it, and its parent block is the predecessor.

mlir/include/mlir/IR/Operation.h

 
 #include "mlir/IR/Block.h"
 #include "mlir/IR/Diagnostics.h"
 #include "mlir/IR/OperationSupport.h"
 #include "mlir/IR/Region.h"
 #include "llvm/ADT/Twine.h"
 
 namespace mlir {
-/// Terminator operations can have Block operands to represent successors.
-using BlockOperand = IROperandImpl<Block>;
-
 /// Operation is a basic unit of execution within a function. Operations can
 /// be nested within other operations effectively forming a tree. Child
 /// operations are organized into operation blocks represented by a 'Block'
 /// class.
 class Operation final
     : public llvm::ilist_node_with_parent<Operation, Block>,
-      private llvm::TrailingObjects<Operation, OpResult, BlockOperand, unsigned,
-                                    Region, detail::OperandStorage> {
+      private llvm::TrailingObjects<Operation, OpResult, BlockOperand, Region,
+                                    detail::OperandStorage> {
 public:
   /// Create a new Operation with the specific fields.
   static Operation *create(Location location, OperationName name,
                            ArrayRef<Type> resultTypes, ArrayRef<Value> operands,
                            ArrayRef<NamedAttribute> attributes,
                            ArrayRef<Block *> successors, unsigned numRegions,
                            bool resizableOperandList);
 
Value getSuccessorOperand(unsigned succIndex, unsigned opIndex) {
     return getOperand(getSuccessorOperandIndex(succIndex) + opIndex);
   }
 
   bool hasSuccessors() { return numSuccs != 0; }
   unsigned getNumSuccessors() { return numSuccs; }
   unsigned getNumSuccessorOperands(unsigned index) {
     assert(!isKnownNonTerminator() && "only terminators may have successors");
     assert(index < getNumSuccessors());
-    return getTrailingObjects<unsigned>()[index];
+    return getBlockOperands()[index].numSuccessorOperands;
   }
 
   Block *getSuccessor(unsigned index) {
     assert(index < getNumSuccessors());
     return getBlockOperands()[index].get();
   }
   void setSuccessor(Block *block, unsigned index);
 
   /// Erase a specific operand from the operand list of the successor at
   /// 'index'.
   void eraseSuccessorOperand(unsigned succIndex, unsigned opIndex) {
     assert(succIndex < getNumSuccessors());
     assert(opIndex < getNumSuccessorOperands(succIndex));
     getOperandStorage().eraseOperand(getSuccessorOperandIndex(succIndex) +
                                      opIndex);
-    --getTrailingObjects<unsigned>()[succIndex];
+    --getBlockOperands()[succIndex].numSuccessorOperands;
   }
 
   /// Get the index of the first operand of the successor at the provided
   /// index.
   unsigned getSuccessorOperandIndex(unsigned index);
 
   /// Return a pair (successorIndex, successorArgIndex) containing the index
   /// of the successor that `operandIndex` belongs to and the index of the
private:
 
   // allow block to access the 'orderIndex' field.
   friend class Block;
 
   // allow ilist_node_with_parent to access the 'getParent' method.
   friend class llvm::ilist_node_with_parent<Operation, Block>;
 
   // This stuff is used by the TrailingObjects template.
-  friend llvm::TrailingObjects<Operation, OpResult, BlockOperand, unsigned,
-                               Region, detail::OperandStorage>;
+  friend llvm::TrailingObjects<Operation, OpResult, BlockOperand, Region,
+                               detail::OperandStorage>;
   size_t numTrailingObjects(OverloadToken<OpResult>) const {
     return numResults;
   }
   size_t numTrailingObjects(OverloadToken<BlockOperand>) const {
     return numSuccs;
   }
   size_t numTrailingObjects(OverloadToken<Region>) const { return numRegions; }
-  size_t numTrailingObjects(OverloadToken<unsigned>) const { return numSuccs; }
 };
 
 inline raw_ostream &operator<<(raw_ostream &os, Operation &op) {
   op.print(os);
   return os;
 }
 
 /// This class implements use iterator for the Operation. This iterates over all

mlir/include/mlir/IR/UseDefLists.h

 #define MLIR_IR_USEDEFLISTS_H
 
 #include "mlir/IR/Location.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/iterator_range.h"
 
 namespace mlir {
 
+class Block;
 class IROperand;
 class Operation;
 class Value;
 template <typename OperandType> class ValueUseIterator;
 template <typename OperandType> class ValueUserIterator;
 
+//===----------------------------------------------------------------------===//
+// IRObjectWithUseList
+//===----------------------------------------------------------------------===//
+
 class IRObjectWithUseList {
 public:
   ~IRObjectWithUseList() {
     assert(use_empty() && "Cannot destroy a value that still has uses!");
   }
 
   /// Returns true if this value has no uses.
   bool use_empty() const { return firstUse == nullptr; }
protected:
   IROperand *getFirstUse() { return firstUse; }
   const IROperand *getFirstUse() const { return firstUse; }
 
 private:
   friend class IROperand;
   IROperand *firstUse = nullptr;
 };
 
+//===----------------------------------------------------------------------===//
+// IROperand
+//===----------------------------------------------------------------------===//
+
 /// A reference to a value, suitable for use as an operand of an operation.
 class IROperand {
 public:
   IROperand(Operation *owner) : owner(owner) {}
   IROperand(Operation *owner, IRObjectWithUseList *value)
       : value(value), owner(owner) {
     insertIntoCurrent();
   }
void insertIntoCurrent() {
     back = &value->firstUse;
     nextUse = value->firstUse;
     if (nextUse)
       nextUse->back = &nextUse;
     value->firstUse = this;
   }
 };
 
-/// A reference to a value, suitable for use as an operand of an operation.
-class OpOperand : public IROperand {
+//===----------------------------------------------------------------------===//
+// BlockOperand
+//===----------------------------------------------------------------------===//
+
+/// Terminator operations can have Block operands to represent successors.
+class BlockOperand : public IROperand {
 public:
-  OpOperand(Operation *owner) : IROperand(owner) {}
-  OpOperand(Operation *owner, Value value);
+  using IROperand::IROperand;
 
-  /// Return the current value being used by this operand.
-  Value get();
+  /// Return the current block being used by this operand.
+  Block *get();
 
   /// Set the current value being used by this operand.
-  void set(Value newValue);
+  void set(Block *block);
 
   /// Return which operand this is in the operand list of the User.
   unsigned getOperandNumber();
+
+private:
+  /// The number of OpOperands that correspond with this block operand.
+  unsigned numSuccessorOperands = 0;
+
+  /// Allow access to 'numSuccessorOperands'.
+  friend Operation;
 };
 
-/// A reference to a value, suitable for use as an operand of an operation,
-/// operation, etc.  IRValueTy is the root type to use for values this tracks,
-/// and SSAUserTy is the type that will contain operands.
-template <typename IRValueTy> class IROperandImpl : public IROperand {
+//===----------------------------------------------------------------------===//
+// OpOperand
+//===----------------------------------------------------------------------===//
+
+/// A reference to a value, suitable for use as an operand of an operation.
+class OpOperand : public IROperand {
 public:
-  IROperandImpl(Operation *owner) : IROperand(owner) {}
-  IROperandImpl(Operation *owner, IRValueTy *value) : IROperand(owner, value) {}
+  OpOperand(Operation *owner) : IROperand(owner) {}
+  OpOperand(Operation *owner, Value value);
 
   /// Return the current value being used by this operand.
-  IRValueTy *get() { return (IRValueTy *)IROperand::get(); }
+  Value get();
 
   /// Set the current value being used by this operand.
-  void set(IRValueTy *newValue) { IROperand::set(newValue); }
+  void set(Value newValue);
 
   /// Return which operand this is in the operand list of the User.
   unsigned getOperandNumber();
 };
 
+//===----------------------------------------------------------------------===//
+// ValueUseIterator
+//===----------------------------------------------------------------------===//
+
 /// An iterator over all uses of a ValueBase.
 template <typename OperandType>
 class ValueUseIterator
     : public std::iterator<std::forward_iterator_tag, OperandType> {
 public:
   ValueUseIterator() = default;
   explicit ValueUseIterator(OperandType *current) : current(current) {}
   OperandType *operator->() const { return current; }
inline auto IRObjectWithUseList::getUses() const -> use_range {
   return {use_begin(), use_end()};
 }
 
 /// Returns true if this value has exactly one use.
 inline bool IRObjectWithUseList::hasOneUse() const {
   return firstUse && firstUse->getNextOperandUsingThisValue() == nullptr;
 }
 
+//===----------------------------------------------------------------------===//
+// ValueUserIterator
+//===----------------------------------------------------------------------===//
+
 /// An iterator over all users of a ValueBase.
 template <typename OperandType>
 class ValueUserIterator final
     : public llvm::mapped_iterator<ValueUseIterator<OperandType>,
                                    Operation *(*)(OperandType &)> {
   static Operation *unwrap(OperandType &value) { return value.getOwner(); }
 
 public:

mlir/lib/IR/Operation.cpp

 /// Return the result number of this result.
 unsigned OpResult::getResultNumber() const {
   // Results are not stored in place, so we have to find it within the list.
   auto resList = getOwner()->getOpResults();
   return std::distance(resList.begin(), llvm::find(resList, *this));
 }
 
 //===----------------------------------------------------------------------===//
-// OpOperand
-//===----------------------------------------------------------------------===//
-
-OpOperand::OpOperand(Operation *owner, Value value)
-    : IROperand(owner, value.impl) {}
-
-/// Return the current value being used by this operand.
-Value OpOperand::get() { return (detail::ValueImpl *)IROperand::get(); }
-
-/// Set the current value being used by this operand.
-void OpOperand::set(Value newValue) { IROperand::set(newValue.impl); }
-
-/// Return which operand this is in the operand list.
-unsigned OpOperand::getOperandNumber() {
-  return this - &getOwner()->getOpOperands()[0];
-}
-
-//===----------------------------------------------------------------------===//
-// BlockOperand
-//===----------------------------------------------------------------------===//
-
-// TODO: This namespace is only required because of a bug in GCC<7.0.
-namespace mlir {
-/// Return which operand this is in the operand list.
-template <> unsigned BlockOperand::getOperandNumber() {
-  return this - &getOwner()->getBlockOperands()[0];
-}
-} // end namespace mlir
-
-//===----------------------------------------------------------------------===//
 // Operation
 //===----------------------------------------------------------------------===//
 
 /// Create a new Operation with the specific fields.
 Operation *Operation::create(Location location, OperationName name,
                              ArrayRef<Type> resultTypes,
                              ArrayRef<Value> operands,
                              ArrayRef<NamedAttribute> attributes,
Operation *Operation::create(Location location, OperationName name,
                              bool resizableOperandList) {
   unsigned numSuccessors = successors.size();
 
   // Input operands are nullptr-separated for each successor, the null operands
   // aren't actually stored.
   unsigned numOperands = operands.size() - numSuccessors;
 
   // Compute the byte size for the operation and the operand storage.
-  auto byteSize = totalSizeToAlloc<OpResult, BlockOperand, unsigned, Region,
-                                   detail::OperandStorage>(
-      resultTypes.size(), numSuccessors, numSuccessors, numRegions,
+  auto byteSize =
+      totalSizeToAlloc<OpResult, BlockOperand, Region, detail::OperandStorage>(
+          resultTypes.size(), numSuccessors, numRegions,
           /*detail::OperandStorage*/ 1);
   byteSize += llvm::alignTo(detail::OperandStorage::additionalAllocSize(
                                 numOperands, resizableOperandList),
                             alignof(Operation));
   void *rawMem = malloc(byteSize);
 
   // Create the new Operation.
   auto op = ::new (rawMem) Operation(location, name, resultTypes.size(),
                                      numSuccessors, numRegions, attributes);
if (operandIt == operandE) {
     // of successors.
     assert(currentSuccNum == numSuccessors);
     return op;
   }
 
   assert(!op->isKnownNonTerminator() &&
          "Unexpected nullptr in operand list when creating non-terminator.");
   auto instBlockOperands = op->getBlockOperands();
-  unsigned *succOperandCountIt = op->getTrailingObjects<unsigned>();
-  unsigned *succOperandCountE = succOperandCountIt + numSuccessors;
-  (void)succOperandCountE;
+  unsigned *succOperandCount = nullptr;
 
   for (; operandIt != operandE; ++operandIt) {
     // If we encounter a sentinel branch to the next operand update the count
     // variable.
     if (!operands[operandIt]) {
       assert(currentSuccNum < numSuccessors);
 
-      // After the first iteration update the successor operand count
-      // variable.
-      if (currentSuccNum != 0) {
-        ++succOperandCountIt;
-        assert(succOperandCountIt != succOperandCountE &&
-               "More sentinel operands than successors.");
-      }
-
       new (&instBlockOperands[currentSuccNum])
           BlockOperand(op, successors[currentSuccNum]);
-      *succOperandCountIt = 0;
+      succOperandCount =
+          &instBlockOperands[currentSuccNum].numSuccessorOperands;
       ++currentSuccNum;
       continue;
     }
     new (&opOperands[nextOperand++]) OpOperand(op, operands[operandIt]);
-    ++(*succOperandCountIt);
+    ++(*succOperandCount);
   }
 
   // Verify that the amount of sentinel operands is equivalent to the number of
   // successors.
   assert(currentSuccNum == numSuccessors);
 
   return op;
 }

 /// index.
 unsigned Operation::getSuccessorOperandIndex(unsigned index) {
   assert(!isKnownNonTerminator() && "only terminators may have successors");
   assert(index < getNumSuccessors());
 
   // Count the number of operands for each of the successors after, and
   // including, the one at 'index'. This is based upon the assumption that all
   // non successor operands are placed at the beginning of the operand list.
-  auto *successorOpCountBegin = getTrailingObjects<unsigned>();
+  auto blockOperands = getBlockOperands().drop_front(index);
   unsigned postSuccessorOpCount =
-      std::accumulate(successorOpCountBegin + index,
-                      successorOpCountBegin + getNumSuccessors(), 0u);
+      std::accumulate(blockOperands.begin(), blockOperands.end(), 0u,
+                      [](unsigned cur, const BlockOperand &operand) {
+                        return cur + operand.numSuccessorOperands;
+                      });
   return getNumOperands() - postSuccessorOpCount;
 }
 
 Optional<std::pair<unsigned, unsigned>>
 Operation::decomposeSuccessorOperandIndex(unsigned operandIndex) {
   assert(!isKnownNonTerminator() && "only terminators may have successors");
   assert(operandIndex < getNumOperands());
   unsigned currentOperandIndex = getNumOperands();
-  auto *successorOperandCounts = getTrailingObjects<unsigned>();
+  auto blockOperands = getBlockOperands();
   for (unsigned i = 0, e = getNumSuccessors(); i < e; i++) {
     unsigned successorIndex = e - i - 1;
-    currentOperandIndex -= successorOperandCounts[successorIndex];
+    currentOperandIndex -= blockOperands[successorIndex].numSuccessorOperands;
     if (currentOperandIndex <= operandIndex)
       return std::make_pair(successorIndex, operandIndex - currentOperandIndex);
   }
   return None;
 }
 
 auto Operation::getSuccessorOperands(unsigned index) -> operand_range {
   unsigned succOperandIndex = getSuccessorOperandIndex(index);

mlir/lib/IR/Value.cpp

 /// Return the Region in which this Value is defined.
 Region *Value::getParentRegion() {
   if (auto *op = getDefiningOp())
     return op->getParentRegion();
   return cast<BlockArgument>()->getOwner()->getParent();
 }
 
 //===----------------------------------------------------------------------===//
+// BlockOperand
+//===----------------------------------------------------------------------===//
+
+/// Return the current block being used by this operand.
+Block *BlockOperand::get() { return static_cast<Block *>(IROperand::get()); }
+
+/// Set the current value being used by this operand.
+void BlockOperand::set(Block *block) { IROperand::set(block); }
+
+/// Return which operand this is in the operand list.
+unsigned BlockOperand::getOperandNumber() {
+  return this - &getOwner()->getBlockOperands()[0];
+}
+
+//===----------------------------------------------------------------------===//
+// OpOperand
+//===----------------------------------------------------------------------===//
+
+OpOperand::OpOperand(Operation *owner, Value value)
+    : IROperand(owner, value.impl) {}
+
+/// Return the current value being used by this operand.
+Value OpOperand::get() { return (detail::ValueImpl *)IROperand::get(); }
+
+/// Set the current value being used by this operand.
+void OpOperand::set(Value newValue) { IROperand::set(newValue.impl); }
+
+/// Return which operand this is in the operand list.
+unsigned OpOperand::getOperandNumber() {
+  return this - &getOwner()->getOpOperands()[0];
+}
+
+//===----------------------------------------------------------------------===//
 // IRObjectWithUseList implementation.
 //===----------------------------------------------------------------------===//
 
 /// Replace all uses of 'this' value with the new value, updating anything in
 /// the IR that uses 'this' to use the other value instead.  When this returns
 /// there are zero uses of 'this'.
 void IRObjectWithUseList::replaceAllUsesWith(IRObjectWithUseList *newValue) {
   assert(this != newValue && "cannot RAUW a value with itself");