Skip to content

Commit dcb75a3

Browse files
IgWod-IMGaokblast
authored andcommitted
[mlir][spirv] Enable validation of global vars tests (llvm#164974)
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.
1 parent 35eb515 commit dcb75a3

File tree

3 files changed

+22
-19
lines changed

3 files changed

+22
-19
lines changed

mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,12 +1276,19 @@ LogicalResult spirv::GlobalVariableOp::verify() {
12761276
Operation *initOp = SymbolTable::lookupNearestSymbolFrom(
12771277
(*this)->getParentOp(), init.getAttr());
12781278
// TODO: Currently only variable initialization with specialization
1279-
// constants and other variables is supported. They could be normal
1280-
// constants in the module scope as well.
1281-
if (!initOp || !isa<spirv::GlobalVariableOp, spirv::SpecConstantOp,
1282-
spirv::SpecConstantCompositeOp>(initOp)) {
1279+
// constants is supported. There could be normal constants in the module
1280+
// scope as well.
1281+
//
1282+
// In the current setup we also cannot initialize one global variable with
1283+
// another. The problem is that if we try to initialize pointer of type X
1284+
// with another pointer type, the validator fails because it expects the
1285+
// variable to be initialized to be type X, not pointer to X. Now
1286+
// `spirv.GlobalVariable` only allows pointer type, so in the current design
1287+
// we cannot initialize one `spirv.GlobalVariable` with another.
1288+
if (!initOp ||
1289+
!isa<spirv::SpecConstantOp, spirv::SpecConstantCompositeOp>(initOp)) {
12831290
return emitOpError("initializer must be result of a "
1284-
"spirv.SpecConstant or spirv.GlobalVariable or "
1291+
"spirv.SpecConstant or "
12851292
"spirv.SpecConstantCompositeOp op");
12861293
}
12871294
}

mlir/test/Dialect/SPIRV/IR/structure-ops.mlir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ spirv.module Logical GLSL450 {
501501
// -----
502502

503503
spirv.module Logical GLSL450 {
504-
// expected-error @+1 {{op initializer must be result of a spirv.SpecConstant or spirv.GlobalVariable or spirv.SpecConstantCompositeOp op}}
504+
// expected-error @+1 {{op initializer must be result of a spirv.SpecConstant or spirv.SpecConstantCompositeOp op}}
505505
spirv.GlobalVariable @var0 initializer(@var1) : !spirv.ptr<f32, Private>
506506
}
507507

mlir/test/Target/SPIRV/global-variable.mlir

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
// RUN: mlir-translate -no-implicit-module -test-spirv-roundtrip -split-input-file %s | FileCheck %s
22

3+
// RUN: %if spirv-tools %{ rm -rf %t %}
4+
// RUN: %if spirv-tools %{ mkdir %t %}
5+
// RUN: %if spirv-tools %{ mlir-translate --no-implicit-module --serialize-spirv --split-input-file --spirv-save-validation-files-with-prefix=%t/module %s %}
6+
// RUN: %if spirv-tools %{ spirv-val %t %}
7+
38
// CHECK: spirv.GlobalVariable @var0 bind(1, 0) : !spirv.ptr<f32, Input>
49
// CHECK-NEXT: spirv.GlobalVariable @var1 bind(0, 1) : !spirv.ptr<f32, Output>
510
// CHECK-NEXT: spirv.GlobalVariable @var2 built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
611
// CHECK-NEXT: spirv.GlobalVariable @var3 built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
712

8-
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
13+
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
914
spirv.GlobalVariable @var0 bind(1, 0) : !spirv.ptr<f32, Input>
1015
spirv.GlobalVariable @var1 bind(0, 1) : !spirv.ptr<f32, Output>
1116
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], []> {
1419

1520
// -----
1621

17-
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
18-
// CHECK: spirv.GlobalVariable @var1 : !spirv.ptr<f32, Input>
19-
// CHECK-NEXT: spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Input>
20-
spirv.GlobalVariable @var1 : !spirv.ptr<f32, Input>
21-
spirv.GlobalVariable @var2 initializer(@var1) bind(1, 0) : !spirv.ptr<f32, Input>
22-
}
23-
24-
// -----
25-
26-
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
22+
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage, Int8], []> {
2723
// CHECK: spirv.SpecConstant @sc = 1 : i8
2824
// CHECK-NEXT: spirv.GlobalVariable @var initializer(@sc) : !spirv.ptr<i8, Uniform>
2925
spirv.SpecConstant @sc = 1 : i8
@@ -33,7 +29,7 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
3329

3430
// -----
3531

36-
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
32+
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage, Int8], []> {
3733
// CHECK: spirv.SpecConstantComposite @scc (@sc0, @sc1, @sc2) : !spirv.array<3 x i8>
3834
// CHECK-NEXT: spirv.GlobalVariable @var initializer(@scc) : !spirv.ptr<!spirv.array<3 x i8>, Uniform>
3935
spirv.SpecConstant @sc0 = 1 : i8
@@ -47,7 +43,7 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
4743

4844
// -----
4945

50-
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
46+
spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader, Linkage], []> {
5147
spirv.GlobalVariable @globalInvocationID built_in("GlobalInvocationId") : !spirv.ptr<vector<3xi32>, Input>
5248
spirv.func @foo() "None" {
5349
// CHECK: %[[ADDR:.*]] = spirv.mlir.addressof @globalInvocationID : !spirv.ptr<vector<3xi32>, Input>

0 commit comments

Comments
 (0)