[mlir] Replace llvm::OwningArrayRef with std::vector
#168803
Merged
+33
−46
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Member
|
@llvm/pr-subscribers-mlir Author: David Stone (davidstone) ChangesThere are several places where we use Note that Full diff: https://github.com/llvm/llvm-project/pull/168803.diff 3 Files Affected:
diff --git a/mlir/include/mlir/IR/PDLPatternMatch.h.inc b/mlir/include/mlir/IR/PDLPatternMatch.h.inc
index d5fb57d7c360d..4afbcf2924965 100644
--- a/mlir/include/mlir/IR/PDLPatternMatch.h.inc
+++ b/mlir/include/mlir/IR/PDLPatternMatch.h.inc
@@ -152,9 +152,7 @@ public:
void push_back(TypeRange value) {
// The lifetime of a TypeRange can't be guaranteed, so we'll need to
// allocate a storage for it.
- llvm::OwningArrayRef<Type> storage(value.size());
- llvm::copy(value, storage.begin());
- allocatedTypeRanges.emplace_back(std::move(storage));
+ allocatedTypeRanges.emplace_back(value.begin(), value.end());
typeRanges.push_back(allocatedTypeRanges.back());
results.push_back(&typeRanges.back());
}
@@ -174,9 +172,7 @@ public:
void push_back(ValueRange value) {
// The lifetime of a ValueRange can't be guaranteed, so we'll need to
// allocate a storage for it.
- llvm::OwningArrayRef<Value> storage(value.size());
- llvm::copy(value, storage.begin());
- allocatedValueRanges.emplace_back(std::move(storage));
+ allocatedValueRanges.emplace_back(value.begin(), value.end());
valueRanges.push_back(allocatedValueRanges.back());
results.push_back(&valueRanges.back());
}
@@ -206,8 +202,8 @@ protected:
SmallVector<ValueRange> valueRanges;
/// Memory allocated to store ranges in the result list whose lifetime was
/// generated in the native function.
- SmallVector<llvm::OwningArrayRef<Type>> allocatedTypeRanges;
- SmallVector<llvm::OwningArrayRef<Value>> allocatedValueRanges;
+ SmallVector<std::vector<Type>> allocatedTypeRanges;
+ SmallVector<std::vector<Value>> allocatedValueRanges;
};
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Rewrite/ByteCode.cpp b/mlir/lib/Rewrite/ByteCode.cpp
index 42843ea1780c4..159aa54686034 100644
--- a/mlir/lib/Rewrite/ByteCode.cpp
+++ b/mlir/lib/Rewrite/ByteCode.cpp
@@ -1099,12 +1099,12 @@ class ByteCodeRewriteResultList : public PDLResultList {
MutableArrayRef<PDLValue> getResults() { return results; }
/// Return the type ranges allocated by this list.
- MutableArrayRef<llvm::OwningArrayRef<Type>> getAllocatedTypeRanges() {
+ MutableArrayRef<std::vector<Type>> getAllocatedTypeRanges() {
return allocatedTypeRanges;
}
/// Return the value ranges allocated by this list.
- MutableArrayRef<llvm::OwningArrayRef<Value>> getAllocatedValueRanges() {
+ MutableArrayRef<std::vector<Value>> getAllocatedValueRanges() {
return allocatedValueRanges;
}
};
@@ -1112,19 +1112,20 @@ class ByteCodeRewriteResultList : public PDLResultList {
/// This class provides support for executing a bytecode stream.
class ByteCodeExecutor {
public:
- ByteCodeExecutor(
- const ByteCodeField *curCodeIt, MutableArrayRef<const void *> memory,
- MutableArrayRef<llvm::OwningArrayRef<Operation *>> opRangeMemory,
- MutableArrayRef<TypeRange> typeRangeMemory,
- std::vector<llvm::OwningArrayRef<Type>> &allocatedTypeRangeMemory,
- MutableArrayRef<ValueRange> valueRangeMemory,
- std::vector<llvm::OwningArrayRef<Value>> &allocatedValueRangeMemory,
- MutableArrayRef<unsigned> loopIndex, ArrayRef<const void *> uniquedMemory,
- ArrayRef<ByteCodeField> code,
- ArrayRef<PatternBenefit> currentPatternBenefits,
- ArrayRef<PDLByteCodePattern> patterns,
- ArrayRef<PDLConstraintFunction> constraintFunctions,
- ArrayRef<PDLRewriteFunction> rewriteFunctions)
+ ByteCodeExecutor(const ByteCodeField *curCodeIt,
+ MutableArrayRef<const void *> memory,
+ MutableArrayRef<std::vector<Operation *>> opRangeMemory,
+ MutableArrayRef<TypeRange> typeRangeMemory,
+ std::vector<std::vector<Type>> &allocatedTypeRangeMemory,
+ MutableArrayRef<ValueRange> valueRangeMemory,
+ std::vector<std::vector<Value>> &allocatedValueRangeMemory,
+ MutableArrayRef<unsigned> loopIndex,
+ ArrayRef<const void *> uniquedMemory,
+ ArrayRef<ByteCodeField> code,
+ ArrayRef<PatternBenefit> currentPatternBenefits,
+ ArrayRef<PDLByteCodePattern> patterns,
+ ArrayRef<PDLConstraintFunction> constraintFunctions,
+ ArrayRef<PDLRewriteFunction> rewriteFunctions)
: curCodeIt(curCodeIt), memory(memory), opRangeMemory(opRangeMemory),
typeRangeMemory(typeRangeMemory),
allocatedTypeRangeMemory(allocatedTypeRangeMemory),
@@ -1367,13 +1368,9 @@ class ByteCodeExecutor {
if (range.empty()) {
rangeMemory[rangeIndex] = {};
} else {
- // Allocate a buffer for this type range.
- llvm::OwningArrayRef<T> storage(llvm::size(range));
- llvm::copy(range, storage.begin());
-
// Assign this to the range slot and use the range as the value for the
// memory index.
- allocatedRangeMemory.emplace_back(std::move(storage));
+ allocatedRangeMemory.emplace_back(range.begin(), range.end());
rangeMemory[rangeIndex] = allocatedRangeMemory.back();
}
memory[memIndex] = &rangeMemory[rangeIndex];
@@ -1397,11 +1394,11 @@ class ByteCodeExecutor {
/// The current execution memory.
MutableArrayRef<const void *> memory;
- MutableArrayRef<OwningOpRange> opRangeMemory;
+ MutableArrayRef<std::vector<Operation *>> opRangeMemory;
MutableArrayRef<TypeRange> typeRangeMemory;
- std::vector<llvm::OwningArrayRef<Type>> &allocatedTypeRangeMemory;
+ std::vector<std::vector<Type>> &allocatedTypeRangeMemory;
MutableArrayRef<ValueRange> valueRangeMemory;
- std::vector<llvm::OwningArrayRef<Value>> &allocatedValueRangeMemory;
+ std::vector<std::vector<Value>> &allocatedValueRangeMemory;
/// The current loop indices.
MutableArrayRef<unsigned> loopIndex;
@@ -1907,10 +1904,10 @@ void ByteCodeExecutor::executeGetUsers() {
LDBG() << "Executing GetUsers:";
unsigned memIndex = read();
unsigned rangeIndex = read();
- OwningOpRange &range = opRangeMemory[rangeIndex];
+ std::vector<Operation *> &range = opRangeMemory[rangeIndex];
memory[memIndex] = ⦥
- range = OwningOpRange();
+ range.clear();
if (read<PDLValue::Kind>() == PDLValue::Kind::Value) {
// Read the value.
Value value = read<Value>();
@@ -1918,9 +1915,7 @@ void ByteCodeExecutor::executeGetUsers() {
return;
LDBG() << " * Value: " << value;
- // Extract the users of a single value.
- range = OwningOpRange(std::distance(value.user_begin(), value.user_end()));
- llvm::copy(value.getUsers(), range.begin());
+ range.assign(value.user_begin(), value.user_end());
} else {
// Read a range of values.
ValueRange *values = read<ValueRange *>();
@@ -1929,12 +1924,8 @@ void ByteCodeExecutor::executeGetUsers() {
LDBG() << " * Values (" << values->size()
<< "): " << llvm::interleaved(*values);
- // Extract all the users of a range of values.
- SmallVector<Operation *> users;
for (Value value : *values)
- users.append(value.user_begin(), value.user_end());
- range = OwningOpRange(users.size());
- llvm::copy(users, range.begin());
+ range.insert(range.end(), value.user_begin(), value.user_end());
}
LDBG() << " * Result: " << range.size() << " operations";
@@ -2174,7 +2165,8 @@ ByteCodeExecutor::execute(PatternRewriter &rewriter,
executeEraseOp(rewriter);
break;
case ExtractOp:
- executeExtract<Operation *, OwningOpRange, PDLValue::Kind::Operation>();
+ executeExtract<Operation *, std::vector<Operation *>,
+ PDLValue::Kind::Operation>();
break;
case ExtractType:
executeExtract<Type, TypeRange, PDLValue::Kind::Type>();
diff --git a/mlir/lib/Rewrite/ByteCode.h b/mlir/lib/Rewrite/ByteCode.h
index 4aceac7ed3a4c..566c1cb66bdbd 100644
--- a/mlir/lib/Rewrite/ByteCode.h
+++ b/mlir/lib/Rewrite/ByteCode.h
@@ -30,7 +30,6 @@ class PDLByteCode;
/// entries. ByteCodeAddr refers to size of indices into the bytecode.
using ByteCodeField = uint16_t;
using ByteCodeAddr = uint32_t;
-using OwningOpRange = llvm::OwningArrayRef<Operation *>;
//===----------------------------------------------------------------------===//
// PDLByteCodePattern
@@ -94,21 +93,21 @@ class PDLByteCodeMutableState {
/// the bytecode to store ranges of operations. These are always stored by
/// owning references, because at no point in the execution of the byte code
/// we get an indexed range (view) of operations.
- std::vector<OwningOpRange> opRangeMemory;
+ std::vector<std::vector<Operation *>> opRangeMemory;
/// A mutable block of memory used during the matching and rewriting phase of
/// the bytecode to store ranges of types.
std::vector<TypeRange> typeRangeMemory;
/// A set of type ranges that have been allocated by the byte code interpreter
/// to provide a guaranteed lifetime.
- std::vector<llvm::OwningArrayRef<Type>> allocatedTypeRangeMemory;
+ std::vector<std::vector<Type>> allocatedTypeRangeMemory;
/// A mutable block of memory used during the matching and rewriting phase of
/// the bytecode to store ranges of values.
std::vector<ValueRange> valueRangeMemory;
/// A set of value ranges that have been allocated by the byte code
/// interpreter to provide a guaranteed lifetime.
- std::vector<llvm::OwningArrayRef<Value>> allocatedValueRangeMemory;
+ std::vector<std::vector<Value>> allocatedValueRangeMemory;
/// The current index of ranges being iterated over for each level of nesting.
/// These are always maintained at 0 for the loops that are not active, so we
|
Member
|
@llvm/pr-subscribers-mlir-core Author: David Stone (davidstone) ChangesThere are several places where we use Note that Full diff: https://github.com/llvm/llvm-project/pull/168803.diff 3 Files Affected:
diff --git a/mlir/include/mlir/IR/PDLPatternMatch.h.inc b/mlir/include/mlir/IR/PDLPatternMatch.h.inc
index d5fb57d7c360d..4afbcf2924965 100644
--- a/mlir/include/mlir/IR/PDLPatternMatch.h.inc
+++ b/mlir/include/mlir/IR/PDLPatternMatch.h.inc
@@ -152,9 +152,7 @@ public:
void push_back(TypeRange value) {
// The lifetime of a TypeRange can't be guaranteed, so we'll need to
// allocate a storage for it.
- llvm::OwningArrayRef<Type> storage(value.size());
- llvm::copy(value, storage.begin());
- allocatedTypeRanges.emplace_back(std::move(storage));
+ allocatedTypeRanges.emplace_back(value.begin(), value.end());
typeRanges.push_back(allocatedTypeRanges.back());
results.push_back(&typeRanges.back());
}
@@ -174,9 +172,7 @@ public:
void push_back(ValueRange value) {
// The lifetime of a ValueRange can't be guaranteed, so we'll need to
// allocate a storage for it.
- llvm::OwningArrayRef<Value> storage(value.size());
- llvm::copy(value, storage.begin());
- allocatedValueRanges.emplace_back(std::move(storage));
+ allocatedValueRanges.emplace_back(value.begin(), value.end());
valueRanges.push_back(allocatedValueRanges.back());
results.push_back(&valueRanges.back());
}
@@ -206,8 +202,8 @@ protected:
SmallVector<ValueRange> valueRanges;
/// Memory allocated to store ranges in the result list whose lifetime was
/// generated in the native function.
- SmallVector<llvm::OwningArrayRef<Type>> allocatedTypeRanges;
- SmallVector<llvm::OwningArrayRef<Value>> allocatedValueRanges;
+ SmallVector<std::vector<Type>> allocatedTypeRanges;
+ SmallVector<std::vector<Value>> allocatedValueRanges;
};
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Rewrite/ByteCode.cpp b/mlir/lib/Rewrite/ByteCode.cpp
index 42843ea1780c4..159aa54686034 100644
--- a/mlir/lib/Rewrite/ByteCode.cpp
+++ b/mlir/lib/Rewrite/ByteCode.cpp
@@ -1099,12 +1099,12 @@ class ByteCodeRewriteResultList : public PDLResultList {
MutableArrayRef<PDLValue> getResults() { return results; }
/// Return the type ranges allocated by this list.
- MutableArrayRef<llvm::OwningArrayRef<Type>> getAllocatedTypeRanges() {
+ MutableArrayRef<std::vector<Type>> getAllocatedTypeRanges() {
return allocatedTypeRanges;
}
/// Return the value ranges allocated by this list.
- MutableArrayRef<llvm::OwningArrayRef<Value>> getAllocatedValueRanges() {
+ MutableArrayRef<std::vector<Value>> getAllocatedValueRanges() {
return allocatedValueRanges;
}
};
@@ -1112,19 +1112,20 @@ class ByteCodeRewriteResultList : public PDLResultList {
/// This class provides support for executing a bytecode stream.
class ByteCodeExecutor {
public:
- ByteCodeExecutor(
- const ByteCodeField *curCodeIt, MutableArrayRef<const void *> memory,
- MutableArrayRef<llvm::OwningArrayRef<Operation *>> opRangeMemory,
- MutableArrayRef<TypeRange> typeRangeMemory,
- std::vector<llvm::OwningArrayRef<Type>> &allocatedTypeRangeMemory,
- MutableArrayRef<ValueRange> valueRangeMemory,
- std::vector<llvm::OwningArrayRef<Value>> &allocatedValueRangeMemory,
- MutableArrayRef<unsigned> loopIndex, ArrayRef<const void *> uniquedMemory,
- ArrayRef<ByteCodeField> code,
- ArrayRef<PatternBenefit> currentPatternBenefits,
- ArrayRef<PDLByteCodePattern> patterns,
- ArrayRef<PDLConstraintFunction> constraintFunctions,
- ArrayRef<PDLRewriteFunction> rewriteFunctions)
+ ByteCodeExecutor(const ByteCodeField *curCodeIt,
+ MutableArrayRef<const void *> memory,
+ MutableArrayRef<std::vector<Operation *>> opRangeMemory,
+ MutableArrayRef<TypeRange> typeRangeMemory,
+ std::vector<std::vector<Type>> &allocatedTypeRangeMemory,
+ MutableArrayRef<ValueRange> valueRangeMemory,
+ std::vector<std::vector<Value>> &allocatedValueRangeMemory,
+ MutableArrayRef<unsigned> loopIndex,
+ ArrayRef<const void *> uniquedMemory,
+ ArrayRef<ByteCodeField> code,
+ ArrayRef<PatternBenefit> currentPatternBenefits,
+ ArrayRef<PDLByteCodePattern> patterns,
+ ArrayRef<PDLConstraintFunction> constraintFunctions,
+ ArrayRef<PDLRewriteFunction> rewriteFunctions)
: curCodeIt(curCodeIt), memory(memory), opRangeMemory(opRangeMemory),
typeRangeMemory(typeRangeMemory),
allocatedTypeRangeMemory(allocatedTypeRangeMemory),
@@ -1367,13 +1368,9 @@ class ByteCodeExecutor {
if (range.empty()) {
rangeMemory[rangeIndex] = {};
} else {
- // Allocate a buffer for this type range.
- llvm::OwningArrayRef<T> storage(llvm::size(range));
- llvm::copy(range, storage.begin());
-
// Assign this to the range slot and use the range as the value for the
// memory index.
- allocatedRangeMemory.emplace_back(std::move(storage));
+ allocatedRangeMemory.emplace_back(range.begin(), range.end());
rangeMemory[rangeIndex] = allocatedRangeMemory.back();
}
memory[memIndex] = &rangeMemory[rangeIndex];
@@ -1397,11 +1394,11 @@ class ByteCodeExecutor {
/// The current execution memory.
MutableArrayRef<const void *> memory;
- MutableArrayRef<OwningOpRange> opRangeMemory;
+ MutableArrayRef<std::vector<Operation *>> opRangeMemory;
MutableArrayRef<TypeRange> typeRangeMemory;
- std::vector<llvm::OwningArrayRef<Type>> &allocatedTypeRangeMemory;
+ std::vector<std::vector<Type>> &allocatedTypeRangeMemory;
MutableArrayRef<ValueRange> valueRangeMemory;
- std::vector<llvm::OwningArrayRef<Value>> &allocatedValueRangeMemory;
+ std::vector<std::vector<Value>> &allocatedValueRangeMemory;
/// The current loop indices.
MutableArrayRef<unsigned> loopIndex;
@@ -1907,10 +1904,10 @@ void ByteCodeExecutor::executeGetUsers() {
LDBG() << "Executing GetUsers:";
unsigned memIndex = read();
unsigned rangeIndex = read();
- OwningOpRange &range = opRangeMemory[rangeIndex];
+ std::vector<Operation *> &range = opRangeMemory[rangeIndex];
memory[memIndex] = ⦥
- range = OwningOpRange();
+ range.clear();
if (read<PDLValue::Kind>() == PDLValue::Kind::Value) {
// Read the value.
Value value = read<Value>();
@@ -1918,9 +1915,7 @@ void ByteCodeExecutor::executeGetUsers() {
return;
LDBG() << " * Value: " << value;
- // Extract the users of a single value.
- range = OwningOpRange(std::distance(value.user_begin(), value.user_end()));
- llvm::copy(value.getUsers(), range.begin());
+ range.assign(value.user_begin(), value.user_end());
} else {
// Read a range of values.
ValueRange *values = read<ValueRange *>();
@@ -1929,12 +1924,8 @@ void ByteCodeExecutor::executeGetUsers() {
LDBG() << " * Values (" << values->size()
<< "): " << llvm::interleaved(*values);
- // Extract all the users of a range of values.
- SmallVector<Operation *> users;
for (Value value : *values)
- users.append(value.user_begin(), value.user_end());
- range = OwningOpRange(users.size());
- llvm::copy(users, range.begin());
+ range.insert(range.end(), value.user_begin(), value.user_end());
}
LDBG() << " * Result: " << range.size() << " operations";
@@ -2174,7 +2165,8 @@ ByteCodeExecutor::execute(PatternRewriter &rewriter,
executeEraseOp(rewriter);
break;
case ExtractOp:
- executeExtract<Operation *, OwningOpRange, PDLValue::Kind::Operation>();
+ executeExtract<Operation *, std::vector<Operation *>,
+ PDLValue::Kind::Operation>();
break;
case ExtractType:
executeExtract<Type, TypeRange, PDLValue::Kind::Type>();
diff --git a/mlir/lib/Rewrite/ByteCode.h b/mlir/lib/Rewrite/ByteCode.h
index 4aceac7ed3a4c..566c1cb66bdbd 100644
--- a/mlir/lib/Rewrite/ByteCode.h
+++ b/mlir/lib/Rewrite/ByteCode.h
@@ -30,7 +30,6 @@ class PDLByteCode;
/// entries. ByteCodeAddr refers to size of indices into the bytecode.
using ByteCodeField = uint16_t;
using ByteCodeAddr = uint32_t;
-using OwningOpRange = llvm::OwningArrayRef<Operation *>;
//===----------------------------------------------------------------------===//
// PDLByteCodePattern
@@ -94,21 +93,21 @@ class PDLByteCodeMutableState {
/// the bytecode to store ranges of operations. These are always stored by
/// owning references, because at no point in the execution of the byte code
/// we get an indexed range (view) of operations.
- std::vector<OwningOpRange> opRangeMemory;
+ std::vector<std::vector<Operation *>> opRangeMemory;
/// A mutable block of memory used during the matching and rewriting phase of
/// the bytecode to store ranges of types.
std::vector<TypeRange> typeRangeMemory;
/// A set of type ranges that have been allocated by the byte code interpreter
/// to provide a guaranteed lifetime.
- std::vector<llvm::OwningArrayRef<Type>> allocatedTypeRangeMemory;
+ std::vector<std::vector<Type>> allocatedTypeRangeMemory;
/// A mutable block of memory used during the matching and rewriting phase of
/// the bytecode to store ranges of values.
std::vector<ValueRange> valueRangeMemory;
/// A set of value ranges that have been allocated by the byte code
/// interpreter to provide a guaranteed lifetime.
- std::vector<llvm::OwningArrayRef<Value>> allocatedValueRangeMemory;
+ std::vector<std::vector<Value>> allocatedValueRangeMemory;
/// The current index of ranges being iterated over for each level of nesting.
/// These are always maintained at 0 for the loops that are not active, so we
|
davidstone
commented
Nov 20, 2025
| memory[memIndex] = ⦥ | ||
|
|
||
| range = OwningOpRange(); | ||
| range.clear(); |
Contributor
Author
There was a problem hiding this comment.
One note on this line. The old behavior always deallocated memory at this line and then immediately reallocated memory later on. That is much worse for performance but ensures we always use exactly the amount of memory we need. This version holds onto the old memory and allocates only if the new memory requirements are greater than the previous allocation. This avoids memory allocations, but if we have one very large allocation followed by much smaller allocations this will use more memory. I don't understand the ByteCode use case well enough to know if this is an issue. I suspect the old behavior was chosen because that is the only choice you have with llvm::OwningArrayRef, not out of a deliberate design decision, but I want to call out this change. It's a trivial change to go back to the old memory usage pattern.
🐧 Linux x64 Test Results
|
davidstone
deleted the
mlir-use-std-vector-instead-of-owning-array-ref
branch
aadeshps-mcw
pushed a commit
to aadeshps-mcw/llvm-project
that referenced
this pull request
Nov 26, 2025
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.
Priyanshu3820
pushed a commit
to Priyanshu3820/llvm-project
that referenced
this pull request
Nov 26, 2025
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 usingstd::vector. In one case we actually allocate a completely separate container and then allocate + copy the data over becausellvm::OwningArrayRefdoes not (and can't) supportpush_back.Note that
llvm::SmallVectoris 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, andllvm::SmallVectordoes not give us that guarantee.