Skip to content

Commit d69f5e1

Browse files
nicolasvasilachenicolasvasilache
committed
[mlir][memref] Fix subview offset verification.
Offset-specific verification seems to have been lost in one of the recent refactorings. Also add proper tests that would have caught this omission. This addresses the immediate issues discussed in: https://llvm.discourse.group/t/memref-subview-affine-map-and-symbols/4851 Differential Revision: https://reviews.llvm.org/D115427
1 parent cd11312 commit d69f5e1

File tree

4 files changed

+194
-136
lines changed

4 files changed

+194
-136
lines changed

‎mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp

+20-10
Original file line numberDiff line numberDiff line change
@@ -1707,32 +1707,42 @@ void SubViewOp::build(OpBuilder &b, OperationState &result, Value source,
17071707
/// For ViewLikeOpInterface.
17081708
Value SubViewOp::getViewSource() { return source(); }
17091709

1710+
/// Return true if t1 and t2 have equal offsets (both dynamic or of same static
1711+
/// value).
1712+
static bool haveCompatibleOffsets(MemRefType t1, MemRefType t2) {
1713+
AffineExpr t1Offset, t2Offset;
1714+
SmallVector<AffineExpr> t1Strides, t2Strides;
1715+
auto res1 = getStridesAndOffset(t1, t1Strides, t1Offset);
1716+
auto res2 = getStridesAndOffset(t2, t2Strides, t2Offset);
1717+
return succeeded(res1) && succeeded(res2) && t1Offset == t2Offset;
1718+
}
1719+
17101720
/// Checks if `original` Type type can be rank reduced to `reduced` type.
17111721
/// This function is slight variant of `is subsequence` algorithm where
17121722
/// not matching dimension must be 1.
17131723
static SliceVerificationResult
17141724
isRankReducedMemRefType(MemRefType originalType,
1715-
MemRefType candidatecandidateReducedType,
1725+
MemRefType candidateRankReducedType,
17161726
ArrayRef<OpFoldResult> sizes) {
1717-
auto partialRes =
1718-
isRankReducedType(originalType, candidatecandidateReducedType);
1727+
auto partialRes = isRankReducedType(originalType, candidateRankReducedType);
17191728
if (partialRes != SliceVerificationResult::Success)
17201729
return partialRes;
17211730

1722-
MemRefType original = originalType.cast<MemRefType>();
1723-
MemRefType candidateReduced =
1724-
candidatecandidateReducedType.cast<MemRefType>();
1725-
1726-
auto optionalUnusedDimsMask =
1727-
computeMemRefRankReductionMask(original, candidateReduced, sizes);
1731+
auto optionalUnusedDimsMask = computeMemRefRankReductionMask(
1732+
originalType, candidateRankReducedType, sizes);
17281733

17291734
// Sizes cannot be matched in case empty vector is returned.
17301735
if (!optionalUnusedDimsMask.hasValue())
17311736
return SliceVerificationResult::LayoutMismatch;
17321737

1733-
if (original.getMemorySpace() != candidateReduced.getMemorySpace())
1738+
if (originalType.getMemorySpace() !=
1739+
candidateRankReducedType.getMemorySpace())
17341740
return SliceVerificationResult::MemSpaceMismatch;
17351741

1742+
// No amount of stride dropping can reconcile incompatible offsets.
1743+
if (!haveCompatibleOffsets(originalType, candidateRankReducedType))
1744+
return SliceVerificationResult::LayoutMismatch;
1745+
17361746
return SliceVerificationResult::Success;
17371747
}
17381748

‎mlir/test/Dialect/MemRef/invalid.mlir

+36
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,42 @@ func @static_stride_to_dynamic_stride(%arg0 : memref<?x?x?xf32>, %arg1 : index,
660660

661661
// -----
662662

663+
#map0 = affine_map<(d0, d1)[s0] -> (d0 * 16 + d1)>
664+
665+
func @subview_bad_offset_1(%arg0: memref<16x16xf32>) {
666+
%c0 = arith.constant 0 : index
667+
%c8 = arith.constant 8 : index
668+
// expected-error @+1 {{expected result type to be 'memref<8x8xf32, affine_map<(d0, d1)[s0] -> (d0 * 16 + s0 + d1)>>' or a rank-reduced version}}
669+
%s2 = memref.subview %arg0[%c8, %c8][8, 8][1, 1] : memref<16x16xf32> to memref<8x8xf32, #map0>
670+
return
671+
}
672+
673+
// -----
674+
675+
#map0 = affine_map<(d0, d1)[s0] -> (d0 * 16 + d1 + 136)>
676+
677+
func @subview_bad_offset_2(%arg0: memref<16x16xf32>) {
678+
%c0 = arith.constant 0 : index
679+
%c8 = arith.constant 8 : index
680+
// expected-error @+1 {{expected result type to be 'memref<8x8xf32, affine_map<(d0, d1)[s0] -> (d0 * 16 + s0 + d1)>>' or a rank-reduced version}}
681+
%s2 = memref.subview %arg0[%c8, 8][8, 8][1, 1] : memref<16x16xf32> to memref<8x8xf32, #map0>
682+
return
683+
}
684+
685+
// -----
686+
687+
#map0 = affine_map<(d0, d1)[s0] -> (d0 * 16 + d1 + s0 * 437)>
688+
689+
func @subview_bad_offset_3(%arg0: memref<16x16xf32>) {
690+
%c0 = arith.constant 0 : index
691+
%c8 = arith.constant 8 : index
692+
// expected-error @+1 {{expected result type to be 'memref<8x8xf32, affine_map<(d0, d1)[s0] -> (d0 * 16 + s0 + d1)>>' or a rank-reduced version}}
693+
%s2 = memref.subview %arg0[%c8, 8][8, 8][1, 1] : memref<16x16xf32> to memref<8x8xf32, #map0>
694+
return
695+
}
696+
697+
// -----
698+
663699
func @invalid_memref_cast(%arg0 : memref<12x4x16xf32, offset:0, strides:[64, 16, 1]>) {
664700
// expected-error@+1{{operand type 'memref<12x4x16xf32, affine_map<(d0, d1, d2) -> (d0 * 64 + d1 * 16 + d2)>>' and result type 'memref<12x4x16xf32, affine_map<(d0, d1, d2) -> (d0 * 128 + d1 * 32 + d2 * 2)>>' are cast incompatible}}
665701
%0 = memref.cast %arg0 : memref<12x4x16xf32, offset:0, strides:[64, 16, 1]> to memref<12x4x16xf32, offset:0, strides:[128, 32, 2]>

‎mlir/test/Dialect/MemRef/subview.mlir

+138
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
// RUN: mlir-opt %s | mlir-opt | FileCheck %s
2+
// RUN: mlir-opt %s --mlir-print-op-generic | mlir-opt | FileCheck %s
3+
4+
// CHECK-DAG: #[[$BASE_MAP0:map[0-9]+]] = affine_map<(d0, d1, d2) -> (d0 * 64 + d1 * 4 + d2)>
5+
// CHECK-DAG: #[[$BASE_MAP3:map[0-9]+]] = affine_map<(d0, d1, d2)[s0, s1, s2, s3] -> (d0 * s1 + s0 + d1 * s2 + d2 * s3)>
6+
7+
// CHECK-DAG: #[[$BASE_MAP1:map[0-9]+]] = affine_map<(d0)[s0] -> (d0 + s0)>
8+
// CHECK-DAG: #[[$SUBVIEW_MAP1:map[0-9]+]] = affine_map<(d0)[s0, s1] -> (d0 * s1 + s0)>
9+
10+
// CHECK-DAG: #[[$BASE_MAP2:map[0-9]+]] = affine_map<(d0, d1) -> (d0 * 22 + d1)>
11+
// CHECK-DAG: #[[$SUBVIEW_MAP2:map[0-9]+]] = affine_map<(d0, d1)[s0, s1, s2] -> (d0 * s1 + s0 + d1 * s2)>
12+
// CHECK-DAG: #[[$SUBVIEW_MAP3:map[0-9]+]] = affine_map<(d0, d1, d2) -> (d0 * 64 + d1 * 4 + d2 + 8)>
13+
// CHECK-DAG: #[[$SUBVIEW_MAP4:map[0-9]+]] = affine_map<(d0, d1)[s0, s1] -> (d0 * s1 + s0 + d1)>
14+
// CHECK-DAG: #[[$SUBVIEW_MAP5:map[0-9]+]] = affine_map<(d0, d1)[s0] -> (d0 * 8 + s0 + d1 * 2)>
15+
// CHECK-DAG: #[[$SUBVIEW_MAP6:map[0-9]+]] = affine_map<(d0, d1, d2, d3, d4) -> (d0 * 36 + d1 * 36 + d2 * 4 + d3 * 4 + d4)>
16+
// CHECK-DAG: #[[$SUBVIEW_MAP7:map[0-9]+]] = affine_map<(d0, d1, d2, d3, d4, d5)[s0, s1, s2, s3, s4, s5, s6] -> (d0 * s1 + s0 + d1 * s2 + d2 * s3 + d3 * s4 + d4 * s5 + d5 * s6)>
17+
// CHECK-DAG: #[[$SUBVIEW_MAP8:map[0-9]+]] = affine_map<(d0, d1, d2, d3)[s0, s1, s2, s3, s4] -> (d0 * s1 + s0 + d1 * s2 + d2 * s3 + d3 * s4)>
18+
// CHECK-DAG: #[[$SUBVIEW_MAP9:map[0-9]+]] = affine_map<(d0, d1) -> (d0 * 3 + d1 + 6)>
19+
// CHECK-DAG: #[[$SUBVIEW_MAP10:map[0-9]+]] = affine_map<(d0) -> (d0 + 3)>
20+
// CHECK-DAG: #[[$SUBVIEW_MAP11:map[0-9]+]] = affine_map<() -> (4)>
21+
// CHECK-DAG: #[[$SUBVIEW_MAP12:map[0-9]+]] = affine_map<()[s0] -> (s0)>
22+
23+
// CHECK-LABEL: func @memref_subview(%arg0
24+
func @memref_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
25+
%c0 = arith.constant 0 : index
26+
%c1 = arith.constant 1 : index
27+
28+
%0 = memref.alloc() : memref<8x16x4xf32, affine_map<(d0, d1, d2) -> (d0 * 64 + d1 * 4 + d2)>>
29+
// CHECK: subview %0[%c0, %c0, %c0] [%arg0, %arg1, %arg2] [%c1, %c1, %c1] :
30+
// CHECK-SAME: memref<8x16x4xf32, #[[$BASE_MAP0]]>
31+
// CHECK-SAME: to memref<?x?x?xf32, #[[$BASE_MAP3]]>
32+
%1 = memref.subview %0[%c0, %c0, %c0][%arg0, %arg1, %arg2][%c1, %c1, %c1]
33+
: memref<8x16x4xf32, offset:0, strides: [64, 4, 1]> to
34+
memref<?x?x?xf32, offset: ?, strides: [?, ?, ?]>
35+
36+
%2 = memref.alloc()[%arg2] : memref<64xf32, affine_map<(d0)[s0] -> (d0 + s0)>>
37+
// CHECK: memref.subview %2[%c1] [%arg0] [%c1] :
38+
// CHECK-SAME: memref<64xf32, #[[$BASE_MAP1]]>
39+
// CHECK-SAME: to memref<?xf32, #[[$SUBVIEW_MAP1]]>
40+
%3 = memref.subview %2[%c1][%arg0][%c1]
41+
: memref<64xf32, affine_map<(d0)[s0] -> (d0 + s0)>> to
42+
memref<?xf32, affine_map<(d0)[s0, s1] -> (d0 * s1 + s0)>>
43+
44+
%4 = memref.alloc() : memref<64x22xf32, affine_map<(d0, d1) -> (d0 * 22 + d1)>>
45+
// CHECK: memref.subview %4[%c0, %c1] [%arg0, %arg1] [%c1, %c0] :
46+
// CHECK-SAME: memref<64x22xf32, #[[$BASE_MAP2]]>
47+
// CHECK-SAME: to memref<?x?xf32, #[[$SUBVIEW_MAP2]]>
48+
%5 = memref.subview %4[%c0, %c1][%arg0, %arg1][%c1, %c0]
49+
: memref<64x22xf32, offset:0, strides: [22, 1]> to
50+
memref<?x?xf32, offset:?, strides: [?, ?]>
51+
52+
// CHECK: memref.subview %0[0, 2, 0] [4, 4, 4] [1, 1, 1] :
53+
// CHECK-SAME: memref<8x16x4xf32, #[[$BASE_MAP0]]>
54+
// CHECK-SAME: to memref<4x4x4xf32, #[[$SUBVIEW_MAP3]]>
55+
%6 = memref.subview %0[0, 2, 0][4, 4, 4][1, 1, 1]
56+
: memref<8x16x4xf32, offset:0, strides: [64, 4, 1]> to
57+
memref<4x4x4xf32, offset:8, strides: [64, 4, 1]>
58+
59+
%7 = memref.alloc(%arg1, %arg2) : memref<?x?xf32>
60+
// CHECK: memref.subview {{%.*}}[0, 0] [4, 4] [1, 1] :
61+
// CHECK-SAME: memref<?x?xf32>
62+
// CHECK-SAME: to memref<4x4xf32, #[[$SUBVIEW_MAP4]]>
63+
%8 = memref.subview %7[0, 0][4, 4][1, 1]
64+
: memref<?x?xf32> to memref<4x4xf32, offset: ?, strides:[?, 1]>
65+
66+
%9 = memref.alloc() : memref<16x4xf32>
67+
// CHECK: memref.subview {{%.*}}[{{%.*}}, {{%.*}}] [4, 4] [{{%.*}}, {{%.*}}] :
68+
// CHECK-SAME: memref<16x4xf32>
69+
// CHECK-SAME: to memref<4x4xf32, #[[$SUBVIEW_MAP2]]
70+
%10 = memref.subview %9[%arg1, %arg1][4, 4][%arg2, %arg2]
71+
: memref<16x4xf32> to memref<4x4xf32, offset: ?, strides:[?, ?]>
72+
73+
// CHECK: memref.subview {{%.*}}[{{%.*}}, {{%.*}}] [4, 4] [2, 2] :
74+
// CHECK-SAME: memref<16x4xf32>
75+
// CHECK-SAME: to memref<4x4xf32, #[[$SUBVIEW_MAP5]]
76+
%11 = memref.subview %9[%arg1, %arg2][4, 4][2, 2]
77+
: memref<16x4xf32> to memref<4x4xf32, offset: ?, strides:[8, 2]>
78+
79+
%12 = memref.alloc() : memref<1x9x1x4x1xf32, affine_map<(d0, d1, d2, d3, d4) -> (36 * d0 + 36 * d1 + 4 * d2 + 4 * d3 + d4)>>
80+
// CHECK: memref.subview %12[%arg1, %arg1, %arg1, %arg1, %arg1]
81+
// CHECK-SAME: [1, 9, 1, 4, 1] [%arg2, %arg2, %arg2, %arg2, %arg2] :
82+
// CHECK-SAME: memref<1x9x1x4x1xf32, #[[$SUBVIEW_MAP6]]> to memref<9x4xf32, #[[$SUBVIEW_MAP2]]>
83+
%13 = memref.subview %12[%arg1, %arg1, %arg1, %arg1, %arg1][1, 9, 1, 4, 1][%arg2, %arg2, %arg2, %arg2, %arg2] : memref<1x9x1x4x1xf32, offset: 0, strides: [36, 36, 4, 4, 1]> to memref<9x4xf32, offset: ?, strides: [?, ?]>
84+
// CHECK: memref.subview %12[%arg1, %arg1, %arg1, %arg1, %arg1]
85+
// CHECK-SAME: [1, 9, 1, 4, 1] [%arg2, %arg2, %arg2, %arg2, %arg2] :
86+
// CHECK-SAME: memref<1x9x1x4x1xf32, #[[$SUBVIEW_MAP6]]> to memref<1x9x4xf32, #[[$BASE_MAP3]]>
87+
%14 = memref.subview %12[%arg1, %arg1, %arg1, %arg1, %arg1][1, 9, 1, 4, 1][%arg2, %arg2, %arg2, %arg2, %arg2] : memref<1x9x1x4x1xf32, offset: 0, strides: [36, 36, 4, 4, 1]> to memref<1x9x4xf32, offset: ?, strides: [?, ?, ?]>
88+
89+
%15 = memref.alloc(%arg1, %arg2)[%c0, %c1, %arg1, %arg0, %arg0, %arg2, %arg2] : memref<1x?x5x1x?x1xf32, affine_map<(d0, d1, d2, d3, d4, d5)[s0, s1, s2, s3, s4, s5, s6] -> (s0 + s1 * d0 + s2 * d1 + s3 * d2 + s4 * d3 + s5 * d4 + s6 * d5)>>
90+
// CHECK: memref.subview %15[0, 0, 0, 0, 0, 0] [1, %arg1, 5, 1, %arg2, 1] [1, 1, 1, 1, 1, 1] :
91+
// CHECK-SAME: memref<1x?x5x1x?x1xf32, #[[$SUBVIEW_MAP7]]> to memref<?x5x?xf32, #[[$BASE_MAP3]]>
92+
%16 = memref.subview %15[0, 0, 0, 0, 0, 0][1, %arg1, 5, 1, %arg2, 1][1, 1, 1, 1, 1, 1] : memref<1x?x5x1x?x1xf32, offset: ?, strides: [?, ?, ?, ?, ?, ?]> to memref<?x5x?xf32, offset: ?, strides: [?, ?, ?]>
93+
// CHECK: memref.subview %15[%arg1, %arg1, %arg1, %arg1, %arg1, %arg1] [1, %arg1, 5, 1, %arg2, 1] [1, 1, 1, 1, 1, 1] :
94+
// CHECK-SAME: memref<1x?x5x1x?x1xf32, #[[$SUBVIEW_MAP7]]> to memref<?x5x?x1xf32, #[[$SUBVIEW_MAP8]]>
95+
%17 = memref.subview %15[%arg1, %arg1, %arg1, %arg1, %arg1, %arg1][1, %arg1, 5, 1, %arg2, 1][1, 1, 1, 1, 1, 1] : memref<1x?x5x1x?x1xf32, offset: ?, strides: [?, ?, ?, ?, ?, ?]> to memref<?x5x?x1xf32, offset: ?, strides: [?, ?, ?, ?]>
96+
97+
%18 = memref.alloc() : memref<1x8xf32>
98+
// CHECK: memref.subview %18[0, 0] [1, 8] [1, 1] : memref<1x8xf32> to memref<8xf32>
99+
%19 = memref.subview %18[0, 0][1, 8][1, 1] : memref<1x8xf32> to memref<8xf32>
100+
101+
%20 = memref.alloc() : memref<8x16x4xf32>
102+
// CHECK: memref.subview %20[0, 0, 0] [1, 16, 4] [1, 1, 1] : memref<8x16x4xf32> to memref<16x4xf32>
103+
%21 = memref.subview %20[0, 0, 0][1, 16, 4][1, 1, 1] : memref<8x16x4xf32> to memref<16x4xf32>
104+
105+
%22 = memref.subview %20[3, 4, 2][1, 6, 3][1, 1, 1] : memref<8x16x4xf32> to memref<6x3xf32, offset: 210, strides: [4, 1]>
106+
107+
%23 = memref.alloc() : memref<f32>
108+
%78 = memref.subview %23[] [] [] : memref<f32> to memref<f32>
109+
110+
/// Subview with only leading operands.
111+
%24 = memref.alloc() : memref<5x3xf32>
112+
// CHECK: memref.subview %{{.*}}[2] [3] [1] : memref<5x3xf32> to memref<3x3xf32, #[[$SUBVIEW_MAP9]]>
113+
%25 = memref.subview %24[2][3][1]: memref<5x3xf32> to memref<3x3xf32, offset: 6, strides: [3, 1]>
114+
115+
/// Rank-reducing subview with only leading operands.
116+
// CHECK: memref.subview %{{.*}}[1] [1] [1] : memref<5x3xf32> to memref<3xf32, #[[$SUBVIEW_MAP10]]>
117+
%26 = memref.subview %24[1][1][1]: memref<5x3xf32> to memref<3xf32, offset: 3, strides: [1]>
118+
119+
// Corner-case of 0-D rank-reducing subview with an offset.
120+
// CHECK: memref.subview %{{.*}}[1, 1] [1, 1] [1, 1] : memref<5x3xf32> to memref<f32, #[[$SUBVIEW_MAP11]]>
121+
%27 = memref.subview %24[1, 1] [1, 1] [1, 1] : memref<5x3xf32> to memref<f32, affine_map<() -> (4)>>
122+
123+
// CHECK: memref.subview %{{.*}}[%{{.*}}, 1] [1, 1] [1, 1] : memref<5x3xf32> to memref<f32, #[[$SUBVIEW_MAP12]]>
124+
%28 = memref.subview %24[%arg0, 1] [1, 1] [1, 1] : memref<5x3xf32> to memref<f32, affine_map<()[s0] -> (s0)>>
125+
126+
// CHECK: memref.subview %{{.*}}[0, %{{.*}}] [%{{.*}}, 1] [1, 1] : memref<?x?xf32> to memref<?xf32, #[[$SUBVIEW_MAP1]]>
127+
%a30 = memref.alloc(%arg0, %arg0) : memref<?x?xf32>
128+
%30 = memref.subview %a30[0, %arg1][%arg2, 1][1, 1] : memref<?x?xf32> to memref<?xf32, affine_map<(d0)[s0, s1] -> (d0 * s1 + s0)>>
129+
130+
%c8 = arith.constant 8 : index
131+
%a40 = memref.alloc() : memref<16x16xf32>
132+
// CHECK: memref.subview
133+
%40 = memref.subview %a40[%c8, 8][8, 8][1, 1] :
134+
memref<16x16xf32> to memref<8x8xf32, affine_map<(d0, d1)[s0] -> (d0 * 16 + d1 + s0)>>
135+
136+
return
137+
}
138+

0 commit comments

Comments
 (0)