Skip to content

Commit

Permalink
[mlir] Don't allocate an operand storage if the operation is known to…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
River707 committed Apr 27, 2020
1 parent 4dfd1b5 commit 1956a8a
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 13 deletions.
17 changes: 13 additions & 4 deletions mlir/include/mlir/IR/Operation.h
Expand Up @@ -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) {
Expand All @@ -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]; }
Expand Down Expand Up @@ -593,14 +596,15 @@ 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.
~Operation();

/// Returns the operand storage object.
detail::OperandStorage &getOperandStorage() {
assert(hasOperandStorage && "expected operation to have operand storage");
return *getTrailingObjects<detail::OperandStorage>();
}

Expand Down Expand Up @@ -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 different
/// states recorded here:
Expand Down
33 changes: 24 additions & 9 deletions mlir/lib/IR/Operation.cpp
Expand Up @@ -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");
Expand All @@ -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();
Expand All @@ -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;
Expand All @@ -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())
Expand Down Expand Up @@ -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");
}

//===----------------------------------------------------------------------===//
Expand Down

0 comments on commit 1956a8a

Please sign in to comment.