Skip to content

[mlir] ViewLikeInterface - verify ranks in verifyOffsetSizeAndStrideOp #147926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion mlir/lib/Dialect/Utils/StaticValueUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,16 @@ bool isEqualConstantIntOrValueArray(ArrayRef<OpFoldResult> ofrs1,
return true;
}

/// Return a vector of OpFoldResults with the same size a staticValues, but all
/// Return a vector of OpFoldResults with the same size as staticValues, but all
/// elements for which ShapedType::isDynamic is true, will be replaced by
/// dynamicValues.
SmallVector<OpFoldResult> getMixedValues(ArrayRef<int64_t> staticValues,
ValueRange dynamicValues,
MLIRContext *context) {
assert(dynamicValues.size() ==
(unsigned)llvm::count_if(staticValues, ShapedType::isDynamic) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use C++ style static_cast and size_t.

"expected the rank of dynamic values to match the number of "
"values known to be dynamic");
SmallVector<OpFoldResult> res;
res.reserve(staticValues.size());
unsigned numDynamic = 0;
Expand Down
26 changes: 26 additions & 0 deletions mlir/lib/Interfaces/ViewLikeInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,32 @@ SliceBoundsVerificationResult mlir::verifyInBoundsSlice(

LogicalResult
mlir::detail::verifyOffsetSizeAndStrideOp(OffsetSizeAndStrideOpInterface op) {
// A dynamic size is represented as ShapedType::kDynamic in `static_sizes`.
// Its corresponding Value appears in `sizes`. Thus, the number of dynamic
// dimensions in `static_sizes` must equal the rank of `sizes`.
// The same applies to strides and offsets.
unsigned int numDynamicDims =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto or size_t?

llvm::count_if(op.getStaticSizes(), ShapedType::isDynamic);
if (op.getSizes().size() != numDynamicDims) {
return op->emitError("expected sizes rank to match the number of dynamic "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I wouldn't use the term "rank" because users may think of the rank of the source

e.g.:

expected the number of 'sizes' to match the number of dynamic entries in 'static_sizes'

"dimensions (")
<< op.getSizes().size() << " vs " << numDynamicDims << ")";
}
unsigned int numDynamicStrides =
llvm::count_if(op.getStaticStrides(), ShapedType::isDynamic);
if (op.getStrides().size() != numDynamicStrides) {
return op->emitError("expected strides rank to match the number of dynamic "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

"strides (")
<< op.getStrides().size() << " vs " << numDynamicStrides << ")";
}
unsigned int numDynamicOffsets =
llvm::count_if(op.getStaticOffsets(), ShapedType::isDynamic);
if (op.getOffsets().size() != numDynamicOffsets) {
return op->emitError("expected offsets rank to match the number of dynamic "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

"offsets (")
<< op.getOffsets().size() << " vs " << numDynamicOffsets << ")";
}

std::array<unsigned, 3> maxRanks = op.getArrayAttrMaxRanks();
// Offsets can come in 2 flavors:
// 1. Either single entry (when maxRanks == 1).
Expand Down
12 changes: 12 additions & 0 deletions mlir/test/Dialect/MemRef/invalid.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,18 @@ func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {

// -----

// This test is not written in the op's assembly format, to reproduce a mismatch
// between the rank of static_offsets and the number of Values sent as the
// dynamic offsets.
func.func @invalid_subview(%arg0 : memref<?x128xi8, 1>) {
%0 = memref.alloc() :memref<1xf32>
// expected-error@+1 {{expected offsets rank to match the number of dynamic offsets (0 vs 1)}}
"memref.subview"(%0) <{operandSegmentSizes = array<i32: 1, 0, 0, 0>, static_offsets = array<i64: -9223372036854775808>, static_sizes = array<i64: 1>, static_strides = array<i64: 1>}> : (memref<1xf32>) -> memref<1xf32, strided<[1], offset: ?>>
return
}

// -----

func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
%0 = memref.alloc() : memref<8x16x4xf32>
// expected-error@+1 {{expected mixed sizes rank to match mixed strides rank (3 vs 2) so the rank of the result type is well-formed}}
Expand Down
Loading