[mlir][spirv] Enable validation of global vars tests #164974
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
According to the SPIR-V spec (3.3.8 Memory Instructions): "Initializer must be an <id> from a constant instruction or a global (module scope) OpVariable instruction." as such it should not be allowed for one `spirv.globalVariable` to initialize another.
Member
|
@llvm/pr-subscribers-mlir-spirv Author: Igor Wodiany (IgWod-IMG) ChangesAccording to the SPIR-V spec (3.3.8 Memory Instructions): "Initializer must be an Full diff: https://github.com/llvm/llvm-project/pull/164974.diff 3 Files Affected:
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
index fe50865bb7c49..650b0e627ad7b 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
@@ -1278,10 +1278,10 @@ LogicalResult spirv::GlobalVariableOp::verify() {
// TODO: Currently only variable initialization with specialization
// constants and other variables is supported. They could be normal
// constants in the module scope as well.
- if (!initOp || !isa<spirv::GlobalVariableOp, spirv::SpecConstantOp,
- spirv::SpecConstantCompositeOp>(initOp)) {
+ if (!initOp ||
+ !isa<spirv::SpecConstantOp, spirv::SpecConstantCompositeOp>(initOp)) {
return emitOpError("initializer must be result of a "
- "spirv.SpecConstant or spirv.GlobalVariable or "
+ "spirv.SpecConstant or "
"spirv.SpecConstantCompositeOp op");
}
}
diff --git a/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir b/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
index 99ad2a8a2e64b..20bb4eace370b 100644
--- a/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
@@ -501,7 +501,7 @@ spirv.module Logical GLSL450 {
// -----
spirv.module Logical GLSL450 {
- // expected-error @+1 {{op initializer must be result of a spirv.SpecConstant or spirv.GlobalVariable or spirv.SpecConstantCompositeOp op}}
+ // expected-error @+1 {{op initializer must be result of a spirv.SpecConstant or spirv.SpecConstantCompositeOp op}}
spirv.GlobalVariable @var0 initializer(@var1) : !spirv.ptr<f32, Private>
}
diff --git a/mlir/test/Target/SPIRV/global-variable.mlir b/mlir/test/Target/SPIRV/global-variable.mlir
index a70ed316c68d3..a425412989a83 100644
--- a/mlir/test/Target/SPIRV/global-variable.mlir
+++ b/mlir/test/Target/SPIRV/global-variable.mlir
@@ -1,11 +1,16 @@
// RUN: mlir-translate -no-implicit-module -test-spirv-roundtrip -split-input-file %s | FileCheck %s
+// 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 %}
+
// CHECK: spirv.GlobalVariable @var0 bind(1, 0) : !spirv.ptr<f32, Input>
// CHECK-NEXT: spirv.GlobalVariable @var1 bind(0, 1) : !spirv.ptr<f32, Output>
// CHECK-NEXT: spirv.GlobalVariable @var2 built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
// CHECK-NEXT: spirv.GlobalVariable @var3 built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
spirv.GlobalVariable @var0 bind(1, 0) : !spirv.ptr<f32, Input>
spirv.GlobalVariable @var1 bind(0, 1) : !spirv.ptr<f32, Output>
spirv.GlobalVariable @var2 {built_in = "GlobalInvocationId"} : !spirv.ptr<vector<3xi32>, Input>
@@ -14,16 +19,7 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
// -----
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
- // CHECK: spirv.GlobalVariable @var1 : !spirv.ptr<f32, Input>
- // CHECK-NEXT: spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Input>
- spirv.GlobalVariable @var1 : !spirv.ptr<f32, Input>
- spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Input>
-}
-
-// -----
-
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage, Int8], []> {
// CHECK: spirv.SpecConstant @sc = 1 : i8
// CHECK-NEXT: spirv.GlobalVariable @var initializer(@sc) : !spirv.ptr<i8, Uniform>
spirv.SpecConstant @sc = 1 : i8
@@ -33,7 +29,7 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
// -----
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage, Int8], []> {
// CHECK: spirv.SpecConstantComposite @scc (@sc0, @sc1, @sc2) : !spirv.array<3 x i8>
// CHECK-NEXT: spirv.GlobalVariable @var initializer(@scc) : !spirv.ptr<!spirv.array<3 x i8>, Uniform>
spirv.SpecConstant @sc0 = 1 : i8
@@ -47,7 +43,7 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
// -----
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
spirv.GlobalVariable @globalInvocationID built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
spirv.func @foo() "None" {
// CHECK: %[[ADDR:.*]] = spirv.mlir.addressof @globalInvocationID : !spirv.ptr<vector<3xi32>, Input>
|
Member
|
@llvm/pr-subscribers-mlir Author: Igor Wodiany (IgWod-IMG) ChangesAccording to the SPIR-V spec (3.3.8 Memory Instructions): "Initializer must be an Full diff: https://github.com/llvm/llvm-project/pull/164974.diff 3 Files Affected:
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
index fe50865bb7c49..650b0e627ad7b 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
@@ -1278,10 +1278,10 @@ LogicalResult spirv::GlobalVariableOp::verify() {
// TODO: Currently only variable initialization with specialization
// constants and other variables is supported. They could be normal
// constants in the module scope as well.
- if (!initOp || !isa<spirv::GlobalVariableOp, spirv::SpecConstantOp,
- spirv::SpecConstantCompositeOp>(initOp)) {
+ if (!initOp ||
+ !isa<spirv::SpecConstantOp, spirv::SpecConstantCompositeOp>(initOp)) {
return emitOpError("initializer must be result of a "
- "spirv.SpecConstant or spirv.GlobalVariable or "
+ "spirv.SpecConstant or "
"spirv.SpecConstantCompositeOp op");
}
}
diff --git a/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir b/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
index 99ad2a8a2e64b..20bb4eace370b 100644
--- a/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
@@ -501,7 +501,7 @@ spirv.module Logical GLSL450 {
// -----
spirv.module Logical GLSL450 {
- // expected-error @+1 {{op initializer must be result of a spirv.SpecConstant or spirv.GlobalVariable or spirv.SpecConstantCompositeOp op}}
+ // expected-error @+1 {{op initializer must be result of a spirv.SpecConstant or spirv.SpecConstantCompositeOp op}}
spirv.GlobalVariable @var0 initializer(@var1) : !spirv.ptr<f32, Private>
}
diff --git a/mlir/test/Target/SPIRV/global-variable.mlir b/mlir/test/Target/SPIRV/global-variable.mlir
index a70ed316c68d3..a425412989a83 100644
--- a/mlir/test/Target/SPIRV/global-variable.mlir
+++ b/mlir/test/Target/SPIRV/global-variable.mlir
@@ -1,11 +1,16 @@
// RUN: mlir-translate -no-implicit-module -test-spirv-roundtrip -split-input-file %s | FileCheck %s
+// 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 %}
+
// CHECK: spirv.GlobalVariable @var0 bind(1, 0) : !spirv.ptr<f32, Input>
// CHECK-NEXT: spirv.GlobalVariable @var1 bind(0, 1) : !spirv.ptr<f32, Output>
// CHECK-NEXT: spirv.GlobalVariable @var2 built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
// CHECK-NEXT: spirv.GlobalVariable @var3 built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
spirv.GlobalVariable @var0 bind(1, 0) : !spirv.ptr<f32, Input>
spirv.GlobalVariable @var1 bind(0, 1) : !spirv.ptr<f32, Output>
spirv.GlobalVariable @var2 {built_in = "GlobalInvocationId"} : !spirv.ptr<vector<3xi32>, Input>
@@ -14,16 +19,7 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
// -----
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
- // CHECK: spirv.GlobalVariable @var1 : !spirv.ptr<f32, Input>
- // CHECK-NEXT: spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Input>
- spirv.GlobalVariable @var1 : !spirv.ptr<f32, Input>
- spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Input>
-}
-
-// -----
-
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage, Int8], []> {
// CHECK: spirv.SpecConstant @sc = 1 : i8
// CHECK-NEXT: spirv.GlobalVariable @var initializer(@sc) : !spirv.ptr<i8, Uniform>
spirv.SpecConstant @sc = 1 : i8
@@ -33,7 +29,7 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
// -----
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage, Int8], []> {
// CHECK: spirv.SpecConstantComposite @scc (@sc0, @sc1, @sc2) : !spirv.array<3 x i8>
// CHECK-NEXT: spirv.GlobalVariable @var initializer(@scc) : !spirv.ptr<!spirv.array<3 x i8>, Uniform>
spirv.SpecConstant @sc0 = 1 : i8
@@ -47,7 +43,7 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
// -----
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
spirv.GlobalVariable @globalInvocationID built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
spirv.func @foo() "None" {
// CHECK: %[[ADDR:.*]] = spirv.mlir.addressof @globalInvocationID : !spirv.ptr<vector<3xi32>, Input>
|
kuhar
reviewed
Oct 24, 2025
Member
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
or a global (module scope) OpVariable instruction
I parsed this as global variables should be allowed as initializers? Does spirv-val complain?
Contributor
Author
|
You're right, I confused myself there. I now realised what was the actual reason for the So, in the test --- I changed spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
spirv.GlobalVariable @var1 : !spirv.ptr<f32, Uniform>
spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Uniform>
}The problem is that we try to initialize So, I guess in the hindsight this patch is mostly correct, as currently we cannot support initializing global variable with global variable if they are required to be pointers. It just needs updated comments and description then. Hope that makes sense. In the future we may want to re-work global variables if we want to support that. |
kuhar
approved these changes
Oct 24, 2025
dvbuka
pushed a commit
to dvbuka/llvm-project
that referenced
this pull request
Oct 27, 2025
Currently the target test will fail with:
```
error: line 12: Initializer type must match the data type
%var2 = OpVariable %_ptr_Uniform_float Uniform %var1
```
When passed:
```mlir
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
spirv.GlobalVariable @var1 : !spirv.ptr<f32, Uniform>
spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Uniform>
}
```
The problem is that we try to initialize `f32` pointer with `f32`
pointer, but the validator fails because it expects `var1` to be `f32`,
not a pointer to `f32`. `spirv.GlobalVariable` only allows pointer type,
so in the current design we cannot initialize one `spirv.GlobalVariable`
with another.
So, for now we disallow initialization of one global variable with
another. In the future we may want to re-work global variables if we
want to support that.
Lukacma
pushed a commit
to Lukacma/llvm-project
that referenced
this pull request
Oct 29, 2025
Currently the target test will fail with:
```
error: line 12: Initializer type must match the data type
%var2 = OpVariable %_ptr_Uniform_float Uniform %var1
```
When passed:
```mlir
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
spirv.GlobalVariable @var1 : !spirv.ptr<f32, Uniform>
spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Uniform>
}
```
The problem is that we try to initialize `f32` pointer with `f32`
pointer, but the validator fails because it expects `var1` to be `f32`,
not a pointer to `f32`. `spirv.GlobalVariable` only allows pointer type,
so in the current design we cannot initialize one `spirv.GlobalVariable`
with another.
So, for now we disallow initialization of one global variable with
another. In the future we may want to re-work global variables if we
want to support that.
aokblast
pushed a commit
to aokblast/llvm-project
that referenced
this pull request
Oct 30, 2025
Currently the target test will fail with:
```
error: line 12: Initializer type must match the data type
%var2 = OpVariable %_ptr_Uniform_float Uniform %var1
```
When passed:
```mlir
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
spirv.GlobalVariable @var1 : !spirv.ptr<f32, Uniform>
spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Uniform>
}
```
The problem is that we try to initialize `f32` pointer with `f32`
pointer, but the validator fails because it expects `var1` to be `f32`,
not a pointer to `f32`. `spirv.GlobalVariable` only allows pointer type,
so in the current design we cannot initialize one `spirv.GlobalVariable`
with another.
So, for now we disallow initialization of one global variable with
another. In the future we may want to re-work global variables if we
want to support that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the target test will fail with:
When passed:
The problem is that we try to initialize
f32pointer withf32pointer, but the validator fails because it expectsvar1to bef32, not a pointer tof32.spirv.GlobalVariableonly allows pointer type, so in the current design we cannot initialize onespirv.GlobalVariablewith another.So, for now we disallow initialization of one global variable with another. In the future we may want to re-work global variables if we want to support that.