[mlir][spirv] Simplify scalar type size calculation.

[denis13]

Summary

Simplify scalar type size calculation and handle boolean types.

[merge_guards_bot]

Unit tests: pass. 62001 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[antiagainst]

mlir/test/Conversion/StandardToSPIRV/std-to-spirv.mlir

297

Hmm, we cannot have bool type in externally visible storage classes: https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#OpTypeBool.

This is something that should be fixed. Could you just change to reject memref of booleans for now?

[denis13]

Addressed review comments.

[denis13]

@antiagainst thanks for mention this, fixed!

[merge_guards_bot]

Unit tests: pass. 62001 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[antiagainst]

Thanks!

mlir/lib/Conversion/StandardToSPIRV/ConvertStandardToSPIRVPass.cpp

FuncOpConversion::matchAndRewrite(FuncOp funcOp, ArrayRef<Value> operands,
   if (fnType.getNumResults()) {
     return matchFailure();
   }
 
   TypeConverter::SignatureConversion signatureConverter(fnType.getNumInputs());
   {
     for (auto argType : enumerate(funcOp.getType().getInputs())) {
       auto convertedType = typeConverter.convertType(argType.value());
+      if (!convertedType) {
+        return matchFailure();
+      }
       signatureConverter.addInputs(argType.index(), convertedType);
     }
   }
 
   rewriter.updateRootInPlace(funcOp, [&] {
     funcOp.setType(rewriter.getFunctionType(
         signatureConverter.getConvertedTypes(), llvm::None));
     rewriter.applySignatureConversion(&funcOp.getBody(), signatureConverter);

mlir/lib/Dialect/SPIRV/SPIRVLowering.cpp

Type SPIRVTypeConverter::getIndexType(MLIRContext *context) {
   // GlobalInvocationID, etc. to be 32-bit (unsigned) integers which should be
   // SExtended to 64-bit for index computations.
   return IntegerType::get(32, context);
 }
 
 // TODO(ravishankarm): This is a utility function that should probably be
 // exposed by the SPIR-V dialect. Keeping it local till the use case arises.
 static Optional<int64_t> getTypeNumBytes(Type t) {
-  if (auto integerType = t.dyn_cast<IntegerType>()) {
-    return integerType.getWidth() / 8;
-  } else if (auto floatType = t.dyn_cast<FloatType>()) {
-    return floatType.getWidth() / 8;
+  if (spirv::SPIRVDialect::isValidScalarType(t)) {
+    auto bitWidth = t.getIntOrFloatBitWidth();
+    // According to the SPIR-V spec:
+    // "There is no physical size or bit pattern defined for values with boolean
+    // type. If they are stored (in conjunction with OpVariable), they can only
+    // be used with logical addressing operations, not physical, and only with
+    // non-externally visible shader Storage Classes: Workgroup, CrossWorkgroup,
+    // Private, Function, Input, and Output."
+    if (bitWidth == 1) {
+      return llvm::None;
+    }
+    return bitWidth / 8;
   } else if (auto memRefType = t.dyn_cast<MemRefType>()) {
     // TODO: Layout should also be controlled by the ABI attributes. For now
     // using the layout from MemRef.
     int64_t offset;
     SmallVector<int64_t, 4> strides;
     if (!memRefType.hasStaticShape() ||
         failed(getStridesAndOffset(memRefType, strides, offset))) {
       return llvm::None;

mlir/test/Conversion/StandardToSPIRV/std-to-spirv.mlir

 //===----------------------------------------------------------------------===//
 
 // CHECK-LABEL: @sitofp
 func @sitofp(%arg0 : i32) {
   // CHECK: spv.ConvertSToF
   %0 = std.sitofp %arg0 : i32 to f32
   return
 }
+
+//===----------------------------------------------------------------------===//
+// memref type
+//===----------------------------------------------------------------------===//
+
+// CHECK-LABEL: func @memref_type({{%.*}}: memref<3xi1>) {
+func @memref_type(%arg0: memref<3xi1>) {
+  return
+}