[MLIR][SPIRVToLLVM] SPIRV function fix and nits

[georgemitenkov]

Summary

This patch addresses the comments from https://reviews.llvm.org/D83030 and https://reviews.llvm.org/D82639. this-> is removed when not inside the template.
Also, spv.func is fixed: now convertRegionTypes() is used to apply type conversion on blocks within the function.

[rriddle]

mlir/lib/Conversion/SPIRVToLLVM/ConvertSPIRVToLLVM.cpp

679–682

You don't need applySignatureConversion if you have convertRegionTypes.

[georgemitenkov]

Removed the call to applySignatureConversion().

mlir/lib/Conversion/SPIRVToLLVM/ConvertSPIRVToLLVM.cpp

static IntegerAttr minusOneIntegerAttribute(Type type, Builder builder) {
   }
   auto integerType = type.cast<IntegerType>();
   return builder.getIntegerAttr(integerType, -1);
 }
 
 /// Creates `llvm.mlir.constant` with all bits set for the given type.
 static Value createConstantAllBitsSet(Location loc, Type srcType, Type dstType,
                                       PatternRewriter &rewriter) {
-  if (srcType.isa<VectorType>())
+  if (srcType.isa<VectorType>()) {
     return rewriter.create<LLVM::ConstantOp>(
         loc, dstType,
         SplatElementsAttr::get(srcType.cast<ShapedType>(),
                                minusOneIntegerAttribute(srcType, rewriter)));
+  }
   return rewriter.create<LLVM::ConstantOp>(
       loc, dstType, minusOneIntegerAttribute(srcType, rewriter));
 }
 
 /// Utility function for bitfiled ops:
 ///   - `BitFieldInsert`
 ///   - `BitFieldSExtract`
 ///   - `BitFieldUExtract`
class BitFieldInsertPattern
     : public SPIRVToLLVMConversion<spirv::BitFieldInsertOp> {
 public:
   using SPIRVToLLVMConversion<spirv::BitFieldInsertOp>::SPIRVToLLVMConversion;
 
   LogicalResult
   matchAndRewrite(spirv::BitFieldInsertOp op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const override {
     auto srcType = op.getType();
-    auto dstType = this->typeConverter.convertType(srcType);
+    auto dstType = typeConverter.convertType(srcType);
     if (!dstType)
       return failure();
     Location loc = op.getLoc();
 
     // Process `Offset` and `Count`: broadcast and extend/truncate if needed.
     Value offset = processCountOrOffset(loc, op.offset(), srcType, dstType,
                                         typeConverter, rewriter);
     Value count = processCountOrOffset(loc, op.count(), srcType, dstType,
class BitFieldSExtractPattern
     : public SPIRVToLLVMConversion<spirv::BitFieldSExtractOp> {
 public:
   using SPIRVToLLVMConversion<spirv::BitFieldSExtractOp>::SPIRVToLLVMConversion;
 
   LogicalResult
   matchAndRewrite(spirv::BitFieldSExtractOp op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const override {
     auto srcType = op.getType();
-    auto dstType = this->typeConverter.convertType(srcType);
+    auto dstType = typeConverter.convertType(srcType);
     if (!dstType)
       return failure();
     Location loc = op.getLoc();
 
     // Process `Offset` and `Count`: broadcast and extend/truncate if needed.
     Value offset = processCountOrOffset(loc, op.offset(), srcType, dstType,
                                         typeConverter, rewriter);
     Value count = processCountOrOffset(loc, op.count(), srcType, dstType,
class BitFieldUExtractPattern
     : public SPIRVToLLVMConversion<spirv::BitFieldUExtractOp> {
 public:
   using SPIRVToLLVMConversion<spirv::BitFieldUExtractOp>::SPIRVToLLVMConversion;
 
   LogicalResult
   matchAndRewrite(spirv::BitFieldUExtractOp op, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const override {
     auto srcType = op.getType();
-    auto dstType = this->typeConverter.convertType(srcType);
+    auto dstType = typeConverter.convertType(srcType);
     if (!dstType)
       return failure();
     Location loc = op.getLoc();
 
     // Process `Offset` and `Count`: broadcast and extend/truncate if needed.
     Value offset = processCountOrOffset(loc, op.offset(), srcType, dstType,
                                         typeConverter, rewriter);
     Value count = processCountOrOffset(loc, op.count(), srcType, dstType,
matchAndRewrite(spirv::FunctionCallOp callOp, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const override {
     if (callOp.getNumResults() == 0) {
       rewriter.replaceOpWithNewOp<LLVM::CallOp>(callOp, llvm::None, operands,
                                                 callOp.getAttrs());
       return success();
     }
 
     // Function returns a single result.
-    auto dstType = this->typeConverter.convertType(callOp.getType(0));
+    auto dstType = typeConverter.convertType(callOp.getType(0));
     rewriter.replaceOpWithNewOp<LLVM::CallOp>(callOp, dstType, operands,
                                               callOp.getAttrs());
     return success();
   }
 };
 
 /// Converts SPIR-V floating-point comparisons to llvm.fcmp "predicate"
 template <typename SPIRVOp, LLVM::FCmpPredicate predicate>
public:
   matchAndRewrite(spirv::FuncOp funcOp, ArrayRef<Value> operands,
                   ConversionPatternRewriter &rewriter) const override {
 
     // Convert function signature. At the moment LLVMType converter is enough
     // for currently supported types.
     auto funcType = funcOp.getType();
     TypeConverter::SignatureConversion signatureConverter(
         funcType.getNumInputs());
-    auto llvmType = this->typeConverter.convertFunctionSignature(
+    auto llvmType = typeConverter.convertFunctionSignature(
         funcOp.getType(), /*isVariadic=*/false, signatureConverter);
     if (!llvmType)
       return failure();
 
     // Create a new `LLVMFuncOp`
     Location loc = funcOp.getLoc();
     StringRef name = funcOp.getName();
     auto newFuncOp = rewriter.create<LLVM::LLVMFuncOp>(loc, name, llvmType);
#undef DISPATCH
     // Default: if `spirv::FunctionControl::None`, then no attributes are
     // needed.
     default:
       break;
     }
 
     rewriter.inlineRegionBefore(funcOp.getBody(), newFuncOp.getBody(),
                                 newFuncOp.end());
-    rewriter.applySignatureConversion(&newFuncOp.getBody(), signatureConverter);
+    if (failed(rewriter.convertRegionTypes(&newFuncOp.getBody(), typeConverter,
+                                           &signatureConverter))) {
+      return failure();
+    }
     rewriter.eraseOp(funcOp);
     return success();
   }
 };
 
 //===----------------------------------------------------------------------===//
 // ModuleOp conversion
 //===----------------------------------------------------------------------===//