[Mlir-commits] [mlir] [mlir][Affine] Handle null parent op in getAffineParallelInductionVarOwner (PR #142025)

[Mehdi Amini]

joker-eph wrote:

> Since it is a safer check and the method returns null to let callers handle the failure, making one-line change without tests is reasonable.

I'm not concerned about the change, but about the upstream test coverage in general. It happens that there are things we can't have coverage for, but it's pretty rare.

Here I can construct a test case with this patch:


```
diff --git a/mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td b/mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td
index 2d060f3c2da6..f30a57ce38f7 100644
--- a/mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td
+++ b/mlir/include/mlir/Dialect/MemRef/TransformOps/MemRefTransformOps.td
@@ -84,7 +84,7 @@ def ApplyExpandOpsPatternsOp : Op<Transform_Dialect,
 
 def ApplyExpandStridedMetadataPatternsOp : Op<Transform_Dialect,
     "apply_patterns.memref.expand_strided_metadata",
-    [DeclareOpInterfaceMethods<PatternDescriptorOpInterface>]> {
+    [DeclareOpInterfaceMethods<ConversionPatternDescriptorOpInterface>]> {
   let description = [{
     Collects patterns for expanding memref operations that modify the metadata
     (sizes, offset, strides) of a memref into easier to analyze constructs.
diff --git a/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp b/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
index 89640ac323b6..2de8d08cdc09 100644
--- a/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
+++ b/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
@@ -107,7 +107,7 @@ void transform::ApplyExpandOpsPatternsOp::populatePatterns(
 }
 
 void transform::ApplyExpandStridedMetadataPatternsOp::populatePatterns(
-    RewritePatternSet &patterns) {
+    TypeConverter &typeConverter, RewritePatternSet &patterns) {
   memref::populateExpandStridedMetadataPatterns(patterns);
 }
 
diff --git a/mlir/test/Conversion/MemRefToLLVM/memref-subviewfolder-to-llvm.mlir b/mlir/test/Conversion/MemRefToLLVM/memref-subviewfolder-to-llvm.mlir
new file mode 100644
index 000000000000..9dca88fda118
--- /dev/null
+++ b/mlir/test/Conversion/MemRefToLLVM/memref-subviewfolder-to-llvm.mlir
@@ -0,0 +1,26 @@
+// RUN: mlir-opt -transform-interpreter %s | FileCheck  %s
+
+module {
+  func.func @foo(%arg0: memref<100x100xf32>, %arg1: index, %arg2: index, %arg3: index, %arg4: index) -> memref<?x?xf32, strided<[100, 1], offset: ?>> {
+    %subview = memref.subview %arg0[%arg1, %arg2] [%arg3, %arg4] [1, 1] : memref<100x100xf32> to memref<?x?xf32, strided<[100, 1], offset: ?>>
+    return %subview : memref<?x?xf32, strided<[100, 1], offset: ?>>
+  }
+}
+
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%toplevel_module: !transform.any_op {transform.readonly}) {
+    %func = transform.structured.match ops{["func.func"]} in %toplevel_module
+      : (!transform.any_op) -> !transform.any_op
+    transform.apply_conversion_patterns to %func {
+      transform.apply_conversion_patterns.func.func_to_llvm
+      transform.apply_patterns.memref.expand_strided_metadata
+    } with type_converter {
+      transform.apply_conversion_patterns.memref.memref_to_llvm_type_converter
+        {index_bitwidth = 32, use_opaque_pointers = true}
+    } {
+      legal_dialects = ["llvm"],
+      partial_conversion
+    } : !transform.any_op
+    transform.yield
+  }
+}
```


It's a bit annoying that we can't easily include rewrite patterns through `transform.apply_conversion_patterns`, @ftynse is it possible to extend `transform.apply_conversion_patterns` to allow rewrite patterns alongside conversion ones?

https://github.com/llvm/llvm-project/pull/142025