OpConvertSToF support operands with different bitwidth, close SameBitWidth check in verifier.

[XinWangIntel]

Summary

Signed-off-by: xin1.wang <[e-mail]>

[XinWangIntel]

Spec does not show bitwidth limit. From https://github.com/KhronosGroup/SPIRV-Tools/blob/master/test/val/val_conversion_test.cpp#L372 can get a cast to convert s64 to f32; And std.SIToFPOp dose not has the limit too. May be same issues with other cast ops

[XinWangIntel]

ConvertSToF shall support operands with same bitwidth and different bitwidth. The check of component number shall be done by another way.

[ThomasRaoux]

Why is this only for SToFP op? We should keep it consistent for the different converts. Also you need to add test(s).

[XinWangIntel]

Just to keep minimal changes as i need this op support now. Will check other castOp and add tests soon, thanks.

[XinWangIntel]

SPV_ConvertFToSOp, SPV_ConvertFToUOp, SPV_ConvertSToFOp, SPV_ConvertUToFOp can support same && different bit widths of their operand type and result type. As The type definition limits the type and ODS checks the shape, skip bit widths check for those CastOps are enough now.

[ThomasRaoux]

mlir/lib/Dialect/SPIRV/SPIRVOps.cpp

310–312

This seem to skip more than just the bitwidth check. I don't think we can skip the check that the shape are different. Why can't we just set requireSameBitWidth to false?

[XinWangIntel]

mlir/lib/Dialect/SPIRV/SPIRVOps.cpp

310–312

When we just set requireSameBitWidth to false, it will report error when the bit widths of operand and result type are same(The 344 line check this), all the codes below just get element type and check bit widths, and either request same or different, no other choice, so just ignore. For shape issue, the CastOp definition use SameOperandsAndResultShape trait to check this, no check here.

[ThomasRaoux]

mlir/lib/Dialect/SPIRV/SPIRVOps.cpp

310–312

You're right, my bad.

[XinWangIntel]

I don't have commit access as a new contributor，if everything is fine, can anyone help me push the changes? Really thanks.

[XinWangIntel]

Do i need some changes? or anyone can help push the patch?

mlir/include/mlir/Dialect/SPIRV/SPIRVCastOps.td

let description = [{
 
     #### Example:
 
     ```mlir
     %1 = spv.ConvertFToS %0 : f32 to i32
     %3 = spv.ConvertFToS %2 : vector<3xf32> to vector<3xi32>
     ```
   }];
+
+  let verifier = [{ return verifyCastOp(this->getOperation(), false, true); }];
 }
 
 // -----
 
 def SPV_ConvertFToUOp : SPV_CastOp<"ConvertFToU", SPV_Integer, SPV_Float, []> {
   let summary = [{
     Convert value numerically from floating point to unsigned integer, with
     round toward 0.0.
let description = [{
 
     #### Example:
 
     ```mlir
     %1 = spv.ConvertFToU %0 : f32 to i32
     %3 = spv.ConvertFToU %2 : vector<3xf32> to vector<3xi32>
     ```
   }];
+
+  let verifier = [{ return verifyCastOp(this->getOperation(), false, true); }];
 }
 
 // -----
 
 def SPV_ConvertSToFOp : SPV_CastOp<"ConvertSToF", SPV_Float, SPV_Integer, []> {
   let summary = [{
     Convert value numerically from signed integer to floating point.
   }];
let description = [{
 
     #### Example:
 
     ```mlir
     %1 = spv.ConvertSToF %0 : i32 to f32
     %3 = spv.ConvertSToF %2 : vector<3xi32> to vector<3xf32>
     ```
   }];
+
+  let verifier = [{ return verifyCastOp(this->getOperation(), false, true); }];
 }
 
 // -----
 
 def SPV_ConvertUToFOp : SPV_CastOp<"ConvertUToF", SPV_Float, SPV_Integer, []> {
   let summary = [{
     Convert value numerically from unsigned integer to floating point.
   }];
let description = [{
 
     #### Example:
 
     ```mlir
     %1 = spv.ConvertUToF %0 : i32 to f32
     %3 = spv.ConvertUToF %2 : vector<3xi32> to vector<3xf32>
     ```
   }];
+
+  let verifier = [{ return verifyCastOp(this->getOperation(), false, true); }];
 }
 
 // -----
 
 def SPV_FConvertOp : SPV_CastOp<"FConvert", SPV_Float, SPV_Float, []> {
   let summary = [{
     Convert value numerically from one floating-point width to another
     width.

mlir/lib/Dialect/SPIRV/SPIRVOps.cpp

if (spirv::bitEnumContains(*memAccess, spirv::MemoryAccess::Aligned)) {
       }
     }
     printer << "]";
   }
   elidedAttrs.push_back(spirv::attributeName<spirv::StorageClass>());
 }
 
 static LogicalResult verifyCastOp(Operation *op,
-                                  bool requireSameBitWidth = true) {
+                                  bool requireSameBitWidth = true,
+                                  bool skipBitWidthCheck = false) {
+  // Some CastOps have no limit on bit widths for result and operand type.
+  if (skipBitWidthCheck)
+    return success();
+
   Type operandType = op->getOperand(0).getType();
   Type resultType = op->getResult(0).getType();
 
   // ODS checks that result type and operand type have the same shape.
   if (auto vectorType = operandType.dyn_cast<VectorType>()) {
     operandType = vectorType.getElementType();
     resultType = resultType.cast<VectorType>().getElementType();
   }

mlir/test/Dialect/SPIRV/Serialization/cast-ops.mlir

 // -----
 
 spv.module Logical GLSL450 requires #spv.vce<v1.0, [Shader], []> {
   spv.func @convert_f_to_s(%arg0 : f32) -> i32 "None" {
     // CHECK: {{%.*}} = spv.ConvertFToS {{%.*}} : f32 to i32
     %0 = spv.ConvertFToS %arg0 : f32 to i32
     spv.ReturnValue %0 : i32
   }
+  spv.func @convert_f64_to_s32(%arg0 : f64) -> i32 "None" {
+    // CHECK: {{%.*}} = spv.ConvertFToS {{%.*}} : f64 to i32
+    %0 = spv.ConvertFToS %arg0 : f64 to i32
+    spv.ReturnValue %0 : i32
+  }
   spv.func @convert_f_to_u(%arg0 : f32) -> i32 "None" {
     // CHECK: {{%.*}} = spv.ConvertFToU {{%.*}} : f32 to i32
     %0 = spv.ConvertFToU %arg0 : f32 to i32
     spv.ReturnValue %0 : i32
   }
+  spv.func @convert_f64_to_u32(%arg0 : f64) -> i32 "None" {
+    // CHECK: {{%.*}} = spv.ConvertFToU {{%.*}} : f64 to i32
+    %0 = spv.ConvertFToU %arg0 : f64 to i32
+    spv.ReturnValue %0 : i32
+  }
   spv.func @convert_s_to_f(%arg0 : i32) -> f32 "None" {
     // CHECK: {{%.*}} = spv.ConvertSToF {{%.*}} : i32 to f32
     %0 = spv.ConvertSToF %arg0 : i32 to f32
     spv.ReturnValue %0 : f32
   }
+  spv.func @convert_s64_to_f32(%arg0 : i64) -> f32 "None" {
+    // CHECK: {{%.*}} = spv.ConvertSToF {{%.*}} : i64 to f32
+    %0 = spv.ConvertSToF %arg0 : i64 to f32
+    spv.ReturnValue %0 : f32
+  }
   spv.func @convert_u_to_f(%arg0 : i32) -> f32 "None" {
     // CHECK: {{%.*}} = spv.ConvertUToF {{%.*}} : i32 to f32
     %0 = spv.ConvertUToF %arg0 : i32 to f32
     spv.ReturnValue %0 : f32
   }
+  spv.func @convert_u64_to_f32(%arg0 : i64) -> f32 "None" {
+    // CHECK: {{%.*}} = spv.ConvertUToF {{%.*}} : i64 to f32
+    %0 = spv.ConvertUToF %arg0 : i64 to f32
+    spv.ReturnValue %0 : f32
+  }
   spv.func @f_convert(%arg0 : f32) -> f64 "None" {
     // CHECK: {{%.*}} = spv.FConvert {{%.*}} : f32 to f64
     %0 = spv.FConvert %arg0 : f32 to f64
     spv.ReturnValue %0 : f64
   }
   spv.func @s_convert(%arg0 : i32) -> i64 "None" {
     // CHECK: {{%.*}} = spv.SConvert {{%.*}} : i32 to i64
     %0 = spv.SConvert %arg0 : i32 to i64
     spv.ReturnValue %0 : i64
   }
   spv.func @u_convert(%arg0 : i32) -> i64 "None" {
     // CHECK: {{%.*}} = spv.UConvert {{%.*}} : i32 to i64
     %0 = spv.UConvert %arg0 : i32 to i64
     spv.ReturnValue %0 : i64
   }
 }

mlir/test/Dialect/SPIRV/ops.mlir

 func @convert_f_to_s_scalar(%arg0 : f32) -> i32 {
   // CHECK: {{%.*}} = spv.ConvertFToS {{%.*}} : f32 to i32
   %0 = spv.ConvertFToS %arg0 : f32 to i32
   spv.ReturnValue %0 : i32
 }
 
 // -----
 
+func @convert_f64_to_s32_scalar(%arg0 : f64) -> i32 {
+  // CHECK: {{%.*}} = spv.ConvertFToS {{%.*}} : f64 to i32
+  %0 = spv.ConvertFToS %arg0 : f64 to i32
+  spv.ReturnValue %0 : i32
+}
+
+// -----
+
+func @convert_f_to_s_vector(%arg0 : vector<3xf32>) -> vector<3xi32> {
+  // CHECK: {{%.*}} = spv.ConvertFToS {{%.*}} : vector<3xf32> to vector<3xi32>
+  %0 = spv.ConvertFToS %arg0 : vector<3xf32> to vector<3xi32>
+  spv.ReturnValue %0 : vector<3xi32>
+}
+
+// -----
+
 //===----------------------------------------------------------------------===//
 // spv.ConvertFToU
 //===----------------------------------------------------------------------===//
 
 func @convert_f_to_u_scalar(%arg0 : f32) -> i32 {
   // CHECK: {{%.*}} = spv.ConvertFToU {{%.*}} : f32 to i32
   %0 = spv.ConvertFToU %arg0 : f32 to i32
   spv.ReturnValue %0 : i32
 }
 
 // -----
 
+func @convert_f64_to_u32_scalar(%arg0 : f64) -> i32 {
+  // CHECK: {{%.*}} = spv.ConvertFToU {{%.*}} : f64 to i32
+  %0 = spv.ConvertFToU %arg0 : f64 to i32
+  spv.ReturnValue %0 : i32
+}
+
+// -----
+
 func @convert_f_to_u_vector(%arg0 : vector<3xf32>) -> vector<3xi32> {
   // CHECK: {{%.*}} = spv.ConvertFToU {{%.*}} : vector<3xf32> to vector<3xi32>
   %0 = spv.ConvertFToU %arg0 : vector<3xf32> to vector<3xi32>
   spv.ReturnValue %0 : vector<3xi32>
 }
 
 // -----
 
 func @convert_f_to_u_coopmatrix(%arg0 : !spv.coopmatrix<8x16xf32, Subgroup>) {
   // CHECK: {{%.*}} = spv.ConvertFToU {{%.*}} : !spv.coopmatrix<8x16xf32, Subgroup> to !spv.coopmatrix<8x16xi32, Subgroup>
   %0 = spv.ConvertFToU %arg0 : !spv.coopmatrix<8x16xf32, Subgroup> to !spv.coopmatrix<8x16xi32, Subgroup>
   spv.Return
 }
 
 // -----
 
-func @convert_f_to_u_scalar_invalid(%arg0 : f16) -> i32 {
-  // expected-error @+1 {{expected the same bit widths for operand type and result type, but provided 'f16' and 'i32'}}
-  %0 = spv.ConvertFToU %arg0 : f16 to i32
-  spv.ReturnValue %0 : i32
-}
-
-// -----
-
 //===----------------------------------------------------------------------===//
 // spv.ConvertSToF
 //===----------------------------------------------------------------------===//
 
 func @convert_s_to_f_scalar(%arg0 : i32) -> f32 {
   // CHECK: {{%.*}} = spv.ConvertSToF {{%.*}} : i32 to f32
   %0 = spv.ConvertSToF %arg0 : i32 to f32
   spv.ReturnValue %0 : f32
 }
 
 // -----
 
+func @convert_s64_to_f32_scalar(%arg0 : i64) -> f32 {
+  // CHECK: {{%.*}} = spv.ConvertSToF {{%.*}} : i64 to f32
+  %0 = spv.ConvertSToF %arg0 : i64 to f32
+  spv.ReturnValue %0 : f32
+}
+
+// -----
+
+func @convert_s_to_f_vector(%arg0 : vector<3xi32>) -> vector<3xf32> {
+  // CHECK: {{%.*}} = spv.ConvertSToF {{%.*}} : vector<3xi32> to vector<3xf32>
+  %0 = spv.ConvertSToF %arg0 : vector<3xi32> to vector<3xf32>
+  spv.ReturnValue %0 : vector<3xf32>
+}
+
+// -----
+
 //===----------------------------------------------------------------------===//
 // spv.ConvertUToF
 //===----------------------------------------------------------------------===//
 
 func @convert_u_to_f_scalar(%arg0 : i32) -> f32 {
   // CHECK: {{%.*}} = spv.ConvertUToF {{%.*}} : i32 to f32
   %0 = spv.ConvertUToF %arg0 : i32 to f32
   spv.ReturnValue %0 : f32
 }
 
 // -----
 
+func @convert_u64_to_f32_scalar(%arg0 : i64) -> f32 {
+  // CHECK: {{%.*}} = spv.ConvertUToF {{%.*}} : i64 to f32
+  %0 = spv.ConvertUToF %arg0 : i64 to f32
+  spv.ReturnValue %0 : f32
+}
+
+// -----
+
+func @convert_u_to_f_vector(%arg0 : vector<3xi32>) -> vector<3xf32> {
+  // CHECK: {{%.*}} = spv.ConvertUToF {{%.*}} : vector<3xi32> to vector<3xf32>
+  %0 = spv.ConvertUToF %arg0 : vector<3xi32> to vector<3xf32>
+  spv.ReturnValue %0 : vector<3xf32>
+}
+
+// -----
+
 //===----------------------------------------------------------------------===//
 // spv.FConvert
 //===----------------------------------------------------------------------===//
 
 func @f_convert_scalar(%arg0 : f32) -> f64 {
   // CHECK: {{%.*}} = spv.FConvert {{%.*}} : f32 to f64
   %0 = spv.FConvert %arg0 : f32 to f64
   spv.ReturnValue %0 : f64