[Mlir-commits] [mlir] [mlir][spirv] Remove nonlocal call verification. (PR #159399)
Erick Ochoa Lopez
llvmlistbot at llvm.org
Wed Sep 17 10:00:54 PDT 2025
https://github.com/amd-eochoalo updated https://github.com/llvm/llvm-project/pull/159399
>From aed3836d1e30fc59c2da968541a4c58bbfcaed68 Mon Sep 17 00:00:00 2001
From: Erick Ochoa <erick.ochoalopez at amd.com>
Date: Wed, 17 Sep 2025 12:47:33 -0400
Subject: [PATCH 1/3] [mlir][spirv] Remove nonlocal call verification.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
`spirv.FunctionCall`'s verifier was being too aggressive.
It included verification of non-local properties by
looking at the callee's definition.
This caused problems in cases where callee had verification
errors and could lead to null pointer dereferencing.
According to MLIR's developers guide
TLDR: only verify local aspects of an operation,
in particular don’t follow def-use chains
(don’t look at the producer of any operand or the user
of any results).
https://mlir.llvm.org/getting_started/DeveloperGuide/#ir-verifier
---
mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp | 30 --------------------
1 file changed, 30 deletions(-)
diff --git a/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp b/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
index 890406df74e72..95d63ddee6824 100644
--- a/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/ControlFlowOps.cpp
@@ -160,42 +160,12 @@ LogicalResult FunctionCallOp::verify() {
<< fnName.getValue() << "' not found in nearest symbol table";
}
- auto functionType = funcOp.getFunctionType();
-
if (getNumResults() > 1) {
return emitOpError(
"expected callee function to have 0 or 1 result, but provided ")
<< getNumResults();
}
- if (functionType.getNumInputs() != getNumOperands()) {
- return emitOpError("has incorrect number of operands for callee: expected ")
- << functionType.getNumInputs() << ", but provided "
- << getNumOperands();
- }
-
- for (uint32_t i = 0, e = functionType.getNumInputs(); i != e; ++i) {
- if (getOperand(i).getType() != functionType.getInput(i)) {
- return emitOpError("operand type mismatch: expected operand type ")
- << functionType.getInput(i) << ", but provided "
- << getOperand(i).getType() << " for operand number " << i;
- }
- }
-
- if (functionType.getNumResults() != getNumResults()) {
- return emitOpError(
- "has incorrect number of results has for callee: expected ")
- << functionType.getNumResults() << ", but provided "
- << getNumResults();
- }
-
- if (getNumResults() &&
- (getResult(0).getType() != functionType.getResult(0))) {
- return emitOpError("result type mismatch: expected ")
- << functionType.getResult(0) << ", but provided "
- << getResult(0).getType();
- }
-
return success();
}
>From bc34970cf59fe36b3e311b3f12985c62d75b5f03 Mon Sep 17 00:00:00 2001
From: Erick Ochoa <erick.ochoalopez at amd.com>
Date: Wed, 17 Sep 2025 12:56:57 -0400
Subject: [PATCH 2/3] Update tests
---
.../Dialect/SPIRV/IR/control-flow-ops.mlir | 42 -------------------
1 file changed, 42 deletions(-)
diff --git a/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir b/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
index 8ec0bf5bbaacf..f29f673b3dd3a 100644
--- a/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
@@ -210,48 +210,6 @@ spirv.module Logical GLSL450 {
// -----
-spirv.module Logical GLSL450 {
- spirv.func @f_result_type_mismatch(%arg0 : i32, %arg1 : i32) -> () "None" {
- // expected-error @+1 {{has incorrect number of results has for callee: expected 0, but provided 1}}
- %1 = spirv.FunctionCall @f_result_type_mismatch(%arg0, %arg0) : (i32, i32) -> (i32)
- spirv.Return
- }
-}
-
-// -----
-
-spirv.module Logical GLSL450 {
- spirv.func @f_type_mismatch(%arg0 : i32, %arg1 : i32) -> () "None" {
- // expected-error @+1 {{has incorrect number of operands for callee: expected 2, but provided 1}}
- spirv.FunctionCall @f_type_mismatch(%arg0) : (i32) -> ()
- spirv.Return
- }
-}
-
-// -----
-
-spirv.module Logical GLSL450 {
- spirv.func @f_type_mismatch(%arg0 : i32, %arg1 : i32) -> () "None" {
- %0 = spirv.Constant 2.0 : f32
- // expected-error @+1 {{operand type mismatch: expected operand type 'i32', but provided 'f32' for operand number 1}}
- spirv.FunctionCall @f_type_mismatch(%arg0, %0) : (i32, f32) -> ()
- spirv.Return
- }
-}
-
-// -----
-
-spirv.module Logical GLSL450 {
- spirv.func @f_type_mismatch(%arg0 : i32, %arg1 : i32) -> i32 "None" {
- %cst = spirv.Constant 0: i32
- // expected-error @+1 {{result type mismatch: expected 'i32', but provided 'f32'}}
- %0 = spirv.FunctionCall @f_type_mismatch(%arg0, %arg0) : (i32, i32) -> f32
- spirv.ReturnValue %cst: i32
- }
-}
-
-// -----
-
spirv.module Logical GLSL450 {
spirv.func @f_foo(%arg0 : i32, %arg1 : i32) -> i32 "None" {
// expected-error @+1 {{op callee function 'f_undefined' not found in nearest symbol table}}
>From f0917d98592d4959e77cc5a912f20cef139c0f63 Mon Sep 17 00:00:00 2001
From: Erick Ochoa <erick.ochoalopez at amd.com>
Date: Wed, 17 Sep 2025 13:00:38 -0400
Subject: [PATCH 3/3] Add test case from issue
---
.../Dialect/SPIRV/IR/control-flow-ops.mlir | 29 +++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir b/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
index f29f673b3dd3a..bfa33559fff61 100644
--- a/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
@@ -220,6 +220,35 @@ spirv.module Logical GLSL450 {
// -----
+"builtin.module"() ({
+ "spirv.module"() <{
+ addressing_model = #spirv.addressing_model<Logical>,
+ memory_model = #spirv.memory_model<GLSL450>
+ }> ({
+ "spirv.func"() <{
+ function_control = #spirv.function_control<None>,
+ function_type = (f32) -> f32,
+ sym_name = "bar"
+ }> ({
+ ^bb0(%arg0: f32):
+ %0 = "spirv.FunctionCall"(%arg0) <{callee = @foo}> : (f32) -> f32
+ "spirv.ReturnValue"(%0) : (f32) -> ()
+ }) : () -> ()
+ // expected-error @+1 {{requires attribute 'function_type'}}
+ "spirv.func"() <{
+ function_control = #spirv.function_control<None>,
+ message = "2nd parent",
+ sym_name = "foo"
+ // This is invalid MLIR because function_type is missing from spirv.func
+ }> ({
+ ^bb0(%arg0: f32):
+ "spirv.ReturnValue"(%arg0) : (f32) -> ()
+ }) : () -> ()
+ }) : () -> ()
+}) : () -> ()
+
+// -----
+
//===----------------------------------------------------------------------===//
// spirv.mlir.loop
//===----------------------------------------------------------------------===//
More information about the Mlir-commits
mailing list