[Mlir-commits] [mlir] 1956a8a - [mlir] Don't allocate an operand storage if the operation is known to never have operands

[River Riddle]

Author: River Riddle
Date: 2020-04-26T21:34:02-07:00
New Revision: 1956a8a7cb79e94dbe073e36eba2d6b003f91046

URL: https://github.com/llvm/llvm-project/commit/1956a8a7cb79e94dbe073e36eba2d6b003f91046
DIFF: https://github.com/llvm/llvm-project/commit/1956a8a7cb79e94dbe073e36eba2d6b003f91046.diff

LOG: [mlir] Don't allocate an operand storage if the operation is known to never have operands

Certain classes of operations, such as FuncOp, are known to never have operands. This revision adds a bit to operation to detect this case and avoid allocating the unnecessary operand storage. This saves 1 word for each instance of these operations.

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

Added: 
    

Modified: 
    mlir/include/mlir/IR/Operation.h
    mlir/lib/IR/Operation.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
index 33c75ef97eda..1dc0e787aa4f 100644
--- a/mlir/include/mlir/IR/Operation.h
+++ b/mlir/include/mlir/IR/Operation.h
@@ -205,7 +205,9 @@ class Operation final
   /// 'operands'.
   void setOperands(ValueRange operands);
 
-  unsigned getNumOperands() { return getOperandStorage().size(); }
+  unsigned getNumOperands() {
+    return LLVM_LIKELY(hasOperandStorage) ? getOperandStorage().size() : 0;
+  }
 
   Value getOperand(unsigned idx) { return getOpOperand(idx).get(); }
   void setOperand(unsigned idx, Value value) {
@@ -226,7 +228,8 @@ class Operation final
   void eraseOperand(unsigned idx) { getOperandStorage().eraseOperand(idx); }
 
   MutableArrayRef<OpOperand> getOpOperands() {
-    return getOperandStorage().getOperands();
+    return LLVM_LIKELY(hasOperandStorage) ? getOperandStorage().getOperands()
+                                          : MutableArrayRef<OpOperand>();
   }
 
   OpOperand &getOpOperand(unsigned idx) { return getOpOperands()[idx]; }
@@ -593,7 +596,7 @@ class Operation final
 private:
   Operation(Location location, OperationName name, ArrayRef<Type> resultTypes,
             unsigned numSuccessors, unsigned numRegions,
-            const NamedAttributeList &attributes);
+            const NamedAttributeList &attributes, bool hasOperandStorage);
 
   // Operations are deleted through the destroy() member because they are
   // allocated with malloc.
@@ -601,6 +604,7 @@ class Operation final
 
   /// Returns the operand storage object.
   detail::OperandStorage &getOperandStorage() {
+    assert(hasOperandStorage && "expected operation to have operand storage");
     return *getTrailingObjects<detail::OperandStorage>();
   }
 
@@ -633,7 +637,12 @@ class Operation final
   mutable unsigned orderIndex = 0;
 
   const unsigned numSuccs;
-  const unsigned numRegions : 31;
+  const unsigned numRegions : 30;
+
+  /// This bit signals whether this operation has an operand storage or not. The
+  /// operand storage may be elided for operations that are known to never have
+  /// operands.
+  bool hasOperandStorage : 1;
 
   /// This holds the result types of the operation. There are three 
diff erent
   /// states recorded here:

diff  --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index 97ff4cd72a22..a60e6bf385f2 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -105,20 +105,29 @@ Operation *Operation::create(Location location, OperationName name,
   unsigned numSuccessors = successors.size();
   unsigned numOperands = operands.size();
 
+  // If the operation is known to have no operands, don't allocate an operand
+  // storage.
+  bool needsOperandStorage = true;
+  if (operands.empty()) {
+    if (const AbstractOperation *abstractOp = name.getAbstractOperation())
+      needsOperandStorage = !abstractOp->hasTrait<OpTrait::ZeroOperands>();
+  }
+
   // Compute the byte size for the operation and the operand storage.
   auto byteSize =
       totalSizeToAlloc<detail::InLineOpResult, detail::TrailingOpResult,
                        BlockOperand, Region, detail::OperandStorage>(
           numInlineResults, numTrailingResults, numSuccessors, numRegions,
-          /*detail::OperandStorage*/ 1);
+          needsOperandStorage ? 1 : 0);
   byteSize +=
       llvm::alignTo(detail::OperandStorage::additionalAllocSize(numOperands),
                     alignof(Operation));
   void *rawMem = malloc(byteSize);
 
   // Create the new Operation.
-  auto op = ::new (rawMem) Operation(location, name, resultTypes, numSuccessors,
-                                     numRegions, attributes);
+  Operation *op =
+      ::new (rawMem) Operation(location, name, resultTypes, numSuccessors,
+                               numRegions, attributes, needsOperandStorage);
 
   assert((numSuccessors == 0 || !op->isKnownNonTerminator()) &&
          "unexpected successors in a non-terminator operation");
@@ -134,7 +143,8 @@ Operation *Operation::create(Location location, OperationName name,
     new (&op->getRegion(i)) Region(op);
 
   // Initialize the operands.
-  new (&op->getOperandStorage()) detail::OperandStorage(op, operands);
+  if (needsOperandStorage)
+    new (&op->getOperandStorage()) detail::OperandStorage(op, operands);
 
   // Initialize the successors.
   auto blockOperands = op->getBlockOperands();
@@ -146,9 +156,11 @@ Operation *Operation::create(Location location, OperationName name,
 
 Operation::Operation(Location location, OperationName name,
                      ArrayRef<Type> resultTypes, unsigned numSuccessors,
-                     unsigned numRegions, const NamedAttributeList &attributes)
+                     unsigned numRegions, const NamedAttributeList &attributes,
+                     bool hasOperandStorage)
     : location(location), numSuccs(numSuccessors), numRegions(numRegions),
-      hasSingleResult(false), name(name), attrs(attributes) {
+      hasOperandStorage(hasOperandStorage), hasSingleResult(false), name(name),
+      attrs(attributes) {
   if (!resultTypes.empty()) {
     // If there is a single result it is stored in-place, otherwise use a tuple.
     hasSingleResult = resultTypes.size() == 1;
@@ -164,8 +176,9 @@ Operation::Operation(Location location, OperationName name,
 Operation::~Operation() {
   assert(block == nullptr && "operation destroyed but still in a block");
 
-  // Explicitly run the destructors for the operands and results.
-  getOperandStorage().~OperandStorage();
+  // Explicitly run the destructors for the operands.
+  if (hasOperandStorage)
+    getOperandStorage().~OperandStorage();
 
   // Explicitly run the destructors for the successors.
   for (auto &successor : getBlockOperands())
@@ -225,7 +238,9 @@ void Operation::replaceUsesOfWith(Value from, Value to) {
 /// Replace the current operands of this operation with the ones provided in
 /// 'operands'.
 void Operation::setOperands(ValueRange operands) {
-  getOperandStorage().setOperands(this, operands);
+  if (LLVM_LIKELY(hasOperandStorage))
+    return getOperandStorage().setOperands(this, operands);
+  assert(operands.empty() && "setting operands without an operand storage");
 }
 
 //===----------------------------------------------------------------------===//