Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir][dataflow] Remove early exit in dead code analysis for zero-operand returns #68151

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

pengmai
Copy link
Contributor

@pengmai pengmai commented Oct 3, 2023

The PredecessorState in dead code analysis does not register zero-operand returns as predecessors of their corresponding call ops. This causes bugs with dense dataflow analyses that use dead code analysis to query for the predecessors of CallOpInterfaces.

This was reasonable for sparse analyses, but isn't for dense ones.

…rand returns

The PredecessorState in dead code analysis does not register zero-operand returns as predecessors of their corresponding call ops. This causes bugs with dense analyses that use dead code analysis to query for the predecessors of CallOpInterfaces.
@llvmbot llvmbot added the mlir label Oct 3, 2023
@wsmoses wsmoses requested review from ftynse and wsmoses October 5, 2023 15:06
@llvmbot
Copy link
Member

llvmbot commented Oct 5, 2023

@llvm/pr-subscribers-mlir

Changes

The PredecessorState in dead code analysis does not register zero-operand returns as predecessors of their corresponding call ops. This causes bugs with dense dataflow analyses that use dead code analysis to query for the predecessors of CallOpInterfaces.


Full diff: https://github.com/llvm/llvm-project/pull/68151.diff

3 Files Affected:

  • (modified) mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp (-4)
  • (modified) mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir (+12)
  • (modified) mlir/test/Analysis/DataFlow/test-last-modified-callgraph.mlir (+17)
diff --git a/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp b/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
index d423d37b9770c69..bbe139fd4fb69fe 100644
--- a/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
+++ b/mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
@@ -407,10 +407,6 @@ void DeadCodeAnalysis::visitRegionTerminator(
 
 void DeadCodeAnalysis::visitCallableTerminator(Operation *op,
                                                CallableOpInterface callable) {
-  // If there are no exiting values, we have nothing to do.
-  if (op->getNumOperands() == 0)
-    return;
-
   // Add as predecessors to all callsites this return op.
   auto *callsites = getOrCreateFor<PredecessorState>(op, callable);
   bool canResolve = op->hasTrait<OpTrait::ReturnLike>();
diff --git a/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir b/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
index 020c8b2a0e84dc7..b1af670440c2d97 100644
--- a/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
+++ b/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
@@ -118,6 +118,18 @@ func.func @test_callgraph(%cond: i1, %arg0: i32) -> i32 {
   return %2 : i32
 }
 
+func.func private @bax(%arg0: i32) {
+  return {void_return}
+}
+
+func.func @test_callgraph_void_return(%arg0: i32) -> i32 {
+  // CHECK: call_void_return:
+  // CHECK: op_preds: (all) predecessors:
+  // CHECK:   func.return {void_return}
+  func.call @bax(%arg0) {tag = "call_void_return"}: (i32) -> ()
+  return %arg0 : i32
+}
+
 // CHECK: test_unknown_branch:
 // CHECK:  region #0
 // CHECK:   ^bb0 = live
diff --git a/mlir/test/Analysis/DataFlow/test-last-modified-callgraph.mlir b/mlir/test/Analysis/DataFlow/test-last-modified-callgraph.mlir
index 4a243571c231a19..709d787bb306bef 100644
--- a/mlir/test/Analysis/DataFlow/test-last-modified-callgraph.mlir
+++ b/mlir/test/Analysis/DataFlow/test-last-modified-callgraph.mlir
@@ -68,6 +68,23 @@ func.func @test_multiple_return_sites(%cond: i1, %a: i32, %ptr: memref<i32>) ->
 
 // -----
 
+// CHECK-LABEL: test_tag: after_call
+// CHECK: operand #0
+// CHECK-NEXT: - write0
+func.func private @void_return(%ptr: memref<i32>) {
+  return
+}
+
+func.func @test_call_void_return() {
+  %ptr = memref.alloc() : memref<i32>
+  %c0 = arith.constant 0 : i32
+  memref.store %c0, %ptr[] {tag_name = "write0"} : memref<i32>
+  func.call @void_return(%ptr) : (memref<i32>) -> ()
+  memref.load %ptr[] {tag = "after_call"} : memref<i32>
+  return
+}
+
+// -----
 
 func.func private @callee(%arg0: memref<f32>) -> memref<f32> {
   %2 = arith.constant 2.0 : f32

@ftynse ftynse merged commit 4732b0c into llvm:main Oct 5, 2023
3 checks passed
@pengmai pengmai deleted the bugfix/dense-dead-code-analysis branch October 5, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants