[mlir][Vector] Remove 0-d corner case condition. #112937

harrisonGPU · 2024-10-18T17:12:47Z

Remove 0-d corner case condition.

Closes #112913

…reLowering.

llvmbot · 2024-11-05T12:14:58Z

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-vector

Author: Harrison Hao (harrisonGPU)

Changes

Support 0-d vectors natively in VectorStoreToMemrefStoreLowering.

Closes #112913

Full diff: https://github.com/llvm/llvm-project/pull/112937.diff

4 Files Affected:

(modified) mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp (+3-9)
(modified) mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir (+2-3)
(modified) mlir/test/Dialect/SPIRV/IR/availability.mlir (+18-18)
(modified) mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir (+3-5)

diff --git a/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp b/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp
index f9428a4ce28640..1cb3baaef82baf 100644
--- a/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp
@@ -530,15 +530,9 @@ struct VectorStoreToMemrefStoreLowering
       return rewriter.notifyMatchFailure(storeOp, "not single element vector");
 
     Value extracted;
-    if (vecType.getRank() == 0) {
-      // TODO: Unifiy once ExtractOp supports 0-d vectors.
-      extracted = rewriter.create<vector::ExtractElementOp>(
-          storeOp.getLoc(), storeOp.getValueToStore());
-    } else {
-      SmallVector<int64_t> indices(vecType.getRank(), 0);
-      extracted = rewriter.create<vector::ExtractOp>(
-          storeOp.getLoc(), storeOp.getValueToStore(), indices);
-    }
+    SmallVector<int64_t> indices(vecType.getRank(), 0);
+    extracted = rewriter.create<vector::ExtractOp>(
+        storeOp.getLoc(), storeOp.getValueToStore(), indices);
 
     rewriter.replaceOpWithNewOp<memref::StoreOp>(
         storeOp, extracted, storeOp.getBase(), storeOp.getIndices());
diff --git a/mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir b/mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir
index c1de24fd0403ce..abbdbe02ce6c1e 100644
--- a/mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir
+++ b/mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir
@@ -2971,9 +2971,8 @@ func.func @vector_store_op_0d(%memref : memref<200x100xf32>, %i : index, %j : in
 // CHECK-LABEL: func @vector_store_op_0d
 // CHECK: %[[val:.*]] = arith.constant dense<1.100000e+01> : vector<f32>
 // CHECK: %[[cast:.*]] = builtin.unrealized_conversion_cast %[[val]] : vector<f32> to vector<1xf32>
-// CHECK: %[[c0:.*]] = llvm.mlir.constant(0 : index) : i64
-// CHECK: %[[extracted:.*]] = llvm.extractelement %[[cast]][%[[c0]] : i64] : vector<1xf32>
-// CHECK: memref.store %[[extracted]], %{{.*}}[%{{.*}}, %{{.*}}]
+// CHECK: %[[cast2:.*]] = builtin.unrealized_conversion_cast %[[cast]] : vector<1xf32> to f32
+// CHECK: memref.store %[[cast2]], %{{.*}}[%{{.*}}, %{{.*}}]
 
 // -----
 
diff --git a/mlir/test/Dialect/SPIRV/IR/availability.mlir b/mlir/test/Dialect/SPIRV/IR/availability.mlir
index c583a48eba2704..ceebeeffcf2677 100644
--- a/mlir/test/Dialect/SPIRV/IR/availability.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/availability.mlir
@@ -58,7 +58,7 @@ func.func @module_physical_storage_buffer64_vulkan() {
 func.func @sdot_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.SDot %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -68,7 +68,7 @@ func.func @sdot_scalar_i32_i32(%a: i32) -> i32 {
 func.func @sdot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.SDot %a, %a: vector<4xi8> -> i64
   return %r: i64
@@ -78,7 +78,7 @@ func.func @sdot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
 func.func @sdot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.SDot %a, %a: vector<4xi16> -> i64
   return %r: i64
@@ -88,7 +88,7 @@ func.func @sdot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
 func.func @sudot_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.SUDot %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -98,7 +98,7 @@ func.func @sudot_scalar_i32_i32(%a: i32) -> i32 {
 func.func @sudot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.SUDot %a, %a: vector<4xi8> -> i64
   return %r: i64
@@ -108,7 +108,7 @@ func.func @sudot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
 func.func @sudot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.SUDot %a, %a: vector<4xi16> -> i64
   return %r: i64
@@ -118,7 +118,7 @@ func.func @sudot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
 func.func @udot_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.UDot %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -128,7 +128,7 @@ func.func @udot_scalar_i32_i32(%a: i32) -> i32 {
 func.func @udot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.UDot %a, %a: vector<4xi8> -> i64
   return %r: i64
@@ -138,7 +138,7 @@ func.func @udot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
 func.func @udot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.UDot %a, %a: vector<4xi16> -> i64
   return %r: i64
@@ -148,7 +148,7 @@ func.func @udot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
 func.func @sdot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.SDotAccSat %a, %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -158,7 +158,7 @@ func.func @sdot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
 func.func @sdot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.SDotAccSat %a, %a, %acc: vector<4xi8> -> i64
   return %r: i64
@@ -168,7 +168,7 @@ func.func @sdot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
 func.func @sdot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.SDotAccSat %a, %a, %acc: vector<4xi16> -> i64
   return %r: i64
@@ -178,7 +178,7 @@ func.func @sdot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
 func.func @sudot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.SUDotAccSat %a, %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -188,7 +188,7 @@ func.func @sudot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
 func.func @sudot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.SUDotAccSat %a, %a, %acc: vector<4xi8> -> i64
   return %r: i64
@@ -198,7 +198,7 @@ func.func @sudot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
 func.func @sudot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.SUDotAccSat %a, %a, %acc: vector<4xi16> -> i64
   return %r: i64
@@ -208,7 +208,7 @@ func.func @sudot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
 func.func @udot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.UDotAccSat %a, %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -218,7 +218,7 @@ func.func @udot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
 func.func @udot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.UDotAccSat %a, %a, %acc: vector<4xi8> -> i64
   return %r: i64
@@ -228,7 +228,7 @@ func.func @udot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
 func.func @udot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.UDotAccSat %a, %a, %acc: vector<4xi16> -> i64
   return %r: i64
diff --git a/mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir b/mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir
index f90111b4c88618..f75f8f8489efc1 100644
--- a/mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir
+++ b/mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir
@@ -7,15 +7,13 @@ func.func @vector_transfer_ops_0d_memref(%mem: memref<f32>, %vec: vector<1x1x1xf
     %f0 = arith.constant 0.0 : f32
 
 //  CHECK-NEXT:   %[[S:.*]] = memref.load %[[MEM]][] : memref<f32>
-//  CHECK-NEXT:   %[[V:.*]] = vector.broadcast %[[S]] : f32 to vector<f32>
     %0 = vector.transfer_read %mem[], %f0 : memref<f32>, vector<f32>
 
-//  CHECK-NEXT:   %[[SS:.*]] = vector.extractelement %[[V]][] : vector<f32>
-//  CHECK-NEXT:   memref.store %[[SS]], %[[MEM]][] : memref<f32>
+//  CHECK-NEXT:   memref.store %[[S]], %[[MEM]][] : memref<f32>
     vector.transfer_write %0, %mem[] : vector<f32>, memref<f32>
 
-//  CHECK-NEXT:   %[[VV:.*]] = vector.extract %arg1[0, 0, 0] : f32 from vector<1x1x1xf32>
-//  CHECK-NEXT:   memref.store %[[VV]], %[[MEM]][] : memref<f32>
+//  CHECK-NEXT:   %[[V:.*]] = vector.extract %arg1[0, 0, 0] : f32 from vector<1x1x1xf32>
+//  CHECK-NEXT:   memref.store %[[V]], %[[MEM]][] : memref<f32>
     vector.store %vec, %mem[] : memref<f32>, vector<1x1x1xf32>
 
     return

llvmbot · 2024-11-05T12:14:59Z

@llvm/pr-subscribers-mlir-spirv

Author: Harrison Hao (harrisonGPU)

Changes

Support 0-d vectors natively in VectorStoreToMemrefStoreLowering.

Closes #112913

Full diff: https://github.com/llvm/llvm-project/pull/112937.diff

4 Files Affected:

(modified) mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp (+3-9)
(modified) mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir (+2-3)
(modified) mlir/test/Dialect/SPIRV/IR/availability.mlir (+18-18)
(modified) mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir (+3-5)

diff --git a/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp b/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp
index f9428a4ce28640..1cb3baaef82baf 100644
--- a/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/LowerVectorTransfer.cpp
@@ -530,15 +530,9 @@ struct VectorStoreToMemrefStoreLowering
       return rewriter.notifyMatchFailure(storeOp, "not single element vector");
 
     Value extracted;
-    if (vecType.getRank() == 0) {
-      // TODO: Unifiy once ExtractOp supports 0-d vectors.
-      extracted = rewriter.create<vector::ExtractElementOp>(
-          storeOp.getLoc(), storeOp.getValueToStore());
-    } else {
-      SmallVector<int64_t> indices(vecType.getRank(), 0);
-      extracted = rewriter.create<vector::ExtractOp>(
-          storeOp.getLoc(), storeOp.getValueToStore(), indices);
-    }
+    SmallVector<int64_t> indices(vecType.getRank(), 0);
+    extracted = rewriter.create<vector::ExtractOp>(
+        storeOp.getLoc(), storeOp.getValueToStore(), indices);
 
     rewriter.replaceOpWithNewOp<memref::StoreOp>(
         storeOp, extracted, storeOp.getBase(), storeOp.getIndices());
diff --git a/mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir b/mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir
index c1de24fd0403ce..abbdbe02ce6c1e 100644
--- a/mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir
+++ b/mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir
@@ -2971,9 +2971,8 @@ func.func @vector_store_op_0d(%memref : memref<200x100xf32>, %i : index, %j : in
 // CHECK-LABEL: func @vector_store_op_0d
 // CHECK: %[[val:.*]] = arith.constant dense<1.100000e+01> : vector<f32>
 // CHECK: %[[cast:.*]] = builtin.unrealized_conversion_cast %[[val]] : vector<f32> to vector<1xf32>
-// CHECK: %[[c0:.*]] = llvm.mlir.constant(0 : index) : i64
-// CHECK: %[[extracted:.*]] = llvm.extractelement %[[cast]][%[[c0]] : i64] : vector<1xf32>
-// CHECK: memref.store %[[extracted]], %{{.*}}[%{{.*}}, %{{.*}}]
+// CHECK: %[[cast2:.*]] = builtin.unrealized_conversion_cast %[[cast]] : vector<1xf32> to f32
+// CHECK: memref.store %[[cast2]], %{{.*}}[%{{.*}}, %{{.*}}]
 
 // -----
 
diff --git a/mlir/test/Dialect/SPIRV/IR/availability.mlir b/mlir/test/Dialect/SPIRV/IR/availability.mlir
index c583a48eba2704..ceebeeffcf2677 100644
--- a/mlir/test/Dialect/SPIRV/IR/availability.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/availability.mlir
@@ -58,7 +58,7 @@ func.func @module_physical_storage_buffer64_vulkan() {
 func.func @sdot_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.SDot %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -68,7 +68,7 @@ func.func @sdot_scalar_i32_i32(%a: i32) -> i32 {
 func.func @sdot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.SDot %a, %a: vector<4xi8> -> i64
   return %r: i64
@@ -78,7 +78,7 @@ func.func @sdot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
 func.func @sdot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.SDot %a, %a: vector<4xi16> -> i64
   return %r: i64
@@ -88,7 +88,7 @@ func.func @sdot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
 func.func @sudot_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.SUDot %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -98,7 +98,7 @@ func.func @sudot_scalar_i32_i32(%a: i32) -> i32 {
 func.func @sudot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.SUDot %a, %a: vector<4xi8> -> i64
   return %r: i64
@@ -108,7 +108,7 @@ func.func @sudot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
 func.func @sudot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.SUDot %a, %a: vector<4xi16> -> i64
   return %r: i64
@@ -118,7 +118,7 @@ func.func @sudot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
 func.func @udot_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.UDot %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -128,7 +128,7 @@ func.func @udot_scalar_i32_i32(%a: i32) -> i32 {
 func.func @udot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.UDot %a, %a: vector<4xi8> -> i64
   return %r: i64
@@ -138,7 +138,7 @@ func.func @udot_vector_4xi8_i64(%a: vector<4xi8>) -> i64 {
 func.func @udot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.UDot %a, %a: vector<4xi16> -> i64
   return %r: i64
@@ -148,7 +148,7 @@ func.func @udot_vector_4xi16_i64(%a: vector<4xi16>) -> i64 {
 func.func @sdot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.SDotAccSat %a, %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -158,7 +158,7 @@ func.func @sdot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
 func.func @sdot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.SDotAccSat %a, %a, %acc: vector<4xi8> -> i64
   return %r: i64
@@ -168,7 +168,7 @@ func.func @sdot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
 func.func @sdot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.SDotAccSat %a, %a, %acc: vector<4xi16> -> i64
   return %r: i64
@@ -178,7 +178,7 @@ func.func @sdot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
 func.func @sudot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.SUDotAccSat %a, %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -188,7 +188,7 @@ func.func @sudot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
 func.func @sudot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.SUDotAccSat %a, %a, %acc: vector<4xi8> -> i64
   return %r: i64
@@ -198,7 +198,7 @@ func.func @sudot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
 func.func @sudot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.SUDotAccSat %a, %a, %acc: vector<4xi16> -> i64
   return %r: i64
@@ -208,7 +208,7 @@ func.func @sudot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
 func.func @udot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8BitPacked] ]
   %r = spirv.UDotAccSat %a, %a, %a, <PackedVectorFormat4x8Bit>: i32 -> i32
   return %r: i32
@@ -218,7 +218,7 @@ func.func @udot_acc_sat_scalar_i32_i32(%a: i32) -> i32 {
 func.func @udot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInput4x8Bit] ]
   %r = spirv.UDotAccSat %a, %a, %acc: vector<4xi8> -> i64
   return %r: i64
@@ -228,7 +228,7 @@ func.func @udot_acc_sat_vector_4xi8_i64(%a: vector<4xi8>, %acc: i64) -> i64 {
 func.func @udot_acc_sat_vector_4xi16_i64(%a: vector<4xi16>, %acc: i64) -> i64 {
   // CHECK: min version: v1.0
   // CHECK: max version: v1.6
-  // CHECK: extensions: [ [SPV_KHR_integer_dot_product] ]
+  // CHECK: extensions: [ [SPV_KHR_16bit_storage] ]
   // CHECK: capabilities: [ [DotProduct] [DotProductInputAll] ]
   %r = spirv.UDotAccSat %a, %a, %acc: vector<4xi16> -> i64
   return %r: i64
diff --git a/mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir b/mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir
index f90111b4c88618..f75f8f8489efc1 100644
--- a/mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir
+++ b/mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir
@@ -7,15 +7,13 @@ func.func @vector_transfer_ops_0d_memref(%mem: memref<f32>, %vec: vector<1x1x1xf
     %f0 = arith.constant 0.0 : f32
 
 //  CHECK-NEXT:   %[[S:.*]] = memref.load %[[MEM]][] : memref<f32>
-//  CHECK-NEXT:   %[[V:.*]] = vector.broadcast %[[S]] : f32 to vector<f32>
     %0 = vector.transfer_read %mem[], %f0 : memref<f32>, vector<f32>
 
-//  CHECK-NEXT:   %[[SS:.*]] = vector.extractelement %[[V]][] : vector<f32>
-//  CHECK-NEXT:   memref.store %[[SS]], %[[MEM]][] : memref<f32>
+//  CHECK-NEXT:   memref.store %[[S]], %[[MEM]][] : memref<f32>
     vector.transfer_write %0, %mem[] : vector<f32>, memref<f32>
 
-//  CHECK-NEXT:   %[[VV:.*]] = vector.extract %arg1[0, 0, 0] : f32 from vector<1x1x1xf32>
-//  CHECK-NEXT:   memref.store %[[VV]], %[[MEM]][] : memref<f32>
+//  CHECK-NEXT:   %[[V:.*]] = vector.extract %arg1[0, 0, 0] : f32 from vector<1x1x1xf32>
+//  CHECK-NEXT:   memref.store %[[V]], %[[MEM]][] : memref<f32>
     vector.store %vec, %mem[] : memref<f32>, vector<1x1x1xf32>
 
     return

banach-space

LG, just a couple of comments. Thanks!

banach-space · 2024-11-06T13:34:49Z

mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir

    %f0 = arith.constant 0.0 : f32

 //  CHECK-NEXT:   %[[S:.*]] = memref.load %[[MEM]][] : memref<f32>
-//  CHECK-NEXT:   %[[V:.*]] = vector.broadcast %[[S]] : f32 to vector<f32>


Where is this broadcast generated? Perhaps that's something that could deleted?

Because vector::ExtractElementOp requires a vector type input, we need to use vector.broadcast to convert scalars into vectors. We directly use vector::ExtractOp, and even for 0-dimensional vectors, there's no longer a need to perform vector.broadcast. :-)

This makes sense, but what code-path generated vector.broadcast to begin with? And how do we know it doesn't require updating?

banach-space · 2024-11-06T13:35:04Z

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

-      extracted = rewriter.create<vector::ExtractOp>(
-          storeOp.getLoc(), storeOp.getValueToStore(), indices);
-    }
+    SmallVector<int64_t> indices(vecType.getRank(), 0);


The comment above this pattern needs updating.

Okay, I have already updated it.

The comment is still referring to vector.extractelement, which your patch gets rid of the vector.extractelement path.

banach-space

This is marked as "draft" again.

Please, could you decide whether this is "draft" or not? Without additional comments, this is very confusing to the reviewers. And not good use of time.

banach-space · 2024-11-07T13:37:52Z

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

-      extracted = rewriter.create<vector::ExtractOp>(
-          storeOp.getLoc(), storeOp.getValueToStore(), indices);
-    }
+    SmallVector<int64_t> indices(vecType.getRank(), 0);


The comment is still referring to vector.extractelement, which your patch gets rid of the vector.extractelement path.

banach-space · 2024-11-07T13:38:51Z

mlir/test/Dialect/Vector/vector-transfer-to-vector-load-store.mlir

    %f0 = arith.constant 0.0 : f32

 //  CHECK-NEXT:   %[[S:.*]] = memref.load %[[MEM]][] : memref<f32>
-//  CHECK-NEXT:   %[[V:.*]] = vector.broadcast %[[S]] : f32 to vector<f32>


This makes sense, but what code-path generated vector.broadcast to begin with? And how do we know it doesn't require updating?

harrisonGPU · 2024-11-07T14:07:15Z

This is marked as "draft" again.

Please, could you decide whether this is "draft" or not? Without additional comments, this is very confusing to the reviewers. And not good use of time.

Sorry Andrzej, because I think I misunderstand this issue, so I need some time to think about it, so sorry. :-)

banach-space · 2024-11-07T21:02:37Z

This is marked as "draft" again.
Please, could you decide whether this is "draft" or not? Without additional comments, this is very confusing to the reviewers. And not good use of time.

Sorry Andrzej, because I think I misunderstand this issue, so I need some time to think about it, so sorry. :-)

I understand :) For clarity in the future, it would help us reviewers if you could leave a quick note or a status update when a PR moves back to "draft" after being initially ready for review. This will ensure that everyone is on the same page and can use their time efficiently.

If there's anything specific you’re stuck on, feel free to share! Happy to help if possible.

dcaballe

LG % ongoing comments. Thanks!

dcaballe

Sorry, I didn't see the "Draft" state. What is missing?

Groverkss

Thank you for the patch. I think there is a misunderstanding of the issue linked. The issue is about removing this pattern. You can see the comment on VectorLoadToMemrefLoad:

/// Replace a 0-d vector.load with a memref.load + vector.broadcast.
// TODO: we shouldn't cross the vector/scalar domains just for this
// but atm we lack the infra to avoid it. Possible solutions include:
// - go directly to LLVM + bitcast
// - introduce a bitcast op and likely a new pointer dialect
// - let memref.load/store additionally support the 0-d vector case
// There are still deeper data layout issues lingering even in this
// trivial case (for architectures for which this matters).

Support non 0-d vectors in this pattern doesn't really get us anything. We want to instead verify if the TransferRead -> vector.load -> LLVM/SPIRV conversions are robust enough to handle 0-d vectorsa and fix them.

harrisonGPU · 2024-11-08T01:59:33Z

LG % ongoing comments. Thanks!

Hi, Diego, I misunderstood this issue, so I will update this PR. Thank you for reviewing this PR. :)

harrisonGPU · 2024-11-08T02:00:26Z

This is marked as "draft" again.
Please, could you decide whether this is "draft" or not? Without additional comments, this is very confusing to the reviewers. And not good use of time.

Sorry Andrzej, because I think I misunderstand this issue, so I need some time to think about it, so sorry. :-)

I understand :) For clarity in the future, it would help us reviewers if you could leave a quick note or a status update when a PR moves back to "draft" after being initially ready for review. This will ensure that everyone is on the same page and can use their time efficiently.

If there's anything specific you’re stuck on, feel free to share! Happy to help if possible.

Thanks Andrzej, I will update this PR again. :-)

harrisonGPU · 2024-11-08T02:02:12Z

Thank you for the patch. I think there is a misunderstanding of the issue linked. The issue is about removing this pattern. You can see the comment on VectorLoadToMemrefLoad:
/// Replace a 0-d vector.load with a memref.load + vector.broadcast.
// TODO: we shouldn't cross the vector/scalar domains just for this
// but atm we lack the infra to avoid it. Possible solutions include:
// - go directly to LLVM + bitcast
// - introduce a bitcast op and likely a new pointer dialect
// - let memref.load/store additionally support the 0-d vector case
// There are still deeper data layout issues lingering even in this
// trivial case (for architectures for which this matters).
Support non 0-d vectors in this pattern doesn't really get us anything. We want to instead verify if the TransferRead -> vector.load -> LLVM/SPIRV conversions are robust enough to handle 0-d vectorsa and fix them.

Okay, I will consider and verify it. :-)

Groverkss · 2024-11-11T13:56:45Z

Nice, the patch is in the right direction. Can you:

Since the 0-d case is supported now, can you remove the VectorLoadToMemrefLoad/VectorStoreToMemrefStore patterns? This will give you a more accurate view of what needs to be updated.
Can you check if the test coverage for 0-d transfer_read/transfer_write is enough? If it is enough, please mention it in the PR description and point it out. Otherwise, can you add more tests related to it?

harrisonGPU requested a review from Groverkss October 18, 2024 17:12

harrisonGPU self-assigned this Oct 18, 2024

harrisonGPU mentioned this pull request Oct 18, 2024

[mlir][Vector] Improve support for 0-d vectors in vector dialect lowerings #112913

Open

[mlir][Vector] Support 0-d vectors natively in VectorStoreToMemrefSto…

5e07a46

…reLowering.

harrisonGPU force-pushed the harrison/mlir branch from 0b96a31 to 5e07a46 Compare November 5, 2024 12:11

harrisonGPU changed the title ~~[MLIR] Merge AnyVector and AnyVectorOfAnyRank type constraints.~~ [mlir][Vector] Support 0-d vectors natively in VectorStoreToMemrefStoreLowering. Nov 5, 2024

harrisonGPU marked this pull request as ready for review November 5, 2024 12:14

harrisonGPU requested review from antiagainst, banach-space, dcaballe, hanhanW, kuhar and nicolasvasilache as code owners November 5, 2024 12:14

llvmbot added mlir:spirv mlir:vectorops mlir mlir:vector labels Nov 5, 2024

harrisonGPU marked this pull request as draft November 5, 2024 12:14

harrisonGPU and others added 3 commits November 5, 2024 20:16

Merge branch 'main' into harrison/mlir

e5b8fac

[MLIR] Fix the lit test failure issue.

3f9f7b0

Merge branch 'main' into harrison/mlir

ec6b5ab

harrisonGPU marked this pull request as ready for review November 6, 2024 01:47

banach-space reviewed Nov 6, 2024

View reviewed changes

[MLIR] Update comments.

5f61d16

harrisonGPU force-pushed the harrison/mlir branch from 2b9e931 to 5f61d16 Compare November 6, 2024 15:50

[MLIR] Update comments again.

6db82e0

harrisonGPU marked this pull request as draft November 6, 2024 15:59

banach-space reviewed Nov 7, 2024

View reviewed changes

dcaballe approved these changes Nov 7, 2024

View reviewed changes

dcaballe reviewed Nov 7, 2024

View reviewed changes

Groverkss requested changes Nov 7, 2024

View reviewed changes

[MLIR][Vector] Remove 0-d corner case condition.

6a5ac3c

harrisonGPU changed the title ~~[mlir][Vector] Support 0-d vectors natively in VectorStoreToMemrefStoreLowering.~~ [mlir][Vector] Remove 0-d corner case condition. Nov 10, 2024

[MLIR] Remove Transfer vectore lower pattern.

700b0ea

harrisonGPU closed this May 21, 2025

harrisonGPU deleted the harrison/mlir branch August 22, 2025 16:36

[mlir][Vector] Remove 0-d corner case condition. #112937

[mlir][Vector] Remove 0-d corner case condition. #112937

Uh oh!

Conversation

harrisonGPU commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 5, 2024

Uh oh!

banach-space left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

banach-space left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harrisonGPU commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

banach-space commented Nov 7, 2024

Uh oh!

dcaballe left a comment

Choose a reason for hiding this comment

Uh oh!

dcaballe left a comment

Choose a reason for hiding this comment

Uh oh!

Groverkss left a comment

Choose a reason for hiding this comment

Uh oh!

harrisonGPU commented Nov 8, 2024

Uh oh!

harrisonGPU commented Nov 8, 2024

Uh oh!

harrisonGPU commented Nov 8, 2024

Uh oh!

Groverkss commented Nov 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

harrisonGPU commented Oct 18, 2024 •

edited

Loading

llvmbot commented Nov 5, 2024 •

edited

Loading

harrisonGPU commented Nov 7, 2024 •

edited

Loading