[Mlir-commits] [mlir] [mlir][spirv] Ensure function declarations precede definitions (PR #164956)
Igor Wodiany
llvmlistbot at llvm.org
Fri Oct 24 04:04:51 PDT 2025
https://github.com/IgWod-IMG created https://github.com/llvm/llvm-project/pull/164956
SPIR-V spec requires that any calls to external functions are preceded by declarations of those external functions. To simplify the verification we strengthen the condition so any external declarations must precede any function definitions - this ensures that external functions are always declared before being used. As an alternative we could allow arbitrary ordering in MLIR and sort functions in the serialization, but in this case it feels like aligning MLIR representation with SPIR-V is a cleaner approach.
>From ba85d0d15b706043b7ce4bc624b5a47aac04f552 Mon Sep 17 00:00:00 2001
From: Igor Wodiany <igor.wodiany at imgtec.com>
Date: Thu, 23 Oct 2025 17:34:41 +0100
Subject: [PATCH] [mlir][spirv] Ensure function declarations precede
definitions
SPIR-V spec requires that any calls to external functions are preceded
by declarations of those external functions. To simplify the verification
we strengthen the condition so any external declarations must precede
any function definitions - this ensures that external functions are always
declared before being used. As an alternative we could allow arbitrary ordering
in MLIR and sort functions in the serialization, but in this case it
feels like aligning MLIR representation with SPIR-V is a cleaner approach.
---
mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp | 22 ++++++++++---
.../SPIRV/IR/function-decorations.mlir | 22 ++++++++++++-
.../Target/SPIRV/function-decorations.mlir | 31 +++++++++++--------
3 files changed, 57 insertions(+), 18 deletions(-)
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
index fe50865bb7c49..6c96ef371d7a4 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
@@ -1516,6 +1516,7 @@ LogicalResult spirv::ModuleOp::verifyRegions() {
DenseMap<std::pair<spirv::FuncOp, spirv::ExecutionModel>, spirv::EntryPointOp>
entryPoints;
mlir::SymbolTable table(*this);
+ bool encounteredFuncDefinition = false;
for (auto &op : *getBody()) {
if (op.getDialect() != dialect)
@@ -1561,10 +1562,23 @@ LogicalResult spirv::ModuleOp::verifyRegions() {
auto hasImportLinkage =
linkageAttr && (linkageAttr.value().getLinkageType().getValue() ==
spirv::LinkageType::Import);
- if (funcOp.isExternal() && !hasImportLinkage)
- return op.emitError(
- "'spirv.module' cannot contain external functions "
- "without 'Import' linkage_attributes (LinkageAttributes)");
+ if (funcOp.isExternal()) {
+ if (!hasImportLinkage)
+ return op.emitError(
+ "'spirv.module' cannot contain external functions "
+ "without 'Import' linkage_attributes (LinkageAttributes)");
+ // This is stronger than necessary. It should be sufficient to
+ // ensure any declarations precede their uses and not all definitions,
+ // however this allows to avoid analysing every function in the module
+ // this way.
+ if (encounteredFuncDefinition)
+ return op.emitError("all functions declarations in 'spirv.module' "
+ "must happen before any definitions");
+ }
+
+ // In SPIR-V "declarations" are functions without a body and "definitions"
+ // functions with a body.
+ encounteredFuncDefinition |= !funcOp.getBody().empty();
// TODO: move this check to spirv.func.
for (auto &block : funcOp)
diff --git a/mlir/test/Dialect/SPIRV/IR/function-decorations.mlir b/mlir/test/Dialect/SPIRV/IR/function-decorations.mlir
index f09767a416f6b..a83cbf0fc0919 100644
--- a/mlir/test/Dialect/SPIRV/IR/function-decorations.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/function-decorations.mlir
@@ -1,6 +1,13 @@
// RUN: mlir-opt -split-input-file -verify-diagnostics %s | FileCheck %s
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
+ // CHECK: linkage_attributes = #spirv.linkage_attributes<linkage_name = "outside.func", linkage_type = <Import>>
+ spirv.func @outside.func.with.linkage(%arg0 : i8) -> () "Pure" attributes {
+ linkage_attributes=#spirv.linkage_attributes<
+ linkage_name="outside.func",
+ linkage_type=<Import>
+ >
+ }
spirv.func @linkage_attr_test_kernel() "DontInline" attributes {} {
%uchar_0 = spirv.Constant 0 : i8
%ushort_1 = spirv.Constant 1 : i16
@@ -8,7 +15,20 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
spirv.FunctionCall @outside.func.with.linkage(%uchar_0):(i8) -> ()
spirv.Return
}
- // CHECK: linkage_attributes = #spirv.linkage_attributes<linkage_name = "outside.func", linkage_type = <Import>>
+ spirv.func @inside.func() -> () "Pure" attributes {} {spirv.Return}
+}
+
+// -----
+
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage, Int8, Int16], []> {
+ spirv.func @linkage_attr_test_kernel() "DontInline" attributes {} {
+ %uchar_0 = spirv.Constant 0 : i8
+ %ushort_1 = spirv.Constant 1 : i16
+ %uint_0 = spirv.Constant 0 : i32
+ spirv.FunctionCall @outside.func.with.linkage(%uchar_0):(i8) -> ()
+ spirv.Return
+ }
+ // expected-error at +1 {{all functions declarations in 'spirv.module' must happen before any definitions}}
spirv.func @outside.func.with.linkage(%arg0 : i8) -> () "Pure" attributes {
linkage_attributes=#spirv.linkage_attributes<
linkage_name="outside.func",
diff --git a/mlir/test/Target/SPIRV/function-decorations.mlir b/mlir/test/Target/SPIRV/function-decorations.mlir
index cf6edaa0a3d5b..b6f4d08271fa4 100644
--- a/mlir/test/Target/SPIRV/function-decorations.mlir
+++ b/mlir/test/Target/SPIRV/function-decorations.mlir
@@ -1,13 +1,11 @@
// RUN: mlir-translate --no-implicit-module --test-spirv-roundtrip --split-input-file %s | FileCheck %s
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
- spirv.func @linkage_attr_test_kernel() "DontInline" attributes {} {
- %uchar_0 = spirv.Constant 0 : i8
- %ushort_1 = spirv.Constant 1 : i16
- %uint_0 = spirv.Constant 0 : i32
- spirv.FunctionCall @outside.func.with.linkage(%uchar_0):(i8) -> ()
- spirv.Return
- }
+// RUN: %if spirv-tools %{ rm -rf %t %}
+// RUN: %if spirv-tools %{ mkdir %t %}
+// RUN: %if spirv-tools %{ mlir-translate --no-implicit-module --serialize-spirv --split-input-file --spirv-save-validation-files-with-prefix=%t/module %s %}
+// RUN: %if spirv-tools %{ spirv-val %t %}
+
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage, Int8, Int16], []> {
// CHECK: linkage_attributes = #spirv.linkage_attributes<linkage_name = "outside.func", linkage_type = <Import>>
spirv.func @outside.func.with.linkage(%arg0 : i8) -> () "Pure" attributes {
linkage_attributes=#spirv.linkage_attributes<
@@ -15,13 +13,20 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
linkage_type=<Import>
>
}
+ spirv.func @linkage_attr_test_kernel() "DontInline" attributes {} {
+ %uchar_0 = spirv.Constant 0 : i8
+ %ushort_1 = spirv.Constant 1 : i16
+ %uint_0 = spirv.Constant 0 : i32
+ spirv.FunctionCall @outside.func.with.linkage(%uchar_0):(i8) -> ()
+ spirv.Return
+ }
spirv.func @inside.func() -> () "Pure" attributes {} {spirv.Return}
}
// -----
spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
- [Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
+ [Shader, PhysicalStorageBufferAddresses, Linkage], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @func_arg_decoration_aliased(%{{.*}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Aliased>})
spirv.func @func_arg_decoration_aliased(
%arg0 : !spirv.ptr<i32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased> }
@@ -33,7 +38,7 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
// -----
spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
- [Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
+ [Shader, PhysicalStorageBufferAddresses, Linkage], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @func_arg_decoration_restrict(%{{.*}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Restrict>})
spirv.func @func_arg_decoration_restrict(
%arg0 : !spirv.ptr<i32,PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Restrict> }
@@ -45,7 +50,7 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
// -----
spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
- [Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
+ [Shader, PhysicalStorageBufferAddresses, Linkage, GenericPointer], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @func_arg_decoration_aliased_pointer(%{{.*}}: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> {spirv.decoration = #spirv.decoration<AliasedPointer>})
spirv.func @func_arg_decoration_aliased_pointer(
%arg0 : !spirv.ptr<!spirv.ptr<i32,PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<AliasedPointer> }
@@ -57,7 +62,7 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
// -----
spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
- [Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
+ [Shader, PhysicalStorageBufferAddresses, Linkage, GenericPointer], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @func_arg_decoration_restrict_pointer(%{{.*}}: !spirv.ptr<!spirv.ptr<i32, PhysicalStorageBuffer>, Generic> {spirv.decoration = #spirv.decoration<RestrictPointer>})
spirv.func @func_arg_decoration_restrict_pointer(
%arg0 : !spirv.ptr<!spirv.ptr<i32,PhysicalStorageBuffer>, Generic> { spirv.decoration = #spirv.decoration<RestrictPointer> }
@@ -69,7 +74,7 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
// -----
spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
- [Shader, PhysicalStorageBufferAddresses], [SPV_KHR_physical_storage_buffer]> {
+ [Shader, PhysicalStorageBufferAddresses, Linkage], [SPV_KHR_physical_storage_buffer]> {
// CHECK-LABEL: spirv.func @fn1(%{{.*}}: i32, %{{.*}}: !spirv.ptr<i32, PhysicalStorageBuffer> {spirv.decoration = #spirv.decoration<Aliased>})
spirv.func @fn1(
%arg0: i32,
More information about the Mlir-commits
mailing list