[Mlir-commits] [mlir] [mlir][DeadCodeAnalysis] Don't Require `RegionBranchTerminatorOpInterface` in `visitRegionTerminator()` (PR #69043)

[Justin Fargnoli]

https://github.com/justinfargnoli updated https://github.com/llvm/llvm-project/pull/69043

>From 8f8b644792d5e26f76d98bd8ba45440443f9a260 Mon Sep 17 00:00:00 2001
From: justinfargnoli <justinfargnoli at gmail.com>
Date: Fri, 13 Oct 2023 18:16:45 -0700
Subject: [PATCH 1/4] Initial commit

---
 .../mlir/Analysis/DataFlow/DeadCodeAnalysis.h        |  2 +-
 mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp      | 12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h b/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
index 7a6fea8326a58b8..ce262c3ac5291ff 100644
--- a/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
+++ b/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
@@ -208,7 +208,7 @@ class DeadCodeAnalysis : public DataFlowAnalysis {
   /// Visit the given terminator operation that exits a region under an
   /// operation with control-flow semantics. These are terminators with no CFG
   /// successors.
-  void visitRegionTerminator(RegionBranchTerminatorOpInterface op,
+  void visitRegionTerminator(Operation *op,
                              RegionBranchOpInterface branch);
 
   /// Visit the given terminator operation that exits a callable region. These
diff --git a/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp b/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
index d423d37b9770c69..30a862fa048f97a 100644
--- a/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
+++ b/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
@@ -259,8 +259,7 @@ LogicalResult DeadCodeAnalysis::visit(ProgramPoint point) {
   if (isRegionOrCallableReturn(op)) {
     if (auto branch = dyn_cast<RegionBranchOpInterface>(op->getParentOp())) {
       // Visit the exiting terminator of a region.
-      visitRegionTerminator(cast<RegionBranchTerminatorOpInterface>(op),
-                            branch);
+      visitRegionTerminator(op, branch);
     } else if (auto callable =
                    dyn_cast<CallableOpInterface>(op->getParentOp())) {
       // Visit the exiting terminator of a callable.
@@ -379,14 +378,17 @@ void DeadCodeAnalysis::visitRegionBranchOperation(
   }
 }
 
-void DeadCodeAnalysis::visitRegionTerminator(
-    RegionBranchTerminatorOpInterface op, RegionBranchOpInterface branch) {
+void DeadCodeAnalysis::visitRegionTerminator(Operation *op, 
+                                             RegionBranchOpInterface branch) {
   std::optional<SmallVector<Attribute>> operands = getOperandValues(op);
   if (!operands)
     return;
 
   SmallVector<RegionSuccessor> successors;
-  op.getSuccessorRegions(*operands, successors);
+  if (auto terminator = dyn_cast<RegionBranchTerminatorOpInterface>(op))
+    terminator.getSuccessorRegions(*operands, successors);
+  else
+    branch.getSuccessorRegions(op->getParentRegion(), successors);
 
   // Mark successor region entry blocks as executable and add this op to the
   // list of predecessors.

>From ec79bc71013c8433a83948a256b313a0f93cf826 Mon Sep 17 00:00:00 2001
From: justinfargnoli <justinfargnoli at gmail.com>
Date: Fri, 13 Oct 2023 18:34:11 -0700
Subject: [PATCH 2/4] Add test

---
 .../DataFlow/test-dead-code-analysis-regression.mlir         | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 mlir/test/Analysis/DataFlow/test-dead-code-analysis-regression.mlir

diff --git a/mlir/test/Analysis/DataFlow/test-dead-code-analysis-regression.mlir b/mlir/test/Analysis/DataFlow/test-dead-code-analysis-regression.mlir
new file mode 100644
index 000000000000000..70c44970519cc24
--- /dev/null
+++ b/mlir/test/Analysis/DataFlow/test-dead-code-analysis-regression.mlir
@@ -0,0 +1,5 @@
+// RUN: mlir-opt -int-range-optimizations %s 
+
+%0 = scf.execute_region -> tensor<5x16xi16> {
+  llvm.unreachable
+}

>From 56b7099c7648be1fe22268bbeb311c9773a2f894 Mon Sep 17 00:00:00 2001
From: justinfargnoli <justinfargnoli at gmail.com>
Date: Sat, 14 Oct 2023 17:20:15 -0400
Subject: [PATCH 3/4] clang-format changes

---
 mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h | 3 +--
 mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp        | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h b/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
index ce262c3ac5291ff..10ef8b6ba5843a9 100644
--- a/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
+++ b/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
@@ -208,8 +208,7 @@ class DeadCodeAnalysis : public DataFlowAnalysis {
   /// Visit the given terminator operation that exits a region under an
   /// operation with control-flow semantics. These are terminators with no CFG
   /// successors.
-  void visitRegionTerminator(Operation *op,
-                             RegionBranchOpInterface branch);
+  void visitRegionTerminator(Operation *op, RegionBranchOpInterface branch);
 
   /// Visit the given terminator operation that exits a callable region. These
   /// are terminators with no CFG successors.
diff --git a/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp b/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
index 30a862fa048f97a..07f9705912260a4 100644
--- a/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
+++ b/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
@@ -378,7 +378,7 @@ void DeadCodeAnalysis::visitRegionBranchOperation(
   }
 }
 
-void DeadCodeAnalysis::visitRegionTerminator(Operation *op, 
+void DeadCodeAnalysis::visitRegionTerminator(Operation *op,
                                              RegionBranchOpInterface branch) {
   std::optional<SmallVector<Attribute>> operands = getOperandValues(op);
   if (!operands)

>From e17c3558d293f41a68f29911f0f4540cb4d03016 Mon Sep 17 00:00:00 2001
From: justinfargnoli <justinfargnoli at gmail.com>
Date: Sat, 21 Oct 2023 14:34:41 -0700
Subject: [PATCH 4/4] Merge test case

---
 .../DataFlow/test-dead-code-analysis-regression.mlir       | 5 -----
 mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir   | 7 +++++++
 2 files changed, 7 insertions(+), 5 deletions(-)
 delete mode 100644 mlir/test/Analysis/DataFlow/test-dead-code-analysis-regression.mlir

diff --git a/mlir/test/Analysis/DataFlow/test-dead-code-analysis-regression.mlir b/mlir/test/Analysis/DataFlow/test-dead-code-analysis-regression.mlir
deleted file mode 100644
index 70c44970519cc24..000000000000000
--- a/mlir/test/Analysis/DataFlow/test-dead-code-analysis-regression.mlir
+++ /dev/null
@@ -1,5 +0,0 @@
-// RUN: mlir-opt -int-range-optimizations %s 
-
-%0 = scf.execute_region -> tensor<5x16xi16> {
-  llvm.unreachable
-}
diff --git a/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir b/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
index 020c8b2a0e84dc7..4f731060e68c61a 100644
--- a/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
+++ b/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
@@ -246,3 +246,10 @@ func.func @test_call_dead_return(%arg0: i32) -> () {
   %0 = func.call @test_dead_return(%arg0) {tag = "test_dead_return"} : (i32) -> i32
   return
 }
+
+func.func @test_dca_doesnt_crash() -> () {
+  %0 = scf.execute_region -> tensor<5x16xi16> {
+    llvm.unreachable
+  }  
+  return
+}