[Mlir-commits] [mlir] [mlir][Transforms] Fix crash in `CFGToSCF` when conversion fails (PR #173590)

[NohHyeon Kwon]

https://github.com/swote-git updated https://github.com/llvm/llvm-project/pull/173590

>From 5161a360eb71bb688be93f62fa3ccf7d115fceb6 Mon Sep 17 00:00:00 2001
From: swote-git <kst7703 at gmail.com>
Date: Fri, 26 Dec 2025 12:42:00 +0900
Subject: [PATCH 1/2] [mlir][Transforms] Fix crash in `CFGToSCF` when
 conversion fails

When `createStructuredBranchRegionOp` fails (e.g., for unsupported ops
like spirv.BranchConditional), the blocks in `conditionalRegions` are
still referenced as successors by `regionEntry`'s terminator. This
causes an assertion failure "Cannot destroy a value that still has
uses!" when `conditionalRegions` is destroyed.

Fix by calling `dropAllReferences()` on the terminator before returning
failure, which drops the successor block references.

Fixes #173566
---
 mlir/lib/Transforms/Utils/CFGToSCF.cpp        |  4 ++-
 .../cf-to-scf-cycle-cleanup.mlir              | 27 +++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 mlir/test/Conversion/ControlFlowToSCF/cf-to-scf-cycle-cleanup.mlir

diff --git a/mlir/lib/Transforms/Utils/CFGToSCF.cpp b/mlir/lib/Transforms/Utils/CFGToSCF.cpp
index dbde75e17b3b8..38df3bb85ffd8 100644
--- a/mlir/lib/Transforms/Utils/CFGToSCF.cpp
+++ b/mlir/lib/Transforms/Utils/CFGToSCF.cpp
@@ -1156,8 +1156,10 @@ static FailureOr<SmallVector<Block *>> transformToStructuredCFBranches(
     FailureOr<Operation *> result = interface.createStructuredBranchRegionOp(
         opBuilder, regionEntry->getTerminator(),
         continuation->getArgumentTypes(), conditionalRegions);
-    if (failed(result))
+    if (failed(result)) {
+      regionEntry->getTerminator()->dropAllReferences();
       return failure();
+    }
     structuredCondOp = *result;
     regionEntry->getTerminator()->erase();
   }
diff --git a/mlir/test/Conversion/ControlFlowToSCF/cf-to-scf-cycle-cleanup.mlir b/mlir/test/Conversion/ControlFlowToSCF/cf-to-scf-cycle-cleanup.mlir
new file mode 100644
index 0000000000000..4cfa19e355011
--- /dev/null
+++ b/mlir/test/Conversion/ControlFlowToSCF/cf-to-scf-cycle-cleanup.mlir
@@ -0,0 +1,27 @@
+// RUN: not mlir-opt %s -lift-cf-to-scf 2>&1 | FileCheck %s
+
+// This test verifies that the pass does not crash when encountering unsupported
+// control flow operations within a cycle (e.g., SPIR-V loops). (issue #173566)
+// It ensures that temporary regions are cleaned up correctly upon failure.
+
+// CHECK: Cannot convert unknown control flow op to structured control flow
+module {
+  func.func @spirv_loop_crash_repro(%arg0: index) {
+    %0 = builtin.unrealized_conversion_cast %arg0 : index to i32
+    %cst8 = spirv.Constant 8 : i32
+    
+    // spirv.mlir.loop creates a CFG cycle. 
+    spirv.mlir.loop {
+      spirv.Branch ^bb1(%0 : i32)
+    ^bb1(%2: i32):
+      %3 = spirv.SLessThan %2, %cst8 : i32
+      spirv.BranchConditional %3, ^bb2, ^bb3
+    ^bb2:
+      %4 = spirv.IAdd %2, %0 : i32
+      spirv.Branch ^bb1(%4 : i32)
+    ^bb3:
+      spirv.mlir.merge
+    }
+    spirv.Return
+  }
+}

>From 62d6639c44b0ce3504129c1da8743ef8e5a6081e Mon Sep 17 00:00:00 2001
From: swote-git <kst7703 at gmail.com>
Date: Fri, 26 Dec 2025 20:59:35 +0900
Subject: [PATCH 2/2] fix: Move blocks back to parent region when  fails. Note:
 This patch was implemented with the assistance of AI tools

---
 mlir/lib/Transforms/Utils/CFGToSCF.cpp                |  4 +++-
 ...ycle-cleanup.mlir => unsupported-op-in-cycle.mlir} | 11 ++++-------
 2 files changed, 7 insertions(+), 8 deletions(-)
 rename mlir/test/Conversion/ControlFlowToSCF/{cf-to-scf-cycle-cleanup.mlir => unsupported-op-in-cycle.mlir} (52%)

diff --git a/mlir/lib/Transforms/Utils/CFGToSCF.cpp b/mlir/lib/Transforms/Utils/CFGToSCF.cpp
index 38df3bb85ffd8..1a3d12661e7e7 100644
--- a/mlir/lib/Transforms/Utils/CFGToSCF.cpp
+++ b/mlir/lib/Transforms/Utils/CFGToSCF.cpp
@@ -1157,7 +1157,9 @@ static FailureOr<SmallVector<Block *>> transformToStructuredCFBranches(
         opBuilder, regionEntry->getTerminator(),
         continuation->getArgumentTypes(), conditionalRegions);
     if (failed(result)) {
-      regionEntry->getTerminator()->dropAllReferences();
+      Region &parentRegion = *regionEntry->getParent();
+      for (Region &reg : conditionalRegions)
+        parentRegion.getBlocks().splice(parentRegion.end(), reg.getBlocks());
       return failure();
     }
     structuredCondOp = *result;
diff --git a/mlir/test/Conversion/ControlFlowToSCF/cf-to-scf-cycle-cleanup.mlir b/mlir/test/Conversion/ControlFlowToSCF/unsupported-op-in-cycle.mlir
similarity index 52%
rename from mlir/test/Conversion/ControlFlowToSCF/cf-to-scf-cycle-cleanup.mlir
rename to mlir/test/Conversion/ControlFlowToSCF/unsupported-op-in-cycle.mlir
index 4cfa19e355011..58bffdb569328 100644
--- a/mlir/test/Conversion/ControlFlowToSCF/cf-to-scf-cycle-cleanup.mlir
+++ b/mlir/test/Conversion/ControlFlowToSCF/unsupported-op-in-cycle.mlir
@@ -1,20 +1,17 @@
-// RUN: not mlir-opt %s -lift-cf-to-scf 2>&1 | FileCheck %s
+// RUN: mlir-opt %s -lift-cf-to-scf -verify-diagnostics
 
-// This test verifies that the pass does not crash when encountering unsupported
-// control flow operations within a cycle (e.g., SPIR-V loops). (issue #173566)
-// It ensures that temporary regions are cleaned up correctly upon failure.
+// verify faliure for unsupported ops in cycles (issue #173566)
 
-// CHECK: Cannot convert unknown control flow op to structured control flow
 module {
   func.func @spirv_loop_crash_repro(%arg0: index) {
     %0 = builtin.unrealized_conversion_cast %arg0 : index to i32
     %cst8 = spirv.Constant 8 : i32
     
-    // spirv.mlir.loop creates a CFG cycle. 
     spirv.mlir.loop {
       spirv.Branch ^bb1(%0 : i32)
     ^bb1(%2: i32):
       %3 = spirv.SLessThan %2, %cst8 : i32
+      // expected-error @+1 {{Cannot convert unknown control flow op to structured control flow}}
       spirv.BranchConditional %3, ^bb2, ^bb3
     ^bb2:
       %4 = spirv.IAdd %2, %0 : i32
@@ -24,4 +21,4 @@ module {
     }
     spirv.Return
   }
-}
+}
\ No newline at end of file