[Mlir-commits] [mlir] 7f4dbd8 - [mlir][GPU][NFC] Remove type converter hack

[Matthias Springer]

Author: Matthias Springer
Date: 2023-08-18T15:28:47+02:00
New Revision: 7f4dbd83dc152a852a3d78460aae7b426f53b9ed

URL: https://github.com/llvm/llvm-project/commit/7f4dbd83dc152a852a3d78460aae7b426f53b9ed
DIFF: https://github.com/llvm/llvm-project/commit/7f4dbd83dc152a852a3d78460aae7b426f53b9ed.diff

LOG: [mlir][GPU][NFC] Remove type converter hack

Remove `dangerousSetOptions` and call `promoteOperands` with the correct arguments directly.

Differential Revision: https://reviews.llvm.org/D158175

Added: 
    

Modified: 
    mlir/include/mlir/Conversion/LLVMCommon/TypeConverter.h
    mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Conversion/LLVMCommon/TypeConverter.h b/mlir/include/mlir/Conversion/LLVMCommon/TypeConverter.h
index 2097aa78ebd70e..94a2c7c2c52ae5 100644
--- a/mlir/include/mlir/Conversion/LLVMCommon/TypeConverter.h
+++ b/mlir/include/mlir/Conversion/LLVMCommon/TypeConverter.h
@@ -92,13 +92,6 @@ class LLVMTypeConverter : public TypeConverter {
 
   const LowerToLLVMOptions &getOptions() const { return options; }
 
-  /// Set the lowering options to `newOptions`. Note: using this after some
-  /// some conversions have been performed can lead to inconsistencies in the
-  /// IR.
-  void dangerousSetOptions(LowerToLLVMOptions newOptions) {
-    options = std::move(newOptions);
-  }
-
   /// Promote the LLVM representation of all operands including promoting MemRef
   /// descriptors to stack and use pointers to struct to avoid the complexity
   /// of the platform-specific C/C++ ABI lowering related to struct argument

diff  --git a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
index a51fbb9b032862..cbcc82b23b8f9c 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp
@@ -981,24 +981,12 @@ Value ConvertLaunchFuncOpToGpuRuntimeCallPattern::generateParamsArray(
     gpu::LaunchFuncOp launchOp, OpAdaptor adaptor, OpBuilder &builder) const {
   auto loc = launchOp.getLoc();
   auto numKernelOperands = launchOp.getNumKernelOperands();
-  SmallVector<Value, 4> arguments;
-  if (kernelBarePtrCallConv) {
-    // Hack the bare pointer value on just for the argument promotion
-    const LLVMTypeConverter *converter = getTypeConverter();
-    LowerToLLVMOptions options = converter->getOptions();
-    LowerToLLVMOptions overrideToMatchKernelOpts = options;
-    overrideToMatchKernelOpts.useBarePtrCallConv = true;
-    LLVMTypeConverter newConverter = *converter;
-    newConverter.dangerousSetOptions(overrideToMatchKernelOpts);
-    arguments = newConverter.promoteOperands(
-        loc, launchOp.getOperands().take_back(numKernelOperands),
-        adaptor.getOperands().take_back(numKernelOperands), builder);
-  } else {
-    arguments = getTypeConverter()->promoteOperands(
-        loc, launchOp.getOperands().take_back(numKernelOperands),
-        adaptor.getOperands().take_back(numKernelOperands), builder);
-  }
-
+  // Note: If `useBarePtrCallConv` is set in the type converter's options,
+  // the value of `kernelBarePtrCallConv` will be ignored.
+  SmallVector<Value, 4> arguments = getTypeConverter()->promoteOperands(
+      loc, launchOp.getOperands().take_back(numKernelOperands),
+      adaptor.getOperands().take_back(numKernelOperands), builder,
+      /*useBarePtrCallConv=*/kernelBarePtrCallConv);
   auto numArguments = arguments.size();
   SmallVector<Type, 4> argumentTypes;
   argumentTypes.reserve(numArguments);
@@ -1112,23 +1100,11 @@ LogicalResult ConvertLaunchFuncOpToGpuRuntimeCallPattern::matchAndRewrite(
       stream = streamCreateCallBuilder.create(loc, rewriter, {}).getResult();
 
     // Lower the kernel operands to match kernel parameters.
-    SmallVector<Value, 4> arguments;
-    if (kernelBarePtrCallConv) {
-      // Hack the bare pointer value on just for the argument promotion
-      const LLVMTypeConverter *converter = getTypeConverter();
-      LowerToLLVMOptions options = converter->getOptions();
-      LowerToLLVMOptions overrideToMatchKernelOpts = options;
-      overrideToMatchKernelOpts.useBarePtrCallConv = true;
-      LLVMTypeConverter newConverter = *converter;
-      newConverter.dangerousSetOptions(overrideToMatchKernelOpts);
-      arguments =
-          newConverter.promoteOperands(loc, launchOp.getKernelOperands(),
-                                       adaptor.getKernelOperands(), rewriter);
-    } else {
-      arguments = getTypeConverter()->promoteOperands(
-          loc, launchOp.getKernelOperands(), adaptor.getKernelOperands(),
-          rewriter);
-    }
+    // Note: If `useBarePtrCallConv` is set in the type converter's options,
+    // the value of `kernelBarePtrCallConv` will be ignored.
+    SmallVector<Value, 4> arguments = getTypeConverter()->promoteOperands(
+        loc, launchOp.getKernelOperands(), adaptor.getKernelOperands(),
+        rewriter, /*useBarePtrCallConv=*/kernelBarePtrCallConv);
 
     rewriter.create<gpu::LaunchFuncOp>(
         launchOp.getLoc(), launchOp.getKernelAttr(),