Skip to content

[mlir][transforms] Skip RemoveDeadValues for function declaration #108221

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

Merged
merged 1 commit into from
Sep 14, 2024

Conversation

CoTinker
Copy link
Contributor

This patch skips RemoveDeadValues if funcOp is declaration, which fixes a crash.
Fixes #107546.

@CoTinker CoTinker requested a review from srishti-pm September 11, 2024 13:32
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Sep 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 11, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Longsheng Mou (CoTinker)

Changes

This patch skips RemoveDeadValues if funcOp is declaration, which fixes a crash.
Fixes #107546.


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

2 Files Affected:

  • (modified) mlir/lib/Transforms/RemoveDeadValues.cpp (+2-2)
  • (modified) mlir/test/Transforms/remove-dead-values.mlir (+5)
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 055256903a1522..1cc0096805616c 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -191,10 +191,10 @@ static void cleanSimpleOp(Operation *op, RunLivenessAnalysis &la) {
 ///   non-live across all callers),
 ///   (5) Dropping the uses of these return values from its callers, AND
 ///   (6) Erasing these return values
-/// iff it is not public.
+/// iff it is not public or declaration.
 static void cleanFuncOp(FunctionOpInterface funcOp, Operation *module,
                         RunLivenessAnalysis &la) {
-  if (funcOp.isPublic())
+  if (funcOp.isPublic() || funcOp.isDeclaration())
     return;
 
   // Get the list of unnecessary (non-live) arguments in `nonLiveArgs`.
diff --git a/mlir/test/Transforms/remove-dead-values.mlir b/mlir/test/Transforms/remove-dead-values.mlir
index 69426fdb620832..41027a6a6632eb 100644
--- a/mlir/test/Transforms/remove-dead-values.mlir
+++ b/mlir/test/Transforms/remove-dead-values.mlir
@@ -357,3 +357,8 @@ func.func @kernel(%arg0: memref<18xf32>) {
 // CHECK: gpu.launch blocks
 // CHECK: memref.store
 // CHECK-NEXT: gpu.terminator
+
+// -----
+
+// CHECK: func.func private @no_block_func_declaration() -> ()
+func.func private @no_block_func_declaration() -> ()

This patch skips `RemoveDeadValues` if funcOp is declaration, which
fixes a crash.
@CoTinker
Copy link
Contributor Author

Ping~

@CoTinker
Copy link
Contributor Author

Thanks.

@CoTinker CoTinker merged commit 1208699 into llvm:main Sep 14, 2024
8 checks passed
@CoTinker CoTinker deleted the declaration branch September 14, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
3 participants