Skip to content

Commit bfbd191

Browse files
authored
[mlir] Replace llvm::OwningArrayRef with std::vector (#168803)
There are several places where we use `llvm::OwningArrayRef`. The interface to this requires us to first construct temporary storage, then allocate space and set the allocated memory to 0, then copy the values we actually want into that memory, then move the array into place. Instead we can just do it all inline in a single pass by using `std::vector`. In one case we actually allocate a completely separate container and then allocate + copy the data over because `llvm::OwningArrayRef` does not (and can't) support `push_back`. Note that `llvm::SmallVector` is not a suitable replacement here because we rely on reference stability on move construction: when the outer container reallocates, we need the the contents of the inner containers to be fixed in memory, and `llvm::SmallVector` does not give us that guarantee.
1 parent 01e5e4f commit bfbd191

File tree

3 files changed

+33
-46
lines changed

3 files changed

+33
-46
lines changed

mlir/include/mlir/IR/PDLPatternMatch.h.inc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,7 @@ public:
152152
void push_back(TypeRange value) {
153153
// The lifetime of a TypeRange can't be guaranteed, so we'll need to
154154
// allocate a storage for it.
155-
llvm::OwningArrayRef<Type> storage(value.size());
156-
llvm::copy(value, storage.begin());
157-
allocatedTypeRanges.emplace_back(std::move(storage));
155+
allocatedTypeRanges.emplace_back(value.begin(), value.end());
158156
typeRanges.push_back(allocatedTypeRanges.back());
159157
results.push_back(&typeRanges.back());
160158
}
@@ -174,9 +172,7 @@ public:
174172
void push_back(ValueRange value) {
175173
// The lifetime of a ValueRange can't be guaranteed, so we'll need to
176174
// allocate a storage for it.
177-
llvm::OwningArrayRef<Value> storage(value.size());
178-
llvm::copy(value, storage.begin());
179-
allocatedValueRanges.emplace_back(std::move(storage));
175+
allocatedValueRanges.emplace_back(value.begin(), value.end());
180176
valueRanges.push_back(allocatedValueRanges.back());
181177
results.push_back(&valueRanges.back());
182178
}
@@ -206,8 +202,8 @@ protected:
206202
SmallVector<ValueRange> valueRanges;
207203
/// Memory allocated to store ranges in the result list whose lifetime was
208204
/// generated in the native function.
209-
SmallVector<llvm::OwningArrayRef<Type>> allocatedTypeRanges;
210-
SmallVector<llvm::OwningArrayRef<Value>> allocatedValueRanges;
205+
SmallVector<std::vector<Type>> allocatedTypeRanges;
206+
SmallVector<std::vector<Value>> allocatedValueRanges;
211207
};
212208

213209
//===----------------------------------------------------------------------===//

mlir/lib/Rewrite/ByteCode.cpp

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,32 +1099,33 @@ class ByteCodeRewriteResultList : public PDLResultList {
10991099
MutableArrayRef<PDLValue> getResults() { return results; }
11001100

11011101
/// Return the type ranges allocated by this list.
1102-
MutableArrayRef<llvm::OwningArrayRef<Type>> getAllocatedTypeRanges() {
1102+
MutableArrayRef<std::vector<Type>> getAllocatedTypeRanges() {
11031103
return allocatedTypeRanges;
11041104
}
11051105

11061106
/// Return the value ranges allocated by this list.
1107-
MutableArrayRef<llvm::OwningArrayRef<Value>> getAllocatedValueRanges() {
1107+
MutableArrayRef<std::vector<Value>> getAllocatedValueRanges() {
11081108
return allocatedValueRanges;
11091109
}
11101110
};
11111111

11121112
/// This class provides support for executing a bytecode stream.
11131113
class ByteCodeExecutor {
11141114
public:
1115-
ByteCodeExecutor(
1116-
const ByteCodeField *curCodeIt, MutableArrayRef<const void *> memory,
1117-
MutableArrayRef<llvm::OwningArrayRef<Operation *>> opRangeMemory,
1118-
MutableArrayRef<TypeRange> typeRangeMemory,
1119-
std::vector<llvm::OwningArrayRef<Type>> &allocatedTypeRangeMemory,
1120-
MutableArrayRef<ValueRange> valueRangeMemory,
1121-
std::vector<llvm::OwningArrayRef<Value>> &allocatedValueRangeMemory,
1122-
MutableArrayRef<unsigned> loopIndex, ArrayRef<const void *> uniquedMemory,
1123-
ArrayRef<ByteCodeField> code,
1124-
ArrayRef<PatternBenefit> currentPatternBenefits,
1125-
ArrayRef<PDLByteCodePattern> patterns,
1126-
ArrayRef<PDLConstraintFunction> constraintFunctions,
1127-
ArrayRef<PDLRewriteFunction> rewriteFunctions)
1115+
ByteCodeExecutor(const ByteCodeField *curCodeIt,
1116+
MutableArrayRef<const void *> memory,
1117+
MutableArrayRef<std::vector<Operation *>> opRangeMemory,
1118+
MutableArrayRef<TypeRange> typeRangeMemory,
1119+
std::vector<std::vector<Type>> &allocatedTypeRangeMemory,
1120+
MutableArrayRef<ValueRange> valueRangeMemory,
1121+
std::vector<std::vector<Value>> &allocatedValueRangeMemory,
1122+
MutableArrayRef<unsigned> loopIndex,
1123+
ArrayRef<const void *> uniquedMemory,
1124+
ArrayRef<ByteCodeField> code,
1125+
ArrayRef<PatternBenefit> currentPatternBenefits,
1126+
ArrayRef<PDLByteCodePattern> patterns,
1127+
ArrayRef<PDLConstraintFunction> constraintFunctions,
1128+
ArrayRef<PDLRewriteFunction> rewriteFunctions)
11281129
: curCodeIt(curCodeIt), memory(memory), opRangeMemory(opRangeMemory),
11291130
typeRangeMemory(typeRangeMemory),
11301131
allocatedTypeRangeMemory(allocatedTypeRangeMemory),
@@ -1367,13 +1368,9 @@ class ByteCodeExecutor {
13671368
if (range.empty()) {
13681369
rangeMemory[rangeIndex] = {};
13691370
} else {
1370-
// Allocate a buffer for this type range.
1371-
llvm::OwningArrayRef<T> storage(llvm::size(range));
1372-
llvm::copy(range, storage.begin());
1373-
13741371
// Assign this to the range slot and use the range as the value for the
13751372
// memory index.
1376-
allocatedRangeMemory.emplace_back(std::move(storage));
1373+
allocatedRangeMemory.emplace_back(range.begin(), range.end());
13771374
rangeMemory[rangeIndex] = allocatedRangeMemory.back();
13781375
}
13791376
memory[memIndex] = &rangeMemory[rangeIndex];
@@ -1397,11 +1394,11 @@ class ByteCodeExecutor {
13971394

13981395
/// The current execution memory.
13991396
MutableArrayRef<const void *> memory;
1400-
MutableArrayRef<OwningOpRange> opRangeMemory;
1397+
MutableArrayRef<std::vector<Operation *>> opRangeMemory;
14011398
MutableArrayRef<TypeRange> typeRangeMemory;
1402-
std::vector<llvm::OwningArrayRef<Type>> &allocatedTypeRangeMemory;
1399+
std::vector<std::vector<Type>> &allocatedTypeRangeMemory;
14031400
MutableArrayRef<ValueRange> valueRangeMemory;
1404-
std::vector<llvm::OwningArrayRef<Value>> &allocatedValueRangeMemory;
1401+
std::vector<std::vector<Value>> &allocatedValueRangeMemory;
14051402

14061403
/// The current loop indices.
14071404
MutableArrayRef<unsigned> loopIndex;
@@ -1907,20 +1904,18 @@ void ByteCodeExecutor::executeGetUsers() {
19071904
LDBG() << "Executing GetUsers:";
19081905
unsigned memIndex = read();
19091906
unsigned rangeIndex = read();
1910-
OwningOpRange &range = opRangeMemory[rangeIndex];
1907+
std::vector<Operation *> &range = opRangeMemory[rangeIndex];
19111908
memory[memIndex] = &range;
19121909

1913-
range = OwningOpRange();
1910+
range.clear();
19141911
if (read<PDLValue::Kind>() == PDLValue::Kind::Value) {
19151912
// Read the value.
19161913
Value value = read<Value>();
19171914
if (!value)
19181915
return;
19191916
LDBG() << " * Value: " << value;
19201917

1921-
// Extract the users of a single value.
1922-
range = OwningOpRange(std::distance(value.user_begin(), value.user_end()));
1923-
llvm::copy(value.getUsers(), range.begin());
1918+
range.assign(value.user_begin(), value.user_end());
19241919
} else {
19251920
// Read a range of values.
19261921
ValueRange *values = read<ValueRange *>();
@@ -1929,12 +1924,8 @@ void ByteCodeExecutor::executeGetUsers() {
19291924
LDBG() << " * Values (" << values->size()
19301925
<< "): " << llvm::interleaved(*values);
19311926

1932-
// Extract all the users of a range of values.
1933-
SmallVector<Operation *> users;
19341927
for (Value value : *values)
1935-
users.append(value.user_begin(), value.user_end());
1936-
range = OwningOpRange(users.size());
1937-
llvm::copy(users, range.begin());
1928+
range.insert(range.end(), value.user_begin(), value.user_end());
19381929
}
19391930

19401931
LDBG() << " * Result: " << range.size() << " operations";
@@ -2174,7 +2165,8 @@ ByteCodeExecutor::execute(PatternRewriter &rewriter,
21742165
executeEraseOp(rewriter);
21752166
break;
21762167
case ExtractOp:
2177-
executeExtract<Operation *, OwningOpRange, PDLValue::Kind::Operation>();
2168+
executeExtract<Operation *, std::vector<Operation *>,
2169+
PDLValue::Kind::Operation>();
21782170
break;
21792171
case ExtractType:
21802172
executeExtract<Type, TypeRange, PDLValue::Kind::Type>();

mlir/lib/Rewrite/ByteCode.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ class PDLByteCode;
3030
/// entries. ByteCodeAddr refers to size of indices into the bytecode.
3131
using ByteCodeField = uint16_t;
3232
using ByteCodeAddr = uint32_t;
33-
using OwningOpRange = llvm::OwningArrayRef<Operation *>;
3433

3534
//===----------------------------------------------------------------------===//
3635
// PDLByteCodePattern
@@ -94,21 +93,21 @@ class PDLByteCodeMutableState {
9493
/// the bytecode to store ranges of operations. These are always stored by
9594
/// owning references, because at no point in the execution of the byte code
9695
/// we get an indexed range (view) of operations.
97-
std::vector<OwningOpRange> opRangeMemory;
96+
std::vector<std::vector<Operation *>> opRangeMemory;
9897

9998
/// A mutable block of memory used during the matching and rewriting phase of
10099
/// the bytecode to store ranges of types.
101100
std::vector<TypeRange> typeRangeMemory;
102101
/// A set of type ranges that have been allocated by the byte code interpreter
103102
/// to provide a guaranteed lifetime.
104-
std::vector<llvm::OwningArrayRef<Type>> allocatedTypeRangeMemory;
103+
std::vector<std::vector<Type>> allocatedTypeRangeMemory;
105104

106105
/// A mutable block of memory used during the matching and rewriting phase of
107106
/// the bytecode to store ranges of values.
108107
std::vector<ValueRange> valueRangeMemory;
109108
/// A set of value ranges that have been allocated by the byte code
110109
/// interpreter to provide a guaranteed lifetime.
111-
std::vector<llvm::OwningArrayRef<Value>> allocatedValueRangeMemory;
110+
std::vector<std::vector<Value>> allocatedValueRangeMemory;
112111

113112
/// The current index of ranges being iterated over for each level of nesting.
114113
/// These are always maintained at 0 for the loops that are not active, so we

0 commit comments

Comments
 (0)