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

[llvmlistbot at llvm.org]

Author: Justin Fargnoli
Date: 2023-10-22T15:34:39-07:00
New Revision: 6833a3808f7ba14247f3ce13d68df0bca991f354

URL: https://github.com/llvm/llvm-project/commit/6833a3808f7ba14247f3ce13d68df0bca991f354
DIFF: https://github.com/llvm/llvm-project/commit/6833a3808f7ba14247f3ce13d68df0bca991f354.diff

LOG: [mlir][DeadCodeAnalysis] Don't Require `RegionBranchTerminatorOpInterface` in `visitRegionTerminator()` (#69043)

Fix for a crash reported in #64975. 

The crash occurs in the cast located
[here](https://github.com/llvm/llvm-project/blob/ece5dd101c7e4dc2fd23428abd312f75fd3d3eaf/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp#L262)
because `llvm.unreachable` doesn't implement
`RegionBranchTerminatorOpInterface`.

The crash is caused by `DeadCodeAnalysis` assuming that
`isa<RegionBranchOpInterface>(op->getParentOp())` implies
`isa<RegionBranchTerminatorOpInterface>(op)` in
`DeadCodeAnalysis::visit()`.

This patch tried to fix this by enabling the analysis to proceed
regardless of whether `op` is a `RegionBranchTerminatorOpInterface`.

Added: 
    

Modified: 
    mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
    mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
    mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h b/mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
index 7a6fea8326a58b8..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(RegionBranchTerminatorOpInterface 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 bbe139fd4fb69fe..7a39e3ae34c1e65 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.

diff  --git a/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir b/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
index b1af670440c2d97..70becc72beb96ae 100644
--- a/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
+++ b/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
@@ -258,3 +258,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
+}