Skip to content

Commit eddf322

Browse files
committed
[mlir][vector] Fix emulation of "narrow" type vector.store
Below are two examples of "narrow" `vector.stores`. The first example does not require partial stores and hence no RMW stores. This is currently emulated correctly. ``` func.func @example_1(%arg0: vector<4xi2>) { %0 = memref.alloc() : memref<13xi2> %c4 = arith.constant 4 : index vector.store %arg0, %0[%c4] : memref<13xi2>, vector<4xi2> return } ``` The second example below does require a partial store (due to the offset) and hence a RMW store. ``` func.func @example_2(%arg0: vector<4xi2>) { %0 = memref.alloc() : memref<13xi2> %c3 = arith.constant 3 : index vector.store %arg0, %0[%c3] : memref<13xi2>, vector<4xi2> return } ``` This is currently incorrectly emulated as a single "full" store (note that the offset is incorrect): ``` func.func @example_2(%arg0: vector<4xi2>) { %alloc = memref.alloc() : memref<4xi8> %0 = vector.bitcast %arg0 : vector<4xi2> to vector<1xi8> %c0 = arith.constant 0 : index vector.store %0, %alloc[%c0] : memref<4xi8>, vector<1xi8> return } ``` This PR fixes this issue. Additional comments are added to clarify the current logic.
1 parent 0106e5a commit eddf322

File tree

2 files changed

+103
-11
lines changed

2 files changed

+103
-11
lines changed

mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -591,12 +591,12 @@ struct ConvertVectorStore final : OpConversionPattern<vector::StoreOp> {
591591
// vector<4xi8>
592592

593593
auto origElements = valueToStore.getType().getNumElements();
594-
// Note, per-element-alignment was already verified above.
595-
bool isDivisibleInSize = origElements % emulatedPerContainerElem == 0;
596594

597595
auto stridedMetadata =
598596
rewriter.create<memref::ExtractStridedMetadataOp>(loc, op.getBase());
599597

598+
// FIXME: ATM, we do not test cases where offsets, sizes, or strides are
599+
// non-zero. As such, this is not needed.
600600
OpFoldResult linearizedIndices;
601601
memref::LinearizedMemRefInfo linearizedInfo;
602602
std::tie(linearizedInfo, linearizedIndices) =
@@ -608,8 +608,7 @@ struct ConvertVectorStore final : OpConversionPattern<vector::StoreOp> {
608608
getAsOpFoldResult(adaptor.getIndices()));
609609

610610
std::optional<int64_t> foldedNumFrontPadElems =
611-
isDivisibleInSize ? 0
612-
: getConstantIntValue(linearizedInfo.intraDataOffset);
611+
getConstantIntValue(linearizedInfo.intraDataOffset);
613612

614613
if (!foldedNumFrontPadElems) {
615614
return rewriter.notifyMatchFailure(
@@ -619,15 +618,39 @@ struct ConvertVectorStore final : OpConversionPattern<vector::StoreOp> {
619618

620619
auto memrefBase = cast<MemRefValue>(adaptor.getBase());
621620

622-
// Conditions when atomic RMWs are not needed:
621+
// RMWs are not needed when:
622+
// * no _partial_ stores are required.
623+
// A partial store is defined as a store in which only a part of the
624+
// container element is overwritten, e.g.
625+
//
626+
// Dest before (8 bits)
627+
// +----------+
628+
// | 11000000 |
629+
// +----------+
630+
//
631+
// Dest after storing 0xF at offset 4 (in bits)
632+
// +----------+
633+
// | 11001111 |
634+
// +----------+
635+
//
636+
// At a higher level, this translats to:
623637
// 1. The source vector size (in bits) is a multiple of byte size.
624-
// 2. The address of the store is aligned to the emulated width boundary.
638+
// 2. The address of the store is aligned to the container type width
639+
// boundary.
640+
//
641+
// EXAMPLE 1:
642+
// Requires partial store:
643+
// vector.store %arg0, %0[%c3] : memref<13xi2>, vector<4xi2>
625644
//
626-
// For example, to store a vector<4xi2> to <13xi2> at offset 4, does not
627-
// need unaligned emulation because the store address is aligned and the
628-
// source is a whole byte.
629-
bool emulationRequiresPartialStores =
630-
!isDivisibleInSize || *foldedNumFrontPadElems != 0;
645+
// EXAMPLE 2:
646+
// Does not require a partial store:
647+
// vector.store %arg0, %0[%c4] : memref<13xi2>, vector<4xi2>
648+
//
649+
// TODO: Take linearizedInfo.linearizedOffset into account. This is
650+
// currently not needed/used/exercised as all our tests set offset to 0.
651+
bool emulationRequiresPartialStores = *foldedNumFrontPadElems != 0;
652+
emulationRequiresPartialStores = true;
653+
631654
if (!emulationRequiresPartialStores) {
632655
// Basic case: storing full bytes.
633656
auto numElements = origElements / emulatedPerContainerElem;

mlir/test/Dialect/Vector/vector-emulate-narrow-type-unaligned.mlir

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,75 @@ func.func @vector_maskedload_i2_constant_mask_unaligned(%passthru: vector<5xi2>)
361361
/// vector.store
362362
///----------------------------------------------------------------------------------------
363363

364+
// -----
365+
366+
// Most basic example to demonstrate where partial stores are not needed.
367+
368+
func.func @vector_store_i2_const_index_no_partial_store(%arg0: vector<4xi2>) {
369+
%0 = memref.alloc() : memref<13xi2>
370+
%c4 = arith.constant 4 : index
371+
vector.store %arg0, %0[%c4] : memref<13xi2>, vector<4xi2>
372+
return
373+
}
374+
// CHECK-LABEL: func.func @vector_store_i2_const_index_no_partial_store(
375+
// CHECK-SAME: %[[ARG_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: vector<4xi2>) {
376+
// CHECK-NOT: memref.generic_atomic_rmw
377+
// CHECK: %[[ALLOC:.*]] = memref.alloc() : memref<4xi8>
378+
// CHECK: %[[C1:.*]] = arith.constant 1 : index
379+
// CHECK: %[[IDX:.*]] = arith.addi %[[C1]], %[[C1]] : index
380+
// CHECK: %[[UPCAST:.*]] = vector.bitcast %[[ARG_0]] : vector<4xi2> to vector<1xi8>
381+
// CHECK: vector.store %[[UPCAST]], %[[ALLOC]]{{\[}}%[[IDX]]] : memref<4xi8>, vector<1xi8>
382+
383+
// -----
384+
385+
// Small modification of the example above to demonstrate where partial stores
386+
// are needed.
387+
388+
func.func @vector_store_i2_const_index_two_partial_stores(%arg0: vector<4xi2>) {
389+
%0 = memref.alloc() : memref<13xi2>
390+
%c3 = arith.constant 3 : index
391+
vector.store %arg0, %0[%c3] : memref<13xi2>, vector<4xi2>
392+
return
393+
}
394+
395+
// CHECK-LABEL: func.func @vector_store_i2_const_index_two_partial_stores(
396+
// CHECK-SAME: %[[ARG_0:[0-9]+|[a-zA-Z$._-][a-zA-Z0-9$._-]*]]: vector<4xi2>) {
397+
// CHECK: %[[VAL_1:.*]] = memref.alloc() : memref<4xi8>
398+
399+
// First atomic RMW:
400+
// CHECK: %[[IDX_1:.*]] = arith.constant 0 : index
401+
// CHECK: %[[MASK_1:.*]] = arith.constant dense<[false, false, false, true]> : vector<4xi1>
402+
// CHECK: %[[INIT:.*]] = arith.constant dense<0> : vector<4xi2>
403+
// CHECK: %[[SLICE_1:.*]] = vector.extract_strided_slice %[[ARG_0]] {offsets = [0], sizes = [1], strides = [1]} : vector<4xi2> to vector<1xi2>
404+
// CHECK: %[[V1:.*]] = vector.insert_strided_slice %[[SLICE_1]], %[[INIT]] {offsets = [3], strides = [1]} : vector<1xi2> into vector<4xi2>
405+
// CHECK: memref.generic_atomic_rmw %[[VAL_1]]{{\[}}%[[IDX_1]]] : memref<4xi8> {
406+
// CHECK: ^bb0(%[[VAL_8:.*]]: i8):
407+
// CHECK: %[[VAL_9:.*]] = vector.from_elements %[[VAL_8]] : vector<1xi8>
408+
// CHECK: %[[DOWNCAST_1:.*]] = vector.bitcast %[[VAL_9]] : vector<1xi8> to vector<4xi2>
409+
// CHECK: %[[SELECT_1:.*]] = arith.select %[[MASK_1]], %[[V1]], %[[DOWNCAST_1]] : vector<4xi1>, vector<4xi2>
410+
// CHECK: %[[UPCAST_1:.*]] = vector.bitcast %[[SELECT_1]] : vector<4xi2> to vector<1xi8>
411+
// CHECK: %[[RES_1:.*]] = vector.extract %[[UPCAST_1]][0] : i8 from vector<1xi8>
412+
// CHECK: memref.atomic_yield %[[RES_1]] : i8
413+
// CHECK: }
414+
415+
// Second atomic RMW:
416+
// CHECK: %[[VAL_14:.*]] = arith.constant 1 : index
417+
// CHECK: %[[IDX_2:.*]] = arith.addi %[[IDX_1]], %[[VAL_14]] : index
418+
// CHECK: %[[VAL_16:.*]] = vector.extract_strided_slice %[[ARG_0]] {offsets = [1], sizes = [3], strides = [1]} : vector<4xi2> to vector<3xi2>
419+
// CHECK: %[[V2:.*]] = vector.insert_strided_slice %[[VAL_16]], %[[INIT]] {offsets = [0], strides = [1]} : vector<3xi2> into vector<4xi2>
420+
// CHECK: %[[MASK_2:.*]] = arith.constant dense<[true, true, true, false]> : vector<4xi1>
421+
// CHECK: memref.generic_atomic_rmw %[[VAL_1]]{{\[}}%[[IDX_2]]] : memref<4xi8> {
422+
// CHECK: ^bb0(%[[VAL_20:.*]]: i8):
423+
// CHECK: %[[VAL_21:.*]] = vector.from_elements %[[VAL_20]] : vector<1xi8>
424+
// CHECK: %[[DONWCAST_2:.*]] = vector.bitcast %[[VAL_21]] : vector<1xi8> to vector<4xi2>
425+
// CHECK: %[[SELECT_2:.*]] = arith.select %[[MASK_2]], %[[V2]], %[[DONWCAST_2]] : vector<4xi1>, vector<4xi2>
426+
// CHECK: %[[UPCAST_2:.*]] = vector.bitcast %[[SELECT_2]] : vector<4xi2> to vector<1xi8>
427+
// CHECK: %[[RES_2:.*]] = vector.extract %[[UPCAST_2]][0] : i8 from vector<1xi8>
428+
// CHECK: memref.atomic_yield %[[RES_2]] : i8
429+
// CHECK: }
430+
431+
// -----
432+
364433
func.func @vector_store_i2_const_index_two_partial_stores(%arg0: vector<3xi2>) {
365434
%src = memref.alloc() : memref<3x3xi2>
366435
%c0 = arith.constant 0 : index

0 commit comments

Comments
 (0)