Skip to content

Commit 86ef031

Browse files
authored
[MLIR][Affine] Fix affine memory op verifiers to check valid dim/symbols properly (#127375)
The checks only checked if the operands were either valid dims and symbols without checking which ones should be valid dims and which should be valid symbols. The necessary methods to check already existed. With this fix, for some existing tests, it leads to a more accurate error message. In other cases, invalid IRs are caught as shown in the test case added. Related to: #120189
1 parent 39d1bd8 commit 86ef031

File tree

4 files changed

+27
-16
lines changed

4 files changed

+27
-16
lines changed

mlir/lib/Dialect/Affine/IR/AffineOps.cpp

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3048,8 +3048,9 @@ void AffineLoadOp::print(OpAsmPrinter &p) {
30483048

30493049
/// Verify common indexing invariants of affine.load, affine.store,
30503050
/// affine.vector_load and affine.vector_store.
3051+
template <typename AffineMemOpTy>
30513052
static LogicalResult
3052-
verifyMemoryOpIndexing(Operation *op, AffineMapAttr mapAttr,
3053+
verifyMemoryOpIndexing(AffineMemOpTy op, AffineMapAttr mapAttr,
30533054
Operation::operand_range mapOperands,
30543055
MemRefType memrefType, unsigned numIndexOperands) {
30553056
AffineMap map = mapAttr.getValue();
@@ -3058,14 +3059,12 @@ verifyMemoryOpIndexing(Operation *op, AffineMapAttr mapAttr,
30583059
if (map.getNumInputs() != numIndexOperands)
30593060
return op->emitOpError("expects as many subscripts as affine map inputs");
30603061

3061-
Region *scope = getAffineScope(op);
30623062
for (auto idx : mapOperands) {
30633063
if (!idx.getType().isIndex())
30643064
return op->emitOpError("index to load must have 'index' type");
3065-
if (!isValidAffineIndexOperand(idx, scope))
3066-
return op->emitOpError(
3067-
"index must be a valid dimension or symbol identifier");
30683065
}
3066+
if (failed(verifyDimAndSymbolIdentifiers(op, mapOperands, map.getNumDims())))
3067+
return failure();
30693068

30703069
return success();
30713070
}
@@ -3076,8 +3075,7 @@ LogicalResult AffineLoadOp::verify() {
30763075
return emitOpError("result type must match element type of memref");
30773076

30783077
if (failed(verifyMemoryOpIndexing(
3079-
getOperation(),
3080-
(*this)->getAttrOfType<AffineMapAttr>(getMapAttrStrName()),
3078+
*this, (*this)->getAttrOfType<AffineMapAttr>(getMapAttrStrName()),
30813079
getMapOperands(), memrefType,
30823080
/*numIndexOperands=*/getNumOperands() - 1)))
30833081
return failure();
@@ -3193,8 +3191,7 @@ LogicalResult AffineStoreOp::verify() {
31933191
"value to store must have the same type as memref element type");
31943192

31953193
if (failed(verifyMemoryOpIndexing(
3196-
getOperation(),
3197-
(*this)->getAttrOfType<AffineMapAttr>(getMapAttrStrName()),
3194+
*this, (*this)->getAttrOfType<AffineMapAttr>(getMapAttrStrName()),
31983195
getMapOperands(), memrefType,
31993196
/*numIndexOperands=*/getNumOperands() - 2)))
32003197
return failure();
@@ -4425,8 +4422,7 @@ static LogicalResult verifyVectorMemoryOp(Operation *op, MemRefType memrefType,
44254422
LogicalResult AffineVectorLoadOp::verify() {
44264423
MemRefType memrefType = getMemRefType();
44274424
if (failed(verifyMemoryOpIndexing(
4428-
getOperation(),
4429-
(*this)->getAttrOfType<AffineMapAttr>(getMapAttrStrName()),
4425+
*this, (*this)->getAttrOfType<AffineMapAttr>(getMapAttrStrName()),
44304426
getMapOperands(), memrefType,
44314427
/*numIndexOperands=*/getNumOperands() - 1)))
44324428
return failure();

mlir/test/Conversion/FuncToSPIRV/func-ops-to-spirv.mlir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func.func @dim_index_out_of_bounds() {
5555
%alloc_4 = memref.alloc() : memref<4xi64>
5656
%dim = memref.dim %alloc_4, %c6 : memref<4xi64>
5757
%alloca_100 = memref.alloca() : memref<100xi64>
58-
// expected-error@+1 {{'affine.vector_load' op index must be a valid dimension or symbol identifier}}
58+
// expected-error@+1 {{'affine.vector_load' op operand cannot be used as a dimension id}}
5959
%70 = affine.vector_load %alloca_100[%dim] : memref<100xi64>, vector<31xi64>
6060
return
6161
}

mlir/test/Dialect/Affine/invalid.mlir

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func.func @affine_load_invalid_dim(%M : memref<10xi32>) {
2525
"unknown"() ({
2626
^bb0(%arg: index):
2727
affine.load %M[%arg] : memref<10xi32>
28-
// expected-error@-1 {{index must be a valid dimension or symbol identifier}}
28+
// expected-error@-1 {{op operand cannot be used as a dimension id}}
2929
cf.br ^bb1
3030
^bb1:
3131
cf.br ^bb1
@@ -535,7 +535,7 @@ func.func @dynamic_dimension_index() {
535535
%idx = "unknown.test"() : () -> (index)
536536
%memref = "unknown.test"() : () -> memref<?x?xf32>
537537
%dim = memref.dim %memref, %idx : memref<?x?xf32>
538-
// expected-error @below {{op index must be a valid dimension or symbol identifier}}
538+
// expected-error@below {{op operand cannot be used as a dimension id}}
539539
affine.load %memref[%dim, %dim] : memref<?x?xf32>
540540
"unknown.terminator"() : () -> ()
541541
}) : () -> ()

mlir/test/Dialect/Affine/load-store-invalid.mlir

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func.func @load_non_affine_index(%arg0 : index) {
3737
%0 = memref.alloc() : memref<10xf32>
3838
affine.for %i0 = 0 to 10 {
3939
%1 = arith.muli %i0, %arg0 : index
40-
// expected-error@+1 {{op index must be a valid dimension or symbol identifier}}
40+
// expected-error@+1 {{op operand cannot be used as a dimension id}}
4141
%v = affine.load %0[%1] : memref<10xf32>
4242
}
4343
return
@@ -50,7 +50,7 @@ func.func @store_non_affine_index(%arg0 : index) {
5050
%1 = arith.constant 11.0 : f32
5151
affine.for %i0 = 0 to 10 {
5252
%2 = arith.muli %i0, %arg0 : index
53-
// expected-error@+1 {{op index must be a valid dimension or symbol identifier}}
53+
// expected-error@+1 {{op operand cannot be used as a dimension id}}
5454
affine.store %1, %0[%2] : memref<10xf32>
5555
}
5656
return
@@ -140,3 +140,18 @@ func.func @dma_wait_non_affine_tag_index(%arg0 : index) {
140140
}
141141
return
142142
}
143+
144+
// -----
145+
146+
func.func @invalid_symbol() {
147+
%alloc = memref.alloc() {alignment = 64 : i64} : memref<23x26xf32>
148+
affine.for %arg1 = 0 to 1 {
149+
affine.for %arg2 = 0 to 26 {
150+
affine.for %arg3 = 0 to 23 {
151+
affine.load %alloc[symbol(%arg1) * 23 + symbol(%arg3), %arg2] : memref<23x26xf32>
152+
// expected-error@above {{op operand cannot be used as a symbol}}
153+
}
154+
}
155+
}
156+
return
157+
}

0 commit comments

Comments
 (0)