[Mlir-commits] [mlir] a039113 - [mlir] Move the Operation OperandStorage to the first trailing object
River Riddle
llvmlistbot at llvm.org
Wed Nov 3 11:34:38 PDT 2021
Author: River Riddle
Date: 2021-11-03T18:34:31Z
New Revision: a0391134462a7a41ab781065322b4caa50eb59d2
URL: https://github.com/llvm/llvm-project/commit/a0391134462a7a41ab781065322b4caa50eb59d2
DIFF: https://github.com/llvm/llvm-project/commit/a0391134462a7a41ab781065322b4caa50eb59d2.diff
LOG: [mlir] Move the Operation OperandStorage to the first trailing object
The main benefits of this change are faster access to operands
(no need to compute the offset, as it is now right after the
operation), simpler code(no need to manage a lot of the "is the
operand storage trailing" logic we had to before). The major
downside to this though, is that operand holding operations now
grow in size by 1 word (as no matter how we do this change, there
will need to be some additional book keeping).
Differential Revision: https://reviews.llvm.org/D111695
Added:
Modified:
mlir/include/mlir/IR/Operation.h
mlir/include/mlir/IR/OperationSupport.h
mlir/lib/IR/Operation.cpp
mlir/lib/IR/OperationSupport.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/IR/Operation.h b/mlir/include/mlir/IR/Operation.h
index 0f74021184b3e..9862112f72d9d 100644
--- a/mlir/include/mlir/IR/Operation.h
+++ b/mlir/include/mlir/IR/Operation.h
@@ -27,8 +27,8 @@ namespace mlir {
/// 'Block' class.
class alignas(8) Operation final
: public llvm::ilist_node_with_parent<Operation, Block>,
- private llvm::TrailingObjects<Operation, BlockOperand, Region,
- detail::OperandStorage> {
+ private llvm::TrailingObjects<Operation, detail::OperandStorage,
+ BlockOperand, Region, OpOperand> {
public:
/// Create a new Operation with the specific fields.
static Operation *create(Location location, OperationName name,
@@ -244,7 +244,10 @@ class alignas(8) Operation final
operand_iterator operand_end() { return getOperands().end(); }
/// Returns an iterator on the underlying Value's.
- operand_range getOperands() { return operand_range(this); }
+ operand_range getOperands() {
+ MutableArrayRef<OpOperand> operands = getOpOperands();
+ return OperandRange(operands.data(), operands.size());
+ }
MutableArrayRef<OpOperand> getOpOperands() {
return LLVM_LIKELY(hasOperandStorage) ? getOperandStorage().getOperands()
@@ -698,8 +701,11 @@ class alignas(8) Operation final
friend class llvm::ilist_node_with_parent<Operation, Block>;
// This stuff is used by the TrailingObjects template.
- friend llvm::TrailingObjects<Operation, BlockOperand, Region,
- detail::OperandStorage>;
+ friend llvm::TrailingObjects<Operation, detail::OperandStorage, BlockOperand,
+ Region, OpOperand>;
+ size_t numTrailingObjects(OverloadToken<detail::OperandStorage>) const {
+ return hasOperandStorage ? 1 : 0;
+ }
size_t numTrailingObjects(OverloadToken<BlockOperand>) const {
return numSuccs;
}
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index 5c41123878982..f56727e51cb06 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -520,47 +520,12 @@ struct OperationState {
//===----------------------------------------------------------------------===//
namespace detail {
-/// This class contains the information for a trailing operand storage.
-struct TrailingOperandStorage final
- : public llvm::TrailingObjects<TrailingOperandStorage, OpOperand> {
-#if defined(BYTE_ORDER) && defined(BIG_ENDIAN) && (BYTE_ORDER == BIG_ENDIAN)
- TrailingOperandStorage() : numOperands(0), capacity(0), reserved(0) {}
-#else
- TrailingOperandStorage() : reserved(0), capacity(0), numOperands(0) {}
-#endif
- ~TrailingOperandStorage() {
- for (auto &operand : getOperands())
- operand.~OpOperand();
- }
-
- /// Return the operands held by this storage.
- MutableArrayRef<OpOperand> getOperands() {
- return {getTrailingObjects<OpOperand>(), numOperands};
- }
-
-#if defined(BYTE_ORDER) && defined(BIG_ENDIAN) && (BYTE_ORDER == BIG_ENDIAN)
- /// The number of operands within the storage.
- unsigned numOperands;
- /// The total capacity number of operands that the storage can hold.
- unsigned capacity : 31;
- /// We reserve a range of bits for use by the operand storage.
- unsigned reserved : 1;
-#else
- /// We reserve a range of bits for use by the operand storage.
- unsigned reserved : 1;
- /// The total capacity number of operands that the storage can hold.
- unsigned capacity : 31;
- /// The number of operands within the storage.
- unsigned numOperands;
-#endif
-};
-
/// This class handles the management of operation operands. Operands are
/// stored either in a trailing array, or a dynamically resizable vector.
-class OperandStorage final
- : private llvm::TrailingObjects<OperandStorage, OpOperand> {
+class alignas(8) OperandStorage {
public:
- OperandStorage(Operation *owner, ValueRange values);
+ OperandStorage(Operation *owner, OpOperand *trailingOperands,
+ ValueRange values);
~OperandStorage();
/// Replace the operands contained in the storage with the ones provided in
@@ -581,62 +546,25 @@ class OperandStorage final
void eraseOperands(const llvm::BitVector &eraseIndices);
/// Get the operation operands held by the storage.
- MutableArrayRef<OpOperand> getOperands() {
- return getStorage().getOperands();
- }
+ MutableArrayRef<OpOperand> getOperands() { return {operandStorage, size()}; }
/// Return the number of operands held in the storage.
- unsigned size() { return getStorage().numOperands; }
-
- /// Returns the additional size necessary for allocating this object.
- static size_t additionalAllocSize(unsigned numOperands) {
- return additionalSizeToAlloc<OpOperand>(numOperands);
- }
+ unsigned size() { return numOperands; }
private:
- /// Pointer type traits for the storage pointer that ensures that we use the
- /// lowest bit for the storage pointer.
- struct StoragePointerLikeTypeTraits
- : llvm::PointerLikeTypeTraits<TrailingOperandStorage *> {
- static constexpr int NumLowBitsAvailable = 1;
- };
-
/// Resize the storage to the given size. Returns the array containing the new
/// operands.
MutableArrayRef<OpOperand> resize(Operation *owner, unsigned newSize);
- /// Returns the current internal storage instance.
- TrailingOperandStorage &getStorage() {
- return LLVM_UNLIKELY(isDynamicStorage()) ? getDynamicStorage()
- : getInlineStorage();
- }
-
- /// Returns the storage container if the storage is inline.
- TrailingOperandStorage &getInlineStorage() {
- assert(!isDynamicStorage() && "expected storage to be inline");
- return inlineStorage;
- }
-
- /// Returns the storage container if this storage is dynamic.
- TrailingOperandStorage &getDynamicStorage() {
- assert(isDynamicStorage() && "expected dynamic storage");
- return *dynamicStorage.getPointer();
- }
-
- /// Returns true if the storage is currently dynamic.
- bool isDynamicStorage() const { return dynamicStorage.getInt(); }
-
- /// The current representation of the storage. This is either a
- /// InlineOperandStorage, or a pointer to a InlineOperandStorage.
- union {
- TrailingOperandStorage inlineStorage;
- llvm::PointerIntPair<TrailingOperandStorage *, 1, bool,
- StoragePointerLikeTypeTraits>
- dynamicStorage;
- };
-
- /// This stuff is used by the TrailingObjects template.
- friend llvm::TrailingObjects<OperandStorage, OpOperand>;
+ /// The total capacity number of operands that the storage can hold.
+ unsigned capacity : 31;
+ /// A flag indicating if the operand storage was dynamically allocated, as
+ /// opposed to inlined into the owning operation.
+ unsigned isStorageDynamic : 1;
+ /// The number of operands within the storage.
+ unsigned numOperands;
+ /// A pointer to the operand storage.
+ OpOperand *operandStorage;
};
} // end namespace detail
@@ -718,7 +646,6 @@ class OperandRange final : public llvm::detail::indexed_accessor_range_base<
OperandRange, OpOperand *, Value, Value, Value> {
public:
using RangeBaseT::RangeBaseT;
- OperandRange(Operation *op);
/// Returns the types of the values within this range.
using type_iterator = ValueTypeIterator<iterator>;
diff --git a/mlir/lib/IR/Operation.cpp b/mlir/lib/IR/Operation.cpp
index 3f1310f73a78b..d232b1d253ac3 100644
--- a/mlir/lib/IR/Operation.cpp
+++ b/mlir/lib/IR/Operation.cpp
@@ -125,9 +125,8 @@ Operation *Operation::create(Location location, OperationName name,
// into account the size of the operation, its trailing objects, and its
// prefixed objects.
size_t byteSize =
- totalSizeToAlloc<BlockOperand, Region, detail::OperandStorage>(
- numSuccessors, numRegions, needsOperandStorage ? 1 : 0) +
- detail::OperandStorage::additionalAllocSize(numOperands);
+ totalSizeToAlloc<detail::OperandStorage, BlockOperand, Region, OpOperand>(
+ needsOperandStorage ? 1 : 0, numSuccessors, numRegions, numOperands);
size_t prefixByteSize = llvm::alignTo(
Operation::prefixAllocSize(numTrailingResults, numInlineResults),
alignof(Operation));
@@ -156,8 +155,10 @@ Operation *Operation::create(Location location, OperationName name,
new (&op->getRegion(i)) Region(op);
// Initialize the operands.
- if (needsOperandStorage)
- new (&op->getOperandStorage()) detail::OperandStorage(op, operands);
+ if (needsOperandStorage) {
+ new (&op->getOperandStorage()) detail::OperandStorage(
+ op, op->getTrailingObjects<OpOperand>(), operands);
+ }
// Initialize the successors.
auto blockOperands = op->getBlockOperands();
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 002d746c1a81f..1202e0f6275d6 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -226,26 +226,22 @@ void OperationState::addRegions(
// OperandStorage
//===----------------------------------------------------------------------===//
-detail::OperandStorage::OperandStorage(Operation *owner, ValueRange values)
- : inlineStorage() {
- auto &inlineStorage = getInlineStorage();
- inlineStorage.numOperands = inlineStorage.capacity = values.size();
- auto *operandPtrBegin = getTrailingObjects<OpOperand>();
- for (unsigned i = 0, e = inlineStorage.numOperands; i < e; ++i)
- new (&operandPtrBegin[i]) OpOperand(owner, values[i]);
+detail::OperandStorage::OperandStorage(Operation *owner,
+ OpOperand *trailingOperands,
+ ValueRange values)
+ : isStorageDynamic(false), operandStorage(trailingOperands) {
+ numOperands = capacity = values.size();
+ for (unsigned i = 0; i < numOperands; ++i)
+ new (&operandStorage[i]) OpOperand(owner, values[i]);
}
detail::OperandStorage::~OperandStorage() {
- // Destruct the current storage container.
- if (isDynamicStorage()) {
- TrailingOperandStorage &storage = getDynamicStorage();
- storage.~TrailingOperandStorage();
- // Work around -Wfree-nonheap-object false positive fixed by D102728.
- auto *mem = &storage;
- free(mem);
- } else {
- getInlineStorage().~TrailingOperandStorage();
- }
+ for (auto &operand : getOperands())
+ operand.~OpOperand();
+
+ // If the storage is dynamic, deallocate it.
+ if (isStorageDynamic)
+ free(operandStorage);
}
/// Replace the operands contained in the storage with the ones provided in
@@ -291,24 +287,22 @@ void detail::OperandStorage::setOperands(Operation *owner, unsigned start,
/// Erase an operand held by the storage.
void detail::OperandStorage::eraseOperands(unsigned start, unsigned length) {
- TrailingOperandStorage &storage = getStorage();
- MutableArrayRef<OpOperand> operands = storage.getOperands();
+ MutableArrayRef<OpOperand> operands = getOperands();
assert((start + length) <= operands.size());
- storage.numOperands -= length;
+ numOperands -= length;
// Shift all operands down if the operand to remove is not at the end.
- if (start != storage.numOperands) {
+ if (start != numOperands) {
auto *indexIt = std::next(operands.begin(), start);
std::rotate(indexIt, std::next(indexIt, length), operands.end());
}
for (unsigned i = 0; i != length; ++i)
- operands[storage.numOperands + i].~OpOperand();
+ operands[numOperands + i].~OpOperand();
}
void detail::OperandStorage::eraseOperands(
const llvm::BitVector &eraseIndices) {
- TrailingOperandStorage &storage = getStorage();
- MutableArrayRef<OpOperand> operands = storage.getOperands();
+ MutableArrayRef<OpOperand> operands = getOperands();
assert(eraseIndices.size() == operands.size());
// Check that at least one operand is erased.
@@ -317,11 +311,11 @@ void detail::OperandStorage::eraseOperands(
return;
// Shift all of the removed operands to the end, and destroy them.
- storage.numOperands = firstErasedIndice;
+ numOperands = firstErasedIndice;
for (unsigned i = firstErasedIndice + 1, e = operands.size(); i < e; ++i)
if (!eraseIndices.test(i))
- operands[storage.numOperands++] = std::move(operands[i]);
- for (OpOperand &operand : operands.drop_front(storage.numOperands))
+ operands[numOperands++] = std::move(operands[i]);
+ for (OpOperand &operand : operands.drop_front(numOperands))
operand.~OpOperand();
}
@@ -329,24 +323,21 @@ void detail::OperandStorage::eraseOperands(
/// operands.
MutableArrayRef<OpOperand> detail::OperandStorage::resize(Operation *owner,
unsigned newSize) {
- TrailingOperandStorage &storage = getStorage();
-
// If the number of operands is less than or equal to the current amount, we
// can just update in place.
- unsigned &numOperands = storage.numOperands;
- MutableArrayRef<OpOperand> operands = storage.getOperands();
+ MutableArrayRef<OpOperand> origOperands = getOperands();
if (newSize <= numOperands) {
// If the number of new size is less than the current, remove any extra
// operands.
for (unsigned i = newSize; i != numOperands; ++i)
- operands[i].~OpOperand();
+ origOperands[i].~OpOperand();
numOperands = newSize;
- return operands.take_front(newSize);
+ return origOperands.take_front(newSize);
}
// If the new size is within the original inline capacity, grow inplace.
- if (newSize <= storage.capacity) {
- OpOperand *opBegin = operands.data();
+ if (newSize <= capacity) {
+ OpOperand *opBegin = origOperands.data();
for (unsigned e = newSize; numOperands != e; ++numOperands)
new (&opBegin[numOperands]) OpOperand(owner);
return MutableArrayRef<OpOperand>(opBegin, newSize);
@@ -354,36 +345,32 @@ MutableArrayRef<OpOperand> detail::OperandStorage::resize(Operation *owner,
// Otherwise, we need to allocate a new storage.
unsigned newCapacity =
- std::max(unsigned(llvm::NextPowerOf2(storage.capacity + 2)), newSize);
- auto *newStorageMem =
- malloc(TrailingOperandStorage::totalSizeToAlloc<OpOperand>(newCapacity));
- auto *newStorage = ::new (newStorageMem) TrailingOperandStorage();
- newStorage->numOperands = newSize;
- newStorage->capacity = newCapacity;
+ std::max(unsigned(llvm::NextPowerOf2(capacity + 2)), newSize);
+ OpOperand *newOperandStorage =
+ reinterpret_cast<OpOperand *>(malloc(sizeof(OpOperand) * newCapacity));
// Move the current operands to the new storage.
- MutableArrayRef<OpOperand> newOperands = newStorage->getOperands();
- std::uninitialized_copy(std::make_move_iterator(operands.begin()),
- std::make_move_iterator(operands.end()),
+ MutableArrayRef<OpOperand> newOperands(newOperandStorage, newSize);
+ std::uninitialized_copy(std::make_move_iterator(origOperands.begin()),
+ std::make_move_iterator(origOperands.end()),
newOperands.begin());
// Destroy the original operands.
- for (auto &operand : operands)
+ for (auto &operand : origOperands)
operand.~OpOperand();
// Initialize any new operands.
for (unsigned e = newSize; numOperands != e; ++numOperands)
new (&newOperands[numOperands]) OpOperand(owner);
- // If the current storage is also dynamic, free it.
- if (isDynamicStorage()) {
- // Work around -Wfree-nonheap-object false positive fixed by D102728.
- auto *mem = &storage;
- free(mem);
- }
+ // If the current storage is dynamic, free it.
+ if (isStorageDynamic)
+ free(operandStorage);
// Update the storage representation to use the new dynamic storage.
- dynamicStorage.setPointerAndInt(newStorage, true);
+ operandStorage = newOperandStorage;
+ capacity = newCapacity;
+ isStorageDynamic = true;
return newOperands;
}
@@ -394,9 +381,6 @@ MutableArrayRef<OpOperand> detail::OperandStorage::resize(Operation *owner,
//===----------------------------------------------------------------------===//
// OperandRange
-OperandRange::OperandRange(Operation *op)
- : OperandRange(op->getOpOperands().data(), op->getNumOperands()) {}
-
unsigned OperandRange::getBeginOperandIndex() const {
assert(!empty() && "range must not be empty");
return base->getOperandNumber();
More information about the Mlir-commits
mailing list