[Mlir-commits] [mlir] Reland "[mlir][Affine] Handle null parent op in getAffineParallelInductionVarOwner" (PR #142785)
Han-Chung Wang
llvmlistbot at llvm.org
Wed Jun 4 08:13:45 PDT 2025
https://github.com/hanhanW created https://github.com/llvm/llvm-project/pull/142785
Below is the original commit description. Furthermore, it applies a [fix](https://github.com/llvm/llvm-project/commit/33a26b9ca2f7b690f545fa309e43b38068699db5) for CMakeList.txt
The issue occurs during a downstream pass which does dialect conversion, where both [`FuncOpConversion`](https://github.com/llvm/llvm-project/blob/cde67b6663f994fcb4ded28fd79b23a13d347c4a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp#L480) and [`SubviewFolder`](https://github.com/llvm/llvm-project/blob/cde67b6663f994fcb4ded28fd79b23a13d347c4a/mlir/lib/Dialect/MemRef/Transforms/ExpandStridedMetadata.cpp#L187) are run together. The original starting IR is:
```mlir
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: ?>>
}
}
```
After `FuncOpConversion` runs, the IR looks like:
```mlir
"builtin.module"() ({
"llvm.func"() <{CConv = #llvm.cconv<ccc>, function_type = !llvm.func<struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)> (ptr, ptr, i64, i64, i64, i64, i64, i64, i64, i64, i64)>, linkage = #llvm.linkage<external>, sym_name = "foo", visibility_ = 0 : i64}> ({
^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr, %arg2: i64, %arg3: i64, %arg4: i64, %arg5: i64, %arg6: i64, %arg7: i64, %arg8: i64, %arg9: i64, %arg10: i64):
%0 = "memref.subview"(<<UNKNOWN SSA VALUE>>, <<UNKNOWN SSA VALUE>>, <<UNKNOWN SSA VALUE>>, <<UNKNOWN SSA VALUE>>, <<UNKNOWN SSA VALUE>>) <{operandSegmentSizes = array<i32: 1, 2, 2, 0>, static_offsets = array<i64: -9223372036854775808, -9223372036854775808>, static_sizes = array<i64: -9223372036854775808, -9223372036854775808>, static_strides = array<i64: 1, 1>}> : (memref<100x100xf32>, index, index, index, index) -> memref<?x?xf32, strided<[100, 1], offset: ?>>
"func.return"(%0) : (memref<?x?xf32, strided<[100, 1], offset: ?>>) -> ()
}) : () -> ()
"func.func"() <{function_type = (memref<100x100xf32>, index, index, index, index) -> memref<?x?xf32, strided<[100, 1], offset: ?>>, sym_name = "foo"}> ({
}) : () -> ()
}) {llvm.data_layout = "", llvm.target_triple = ""} : () -> ()
```
The `<<UNKNOWN SSA VALUE>>`'s here are block arguments of a separate unlinked block, which is disconnected from the rest of the IR (so not only is the IR verifier-invalid, it can't even be parsed). This IR is created by signature conversion in the dialect conversion infra.
Now `SubviewFolder` is applied, and the utility function here is called on one of these disconnected block arguments, causing a crash.
The TestMemRefToLLVMWithTransforms pass is introduced to exercise the bug, and it can be reused by other contributors in the future.
Co-authored-by: Rahul Kayaith <rkayaith at gmail.com>
>From 1c2dc77d7efca1083c357e963aed9cb49c0f883a Mon Sep 17 00:00:00 2001
From: hanhanW <hanhan0912 at gmail.com>
Date: Wed, 4 Jun 2025 07:26:46 -0700
Subject: [PATCH 1/2] Reapply "[mlir][Affine] Handle null parent op in
getAffineParallelInductionVarOwner (#142025)"
This reverts commit 15dff71cac9fb85f3332fb5e5809d3d8665ed176.
---
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 2 +-
.../memref-to-llvm-with-transforms.mlir | 10 +++
mlir/test/lib/Conversion/CMakeLists.txt | 1 +
.../Conversion/MemRefToLLVM/CMakeLists.txt | 21 +++++++
.../TestMemRefToLLVMWithTransforms.cpp | 62 +++++++++++++++++++
mlir/tools/mlir-opt/CMakeLists.txt | 1 +
mlir/tools/mlir-opt/mlir-opt.cpp | 2 +
7 files changed, 98 insertions(+), 1 deletion(-)
create mode 100644 mlir/test/Conversion/MemRefToLLVM/memref-to-llvm-with-transforms.mlir
create mode 100644 mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt
create mode 100644 mlir/test/lib/Conversion/MemRefToLLVM/TestMemRefToLLVMWithTransforms.cpp
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 2364f8957992d..8a708eb29210c 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -2667,7 +2667,7 @@ AffineParallelOp mlir::affine::getAffineParallelInductionVarOwner(Value val) {
if (!ivArg || !ivArg.getOwner())
return nullptr;
Operation *containingOp = ivArg.getOwner()->getParentOp();
- auto parallelOp = dyn_cast<AffineParallelOp>(containingOp);
+ auto parallelOp = dyn_cast_if_present<AffineParallelOp>(containingOp);
if (parallelOp && llvm::is_contained(parallelOp.getIVs(), val))
return parallelOp;
return nullptr;
diff --git a/mlir/test/Conversion/MemRefToLLVM/memref-to-llvm-with-transforms.mlir b/mlir/test/Conversion/MemRefToLLVM/memref-to-llvm-with-transforms.mlir
new file mode 100644
index 0000000000000..f6d0524fce39d
--- /dev/null
+++ b/mlir/test/Conversion/MemRefToLLVM/memref-to-llvm-with-transforms.mlir
@@ -0,0 +1,10 @@
+// RUN: mlir-opt -test-memref-to-llvm-with-transforms %s | FileCheck %s
+
+// Checks that the program does not crash. The functionality of the pattern is
+// already checked in test/Dialect/MemRef/*.mlir
+
+func.func @subview_folder(%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: ?>>
+}
+// CHECK-LABEL: llvm.func @subview_folder
diff --git a/mlir/test/lib/Conversion/CMakeLists.txt b/mlir/test/lib/Conversion/CMakeLists.txt
index c09496be729be..167cce225595b 100644
--- a/mlir/test/lib/Conversion/CMakeLists.txt
+++ b/mlir/test/lib/Conversion/CMakeLists.txt
@@ -1,4 +1,5 @@
add_subdirectory(ConvertToSPIRV)
add_subdirectory(FuncToLLVM)
add_subdirectory(MathToVCIX)
+add_subdirectory(MemRefToLLVM)
add_subdirectory(VectorToSPIRV)
diff --git a/mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt b/mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt
new file mode 100644
index 0000000000000..4a85fbdc76bb6
--- /dev/null
+++ b/mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt
@@ -0,0 +1,21 @@
+# Exclude tests from libMLIR.so
+add_mlir_library(MLIRTestMemRefToLLVMWithTransforms
+ TestMemRefToLLVMWithTransforms.cpp
+
+ EXCLUDE_FROM_LIBMLIR
+
+ LINK_LIBS PUBLIC
+ MLIRTestDialect
+ )
+mlir_target_link_libraries(MLIRTestFuncToLLVM PUBLIC
+ MLIRLLVMCommonConversion
+ MLIRLLVMDialect
+ MLIRMemRefTransforms
+ MLIRPass
+ )
+
+target_include_directories(MLIRTestFuncToLLVM
+ PRIVATE
+ ${CMAKE_CURRENT_SOURCE_DIR}/../../Dialect/Test
+ ${CMAKE_CURRENT_BINARY_DIR}/../../Dialect/Test
+ )
diff --git a/mlir/test/lib/Conversion/MemRefToLLVM/TestMemRefToLLVMWithTransforms.cpp b/mlir/test/lib/Conversion/MemRefToLLVM/TestMemRefToLLVMWithTransforms.cpp
new file mode 100644
index 0000000000000..af3b6608aea16
--- /dev/null
+++ b/mlir/test/lib/Conversion/MemRefToLLVM/TestMemRefToLLVMWithTransforms.cpp
@@ -0,0 +1,62 @@
+//===- TestMemRefToLLVMWithTransforms.cpp ---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Conversion/FuncToLLVM/ConvertFuncToLLVM.h"
+#include "mlir/Conversion/LLVMCommon/ConversionTarget.h"
+#include "mlir/Conversion/LLVMCommon/LoweringOptions.h"
+#include "mlir/Conversion/LLVMCommon/TypeConverter.h"
+#include "mlir/Conversion/MemRefToLLVM/MemRefToLLVM.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/Dialect/MemRef/Transforms/Transforms.h"
+#include "mlir/IR/PatternMatch.h"
+#include "mlir/Pass/Pass.h"
+
+using namespace mlir;
+
+namespace {
+
+struct TestMemRefToLLVMWithTransforms
+ : public PassWrapper<TestMemRefToLLVMWithTransforms,
+ OperationPass<ModuleOp>> {
+ MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestMemRefToLLVMWithTransforms)
+
+ void getDependentDialects(DialectRegistry ®istry) const final {
+ registry.insert<LLVM::LLVMDialect>();
+ }
+
+ StringRef getArgument() const final {
+ return "test-memref-to-llvm-with-transforms";
+ }
+
+ StringRef getDescription() const final {
+ return "Tests conversion of MemRef dialects + `func.func` to LLVM dialect "
+ "with MemRef transforms.";
+ }
+
+ void runOnOperation() override {
+ MLIRContext *ctx = &getContext();
+ LowerToLLVMOptions options(ctx);
+ LLVMTypeConverter typeConverter(ctx, options);
+ RewritePatternSet patterns(ctx);
+ memref::populateExpandStridedMetadataPatterns(patterns);
+ populateFuncToLLVMConversionPatterns(typeConverter, patterns);
+ LLVMConversionTarget target(getContext());
+ if (failed(applyPartialConversion(getOperation(), target,
+ std::move(patterns))))
+ signalPassFailure();
+ }
+};
+
+} // namespace
+
+namespace mlir::test {
+void registerTestMemRefToLLVMWithTransforms() {
+ PassRegistration<TestMemRefToLLVMWithTransforms>();
+}
+} // namespace mlir::test
diff --git a/mlir/tools/mlir-opt/CMakeLists.txt b/mlir/tools/mlir-opt/CMakeLists.txt
index 34db3051d36a0..26d7597347a8a 100644
--- a/mlir/tools/mlir-opt/CMakeLists.txt
+++ b/mlir/tools/mlir-opt/CMakeLists.txt
@@ -28,6 +28,7 @@ if(MLIR_INCLUDE_TESTS)
MLIRMathTestPasses
MLIRTestMathToVCIX
MLIRMemRefTestPasses
+ MLIRTestMemRefToLLVMWithTransforms
MLIRMeshTest
MLIRNVGPUTestPasses
MLIRSCFTestPasses
diff --git a/mlir/tools/mlir-opt/mlir-opt.cpp b/mlir/tools/mlir-opt/mlir-opt.cpp
index 2e08ae6f37980..6ef9ff8e84545 100644
--- a/mlir/tools/mlir-opt/mlir-opt.cpp
+++ b/mlir/tools/mlir-opt/mlir-opt.cpp
@@ -130,6 +130,7 @@ void registerTestMathToVCIXPass();
void registerTestIrdlTestDialectConversionPass();
void registerTestMemRefDependenceCheck();
void registerTestMemRefStrideCalculation();
+void registerTestMemRefToLLVMWithTransforms();
void registerTestMeshReshardingSpmdizationPass();
void registerTestMeshSimplificationsPass();
void registerTestMultiBuffering();
@@ -275,6 +276,7 @@ void registerTestPasses() {
mlir::test::registerTestMathToVCIXPass();
mlir::test::registerTestMemRefDependenceCheck();
mlir::test::registerTestMemRefStrideCalculation();
+ mlir::test::registerTestMemRefToLLVMWithTransforms();
mlir::test::registerTestMeshReshardingSpmdizationPass();
mlir::test::registerTestMeshSimplificationsPass();
mlir::test::registerTestMultiBuffering();
>From 33a26b9ca2f7b690f545fa309e43b38068699db5 Mon Sep 17 00:00:00 2001
From: hanhanW <hanhan0912 at gmail.com>
Date: Wed, 4 Jun 2025 08:11:20 -0700
Subject: [PATCH 2/2] Fix CMakeLists.txt
Signed-off-by: hanhanW <hanhan0912 at gmail.com>
---
mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt b/mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt
index 4a85fbdc76bb6..580c9ca4a6049 100644
--- a/mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt
+++ b/mlir/test/lib/Conversion/MemRefToLLVM/CMakeLists.txt
@@ -7,14 +7,15 @@ add_mlir_library(MLIRTestMemRefToLLVMWithTransforms
LINK_LIBS PUBLIC
MLIRTestDialect
)
-mlir_target_link_libraries(MLIRTestFuncToLLVM PUBLIC
+mlir_target_link_libraries(MLIRTestMemRefToLLVMWithTransforms PUBLIC
+ MLIRFuncToLLVM
MLIRLLVMCommonConversion
MLIRLLVMDialect
MLIRMemRefTransforms
MLIRPass
)
-target_include_directories(MLIRTestFuncToLLVM
+target_include_directories(MLIRTestMemRefToLLVMWithTransforms
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}/../../Dialect/Test
${CMAKE_CURRENT_BINARY_DIR}/../../Dialect/Test
More information about the Mlir-commits
mailing list