Skip to content

Conversation

@CoTinker
Copy link
Contributor

This PR fixes a crash if entry function has inputs. Fixes #136143.

This PR fixes a crash if entry function has inputs.
@llvmbot
Copy link
Member

llvmbot commented Apr 23, 2025

@llvm/pr-subscribers-mlir

Author: Longsheng Mou (CoTinker)

Changes

This PR fixes a crash if entry function has inputs. Fixes #136143.


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

3 Files Affected:

  • (modified) mlir/lib/ExecutionEngine/JitRunner.cpp (+5-1)
  • (removed) mlir/test/mlir-runner/verify-entry-point-result.mlir (-7)
  • (added) mlir/test/mlir-runner/verify-entry-point.mlir (+17)
diff --git a/mlir/lib/ExecutionEngine/JitRunner.cpp b/mlir/lib/ExecutionEngine/JitRunner.cpp
index cf462ddf6f17c..dc8af4e514dcd 100644
--- a/mlir/lib/ExecutionEngine/JitRunner.cpp
+++ b/mlir/lib/ExecutionEngine/JitRunner.cpp
@@ -222,9 +222,13 @@ static Error compileAndExecuteVoidFunction(
     CompileAndExecuteConfig config, std::unique_ptr<llvm::TargetMachine> tm) {
   auto mainFunction = dyn_cast_or_null<LLVM::LLVMFuncOp>(
       SymbolTable::lookupSymbolIn(module, entryPoint));
-  if (!mainFunction || mainFunction.empty())
+  if (!mainFunction || mainFunction.isExternal())
     return makeStringError("entry point not found");
 
+  if (cast<LLVM::LLVMFunctionType>(mainFunction.getFunctionType())
+          .getNumParams() != 0)
+    return makeStringError("function inputs not supported");
+
   auto resultType = dyn_cast<LLVM::LLVMVoidType>(
       mainFunction.getFunctionType().getReturnType());
   if (!resultType)
diff --git a/mlir/test/mlir-runner/verify-entry-point-result.mlir b/mlir/test/mlir-runner/verify-entry-point-result.mlir
deleted file mode 100644
index ad46e0b5fe1bf..0000000000000
--- a/mlir/test/mlir-runner/verify-entry-point-result.mlir
+++ /dev/null
@@ -1,7 +0,0 @@
-// RUN: not mlir-runner %s -e entry -entry-point-result=void 2>&1 | FileCheck %s
-
-// CHECK: Error: expected void function
-llvm.func @entry() -> (i32) {
-  %0 = llvm.mlir.constant(0 : index) : i32
-  llvm.return %0 : i32
-}
diff --git a/mlir/test/mlir-runner/verify-entry-point.mlir b/mlir/test/mlir-runner/verify-entry-point.mlir
new file mode 100644
index 0000000000000..9d166d1be7494
--- /dev/null
+++ b/mlir/test/mlir-runner/verify-entry-point.mlir
@@ -0,0 +1,17 @@
+// RUN: not mlir-runner %s -e entry_point -entry-point-result=void 2>&1 | FileCheck %s --check-prefix=CHECK-ENTRY-POINT
+// RUN: not mlir-runner %s -e entry_inputs -entry-point-result=void 2>&1 | FileCheck %s --check-prefix=CHECK-ENTRY-INPUTS
+// RUN: not mlir-runner %s -e entry_result -entry-point-result=void 2>&1 | FileCheck %s --check-prefix=CHECK-ENTRY-RESULT
+
+// CHECK-ENTRY-POINT: Error: entry point not found
+llvm.func @entry_point() -> ()
+
+// CHECK-ENTRY-INPUTS: Error: function inputs not supported
+llvm.func @entry_inputs(%arg0: i32) {
+  llvm.return
+}
+
+// CHECK-ENTRY-RESULT: Error: expected void function
+llvm.func @entry_result() -> (i32) {
+  %0 = llvm.mlir.constant(0 : index) : i32
+  llvm.return %0 : i32
+}

@llvmbot
Copy link
Member

llvmbot commented Apr 23, 2025

@llvm/pr-subscribers-mlir-execution-engine

Author: Longsheng Mou (CoTinker)

Changes

This PR fixes a crash if entry function has inputs. Fixes #136143.


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

3 Files Affected:

  • (modified) mlir/lib/ExecutionEngine/JitRunner.cpp (+5-1)
  • (removed) mlir/test/mlir-runner/verify-entry-point-result.mlir (-7)
  • (added) mlir/test/mlir-runner/verify-entry-point.mlir (+17)
diff --git a/mlir/lib/ExecutionEngine/JitRunner.cpp b/mlir/lib/ExecutionEngine/JitRunner.cpp
index cf462ddf6f17c..dc8af4e514dcd 100644
--- a/mlir/lib/ExecutionEngine/JitRunner.cpp
+++ b/mlir/lib/ExecutionEngine/JitRunner.cpp
@@ -222,9 +222,13 @@ static Error compileAndExecuteVoidFunction(
     CompileAndExecuteConfig config, std::unique_ptr<llvm::TargetMachine> tm) {
   auto mainFunction = dyn_cast_or_null<LLVM::LLVMFuncOp>(
       SymbolTable::lookupSymbolIn(module, entryPoint));
-  if (!mainFunction || mainFunction.empty())
+  if (!mainFunction || mainFunction.isExternal())
     return makeStringError("entry point not found");
 
+  if (cast<LLVM::LLVMFunctionType>(mainFunction.getFunctionType())
+          .getNumParams() != 0)
+    return makeStringError("function inputs not supported");
+
   auto resultType = dyn_cast<LLVM::LLVMVoidType>(
       mainFunction.getFunctionType().getReturnType());
   if (!resultType)
diff --git a/mlir/test/mlir-runner/verify-entry-point-result.mlir b/mlir/test/mlir-runner/verify-entry-point-result.mlir
deleted file mode 100644
index ad46e0b5fe1bf..0000000000000
--- a/mlir/test/mlir-runner/verify-entry-point-result.mlir
+++ /dev/null
@@ -1,7 +0,0 @@
-// RUN: not mlir-runner %s -e entry -entry-point-result=void 2>&1 | FileCheck %s
-
-// CHECK: Error: expected void function
-llvm.func @entry() -> (i32) {
-  %0 = llvm.mlir.constant(0 : index) : i32
-  llvm.return %0 : i32
-}
diff --git a/mlir/test/mlir-runner/verify-entry-point.mlir b/mlir/test/mlir-runner/verify-entry-point.mlir
new file mode 100644
index 0000000000000..9d166d1be7494
--- /dev/null
+++ b/mlir/test/mlir-runner/verify-entry-point.mlir
@@ -0,0 +1,17 @@
+// RUN: not mlir-runner %s -e entry_point -entry-point-result=void 2>&1 | FileCheck %s --check-prefix=CHECK-ENTRY-POINT
+// RUN: not mlir-runner %s -e entry_inputs -entry-point-result=void 2>&1 | FileCheck %s --check-prefix=CHECK-ENTRY-INPUTS
+// RUN: not mlir-runner %s -e entry_result -entry-point-result=void 2>&1 | FileCheck %s --check-prefix=CHECK-ENTRY-RESULT
+
+// CHECK-ENTRY-POINT: Error: entry point not found
+llvm.func @entry_point() -> ()
+
+// CHECK-ENTRY-INPUTS: Error: function inputs not supported
+llvm.func @entry_inputs(%arg0: i32) {
+  llvm.return
+}
+
+// CHECK-ENTRY-RESULT: Error: expected void function
+llvm.func @entry_result() -> (i32) {
+  %0 = llvm.mlir.constant(0 : index) : i32
+  llvm.return %0 : i32
+}

@CoTinker
Copy link
Contributor Author

Ping~

@joker-eph joker-eph changed the title [mlir-runner] Check entry function not has inputs [mlir-runner] Check entry function does not expect arguments Apr 28, 2025
Copy link
Member

@jpienaar jpienaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG with Mehdi's comment addressed.

@CoTinker CoTinker requested a review from joker-eph May 14, 2025 01:59
@CoTinker CoTinker merged commit 34be80a into llvm:main May 15, 2025
11 checks passed
@CoTinker CoTinker deleted the mlir_runner branch May 15, 2025 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MLIR] unused llvm.mlir.poison crashes at runtime (JitRunner crashed with segmentation fault)

5 participants