[llvm-branch-commits] [mlir] c0185b2 - Revert "[mlir] Make single value `ValueRange`s memory safer (#121996)"
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Jan 16 03:31:34 PST 2025
Author: Emilio Cota
Date: 2025-01-16T11:31:31Z
New Revision: c0185b27a76cd23e745c7ad9db206fbc3ff7b11e
URL: https://github.com/llvm/llvm-project/commit/c0185b27a76cd23e745c7ad9db206fbc3ff7b11e
DIFF: https://github.com/llvm/llvm-project/commit/c0185b27a76cd23e745c7ad9db206fbc3ff7b11e.diff
LOG: Revert "[mlir] Make single value `ValueRange`s memory safer (#121996)"
This reverts commit ab6e63a0df9b67fb6ead026ce4ecdfd666991591.
Added:
Modified:
mlir/include/mlir/IR/TypeRange.h
mlir/include/mlir/IR/ValueRange.h
mlir/lib/IR/OperationSupport.cpp
mlir/lib/IR/TypeRange.cpp
mlir/unittests/IR/OperationSupportTest.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/IR/TypeRange.h b/mlir/include/mlir/IR/TypeRange.h
index fa63435b188e98..99fabab334f922 100644
--- a/mlir/include/mlir/IR/TypeRange.h
+++ b/mlir/include/mlir/IR/TypeRange.h
@@ -29,12 +29,11 @@ namespace mlir {
/// a SmallVector/std::vector. This class should be used in places that are not
/// suitable for a more derived type (e.g. ArrayRef) or a template range
/// parameter.
-class TypeRange
- : public llvm::detail::indexed_accessor_range_base<
- TypeRange,
- llvm::PointerUnion<const Value *, const Type *, OpOperand *,
- detail::OpResultImpl *, Type>,
- Type, Type, Type> {
+class TypeRange : public llvm::detail::indexed_accessor_range_base<
+ TypeRange,
+ llvm::PointerUnion<const Value *, const Type *,
+ OpOperand *, detail::OpResultImpl *>,
+ Type, Type, Type> {
public:
using RangeBaseT::RangeBaseT;
TypeRange(ArrayRef<Type> types = std::nullopt);
@@ -45,11 +44,8 @@ class TypeRange
TypeRange(ValueTypeRange<ValueRangeT> values)
: TypeRange(ValueRange(ValueRangeT(values.begin().getCurrent(),
values.end().getCurrent()))) {}
-
- TypeRange(Type type) : TypeRange(type, /*count=*/1) {}
- template <typename Arg, typename = std::enable_if_t<
- std::is_constructible_v<ArrayRef<Type>, Arg> &&
- !std::is_constructible_v<Type, Arg>>>
+ template <typename Arg, typename = std::enable_if_t<std::is_constructible<
+ ArrayRef<Type>, Arg>::value>>
TypeRange(Arg &&arg) : TypeRange(ArrayRef<Type>(std::forward<Arg>(arg))) {}
TypeRange(std::initializer_list<Type> types)
: TypeRange(ArrayRef<Type>(types)) {}
@@ -60,9 +56,8 @@ class TypeRange
/// * A pointer to the first element of an array of types.
/// * A pointer to the first element of an array of operands.
/// * A pointer to the first element of an array of results.
- /// * A single 'Type' instance.
using OwnerT = llvm::PointerUnion<const Value *, const Type *, OpOperand *,
- detail::OpResultImpl *, Type>;
+ detail::OpResultImpl *>;
/// See `llvm::detail::indexed_accessor_range_base` for details.
static OwnerT offset_base(OwnerT object, ptr
diff _t index);
diff --git a/mlir/include/mlir/IR/ValueRange.h b/mlir/include/mlir/IR/ValueRange.h
index d5b067a79200d8..4b421c08d8418e 100644
--- a/mlir/include/mlir/IR/ValueRange.h
+++ b/mlir/include/mlir/IR/ValueRange.h
@@ -374,16 +374,16 @@ class ResultRange::UseIterator final
/// SmallVector/std::vector. This class should be used in places that are not
/// suitable for a more derived type (e.g. ArrayRef) or a template range
/// parameter.
-class ValueRange final : public llvm::detail::indexed_accessor_range_base<
- ValueRange,
- PointerUnion<const Value *, OpOperand *,
- detail::OpResultImpl *, Value>,
- Value, Value, Value> {
+class ValueRange final
+ : public llvm::detail::indexed_accessor_range_base<
+ ValueRange,
+ PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *>,
+ Value, Value, Value> {
public:
/// The type representing the owner of a ValueRange. This is either a list of
- /// values, operands, or results or a single value.
+ /// values, operands, or results.
using OwnerT =
- PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *, Value>;
+ PointerUnion<const Value *, OpOperand *, detail::OpResultImpl *>;
using RangeBaseT::RangeBaseT;
@@ -392,7 +392,7 @@ class ValueRange final : public llvm::detail::indexed_accessor_range_base<
std::is_constructible<ArrayRef<Value>, Arg>::value &&
!std::is_convertible<Arg, Value>::value>>
ValueRange(Arg &&arg) : ValueRange(ArrayRef<Value>(std::forward<Arg>(arg))) {}
- ValueRange(Value value) : ValueRange(value, /*count=*/1) {}
+ ValueRange(const Value &value) : ValueRange(&value, /*count=*/1) {}
ValueRange(const std::initializer_list<Value> &values)
: ValueRange(ArrayRef<Value>(values)) {}
ValueRange(iterator_range<OperandRange::iterator> values)
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index 803fcd8d18fbd5..957195202d78d2 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -653,15 +653,6 @@ ValueRange::ValueRange(ResultRange values)
/// See `llvm::detail::indexed_accessor_range_base` for details.
ValueRange::OwnerT ValueRange::offset_base(const OwnerT &owner,
ptr
diff _t index) {
- if (llvm::isa_and_nonnull<Value>(owner)) {
- // Prevent out-of-bounds indexing for single values.
- // Note that we do allow an index of 1 as is required by 'slice'ing that
- // returns an empty range. This also matches the usual rules of C++ of being
- // allowed to index past the last element of an array.
- assert(index <= 1 && "out-of-bound offset into single-value 'ValueRange'");
- // Return nullptr to quickly cause segmentation faults on misuse.
- return index == 0 ? owner : nullptr;
- }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(owner))
return {value + index};
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
@@ -670,10 +661,6 @@ ValueRange::OwnerT ValueRange::offset_base(const OwnerT &owner,
}
/// See `llvm::detail::indexed_accessor_range_base` for details.
Value ValueRange::dereference_iterator(const OwnerT &owner, ptr
diff _t index) {
- if (auto value = llvm::dyn_cast_if_present<Value>(owner)) {
- assert(index == 0 && "cannot offset into single-value 'ValueRange'");
- return value;
- }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(owner))
return value[index];
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
diff --git a/mlir/lib/IR/TypeRange.cpp b/mlir/lib/IR/TypeRange.cpp
index 7e5f99c884512e..f8878303727d4f 100644
--- a/mlir/lib/IR/TypeRange.cpp
+++ b/mlir/lib/IR/TypeRange.cpp
@@ -31,23 +31,12 @@ TypeRange::TypeRange(ValueRange values) : TypeRange(OwnerT(), values.size()) {
this->base = result;
else if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(owner))
this->base = operand;
- else if (auto value = llvm::dyn_cast_if_present<Value>(owner))
- this->base = value.getType();
else
this->base = cast<const Value *>(owner);
}
/// See `llvm::detail::indexed_accessor_range_base` for details.
TypeRange::OwnerT TypeRange::offset_base(OwnerT object, ptr
diff _t index) {
- if (llvm::isa_and_nonnull<Type>(object)) {
- // Prevent out-of-bounds indexing for single values.
- // Note that we do allow an index of 1 as is required by 'slice'ing that
- // returns an empty range. This also matches the usual rules of C++ of being
- // allowed to index past the last element of an array.
- assert(index <= 1 && "out-of-bound offset into single-value 'ValueRange'");
- // Return nullptr to quickly cause segmentation faults on misuse.
- return index == 0 ? object : nullptr;
- }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(object))
return {value + index};
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(object))
@@ -59,10 +48,6 @@ TypeRange::OwnerT TypeRange::offset_base(OwnerT object, ptr
diff _t index) {
/// See `llvm::detail::indexed_accessor_range_base` for details.
Type TypeRange::dereference_iterator(OwnerT object, ptr
diff _t index) {
- if (auto type = llvm::dyn_cast_if_present<Type>(object)) {
- assert(index == 0 && "cannot offset into single-value 'TypeRange'");
- return type;
- }
if (const auto *value = llvm::dyn_cast_if_present<const Value *>(object))
return (value + index)->getType();
if (auto *operand = llvm::dyn_cast_if_present<OpOperand *>(object))
diff --git a/mlir/unittests/IR/OperationSupportTest.cpp b/mlir/unittests/IR/OperationSupportTest.cpp
index 2a1b8d2ef7f55b..f94dc784458077 100644
--- a/mlir/unittests/IR/OperationSupportTest.cpp
+++ b/mlir/unittests/IR/OperationSupportTest.cpp
@@ -313,21 +313,4 @@ TEST(OperationEquivalenceTest, HashWorksWithFlags) {
op2->destroy();
}
-TEST(ValueRangeTest, ValueConstructable) {
- MLIRContext context;
- Builder builder(&context);
-
- Operation *useOp =
- createOp(&context, /*operands=*/std::nullopt, builder.getIntegerType(16));
- // Valid construction despite a temporary 'OpResult'.
- ValueRange operands = useOp->getResult(0);
-
- useOp->setOperands(operands);
- EXPECT_EQ(useOp->getNumOperands(), 1u);
- EXPECT_EQ(useOp->getOperand(0), useOp->getResult(0));
-
- useOp->dropAllUses();
- useOp->destroy();
-}
-
} // namespace
More information about the llvm-branch-commits
mailing list