[mlir] Add "memref::MemRefDialect" as dependentDialects for GpuToLLVMConversionPass

[python3kgae]

Summary

For https://github.com/llvm/llvm-project/issues/60070.
The issue is caused by memref.store is not registed.
Registe it by add "memref::MemRefDialect" as dependetDialects for GpuToLLVMConsersionPass.

[ftynse]

Do you know why we are generating ops from the memref dialect in gpu to llvm conversion in the first place?

[python3kgae]

In D141987#4061479, @ftynse wrote:

Do you know why we are generating ops from the memref dialect in gpu to llvm conversion in the first place?

It happens in VectorStoreToMemrefStoreLowering::matchAndRewrite https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp#L2203 where rewriter.replaceOpWithNewOp<memref::StoreOp> is called.

It is added by populateVectorToLLVMConversionPatterns in GpuToLLVMConversionPass::runOnOperation.

But windows build seems go different pattern and not have the dependency. :( So the windows build behavior is the correct path and gpu to llvm conversion don't need memref dialect?

[python3kgae]

Both VectorStoreToMemrefStoreLowering and VectorLoadStoreConversion<mlir::vector::StoreOp, mlir::vector::StoreOpAdaptor> are added to legalize vector.store.
On windows, VectorLoadStoreConversion has higher benefit, so it does not crash.
And on linux, VectorStoreToMemrefStoreLowering has higher benefit, and memref.store is not resisted, so it will crash.

If patterns added for vector.store are expected to be selected, memref.store should be registered.

Still not sure why windows and linux has different benefit for VectorStoreToMemrefStoreLowering and VectorLoadStoreConversion<mlir::vector::StoreOp, mlir::vector::StoreOpAdaptor>. Should I create an issue for the difference?

[ftynse]

Does one of the patterns actually has a higher benefit value? Or do they have the default value and one just happens to be selected instead of the other? In the latter case, it sounds like there is some non-determinism involved (e.g., something stored in a hashmap) that is exposed by platform differences.

[python3kgae]

In D141987#4064561, @ftynse wrote:

Does one of the patterns actually has a higher benefit value? Or do they have the default value and one just happens to be selected instead of the other? In the latter case, it sounds like there is some non-determinism involved (e.g., something stored in a hashmap) that is exposed by platform differences.

The difference is caused by llvm::array_pod_sort used in

unsigned OperationLegalizer::applyCostModelToPatterns(
       unsigned generatedOpDepth = computeOpLegalizationDepth(
           generatedOp, minOpPatternDepth, legalizerPatterns);
       depth = std::max(depth, generatedOpDepth + 1);
     }

If change it to std::stable_sort, both windows and linux will select VectorStoreToMemrefStoreLowering and crash.

[python3kgae]

Create https://reviews.llvm.org/D142110 for the non-determinism between windows and linux.

[ftynse]

It happens in VectorStoreToMemrefStoreLowering::matchAndRewrite https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp#L2203 where rewriter.replaceOpWithNewOp<memref::StoreOp> is called.

I suppose the same problem is possible with just -convert-vector-to-llvm. This is what happens with uncontrolled addition of patterns :(

[python3kgae]

rebase to get stable_sort fix

mlir/include/mlir/Conversion/Passes.td

 
 //===----------------------------------------------------------------------===//
 // GPUCommon
 //===----------------------------------------------------------------------===//
 
 def GpuToLLVMConversionPass : Pass<"gpu-to-llvm", "ModuleOp"> {
   let summary = "Convert GPU dialect to LLVM dialect with GPU runtime calls";
   let constructor = "mlir::createGpuToLLVMConversionPass()";
-  let dependentDialects = ["LLVM::LLVMDialect"];
+  let dependentDialects = [
+    "LLVM::LLVMDialect",
+    "memref::MemRefDialect",
+  ];
 }
 
 def LowerHostCodeToLLVM : Pass<"lower-host-to-llvm", "ModuleOp"> {
   let summary = "Lowers the host module code and `gpu.launch_func` to LLVM";
   let constructor = "mlir::createLowerHostCodeToLLVMPass()";
   let dependentDialects = ["LLVM::LLVMDialect"];
 }
 

mlir/lib/Conversion/GPUCommon/GPUToLLVMConversion.cpp

 #include "mlir/Conversion/LLVMCommon/ConversionTarget.h"
 #include "mlir/Conversion/LLVMCommon/Pattern.h"
 #include "mlir/Conversion/MemRefToLLVM/MemRefToLLVM.h"
 #include "mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h"
 #include "mlir/Dialect/Async/IR/Async.h"
 #include "mlir/Dialect/GPU/IR/GPUDialect.h"
 #include "mlir/Dialect/GPU/Transforms/Passes.h"
 #include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/Dialect/MemRef/IR/MemRef.h"
 #include "mlir/IR/Attributes.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/BuiltinTypes.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"

mlir/test/Conversion/GPUCommon/transfer_write.mlir

+// RUN: mlir-opt %s --gpu-to-llvm | FileCheck %s
+
+  func.func @warp_extract(%arg0: index, %arg1: memref<1024x1024xf32>, %arg2: index, %arg3: vector<1xf32>) {
+    %c0 = arith.constant 0 : index
+    vector.warp_execute_on_lane_0(%arg0)[32] {
+      // CHECK:%[[val:[0-9]+]] = llvm.extractelement
+      // CHECK:%[[base:[0-9]+]] = llvm.extractvalue
+      // CHECK:%[[ptr:[0-9]+]] = llvm.getelementptr %[[base]]
+      // CHECK:llvm.store %[[val]], %[[ptr]]
+      vector.transfer_write %arg3, %arg1[%c0, %c0] {in_bounds = [true]} : vector<1xf32>, memref<1024x1024xf32>
+    }
+    return
+  }