From 0059e5e3b1b1dc57cb8bddb48239cd2c93a990d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Tue, 30 Sep 2025 18:04:55 +0200 Subject: [PATCH 1/4] Reject `local_size_*_id` before SPIR-V 1.2 `OpExecutionModeId` and the `LocalSizeId` execution mode are missing before SPIR-V 1.2[0], and currently when targeting an earlier version, it is ignored and the work group sizes are set to 1 instead without any diagnostic. So to prevent surprises, reject `local_size_*_id` layout qualifiers when targeting a SPIR-V version before 1.2. [0]: https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpExecutionModeId --- glslang/MachineIndependent/ParseHelper.cpp | 26 +++++++++++++--------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 845020afa2..738eb53bc9 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -7202,17 +7202,21 @@ void TParseContext::setLayoutQualifier(const TSourceLoc& loc, TPublicType& publi return; } if (spvVersion.spv != 0) { - if (id == "local_size_x_id") { - publicType.shaderQualifiers.localSizeSpecId[0] = value; - return; - } - if (id == "local_size_y_id") { - publicType.shaderQualifiers.localSizeSpecId[1] = value; - return; - } - if (id == "local_size_z_id") { - publicType.shaderQualifiers.localSizeSpecId[2] = value; - return; + static const char * const specIds[] = { + "local_size_x_id", + "local_size_y_id", + "local_size_z_id", + }; + + for (std::size_t i = 0; i < std::size(specIds); i++) { + if (id == specIds[i]) { + if (spvVersion.spv >= glslang::EShTargetSpv_1_2) + publicType.shaderQualifiers.localSizeSpecId[i] = value; + else + error(loc, "requires SPIR-V 1.2", id.c_str(), ""); + + return; + } } } } From c42661b75fc096ebb218c6938e889b59cb3a37cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Tue, 30 Sep 2025 18:29:38 +0200 Subject: [PATCH 2/4] spirv: only set LocalSizeId mode when necessary Only use `LocalSizeId` when necessary, same changes as #3351 but applied to mesh and task shaders. Fixes #3739 --- SPIRV/GlslangToSpv.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index b409ba3fbd..bfd0f20f3b 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -1984,7 +1984,7 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion, break; } case EShLangTask: - case EShLangMesh: + case EShLangMesh: { if(isMeshShaderExt) { builder.addCapability(spv::Capability::MeshShadingEXT); builder.addExtension(spv::E_SPV_EXT_mesh_shader); @@ -1992,7 +1992,14 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion, builder.addCapability(spv::Capability::MeshShadingNV); builder.addExtension(spv::E_SPV_NV_mesh_shader); } - if (glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_6) { + bool needSizeId = false; + for (int dim = 0; dim < 3; ++dim) { + if ((glslangIntermediate->getLocalSizeSpecId(dim) != glslang::TQualifier::layoutNotSet)) { + needSizeId = true; + break; + } + } + if (glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_6 && needSizeId) { std::vector dimConstId; for (int dim = 0; dim < 3; ++dim) { bool specConst = (glslangIntermediate->getLocalSizeSpecId(dim) != glslang::TQualifier::layoutNotSet); @@ -2024,7 +2031,7 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion, builder.addExecutionMode(shaderEntry, (spv::ExecutionMode)mode); } break; - + } default: break; } From f50e9a61da16a7a7b6cbb4e75adc89af0b9df979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Tue, 30 Sep 2025 18:20:52 +0200 Subject: [PATCH 3/4] spirv: use `LocalSizeId` exec mode when targeting >= SPIR-V 1.2 `OpExecutionModeId` and the `LocalSizeId` execution mode were added in SPIR-V 1.2[0]. So use them when targeting at least 1.2. [0]: https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpExecutionModeId See #3510 --- SPIRV/GlslangToSpv.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index bfd0f20f3b..3ecf4a1e16 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -1836,7 +1836,7 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion, break; } } - if (glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_6 && needSizeId) { + if (glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_2 && needSizeId) { std::vector dimConstId; for (int dim = 0; dim < 3; ++dim) { bool specConst = (glslangIntermediate->getLocalSizeSpecId(dim) != glslang::TQualifier::layoutNotSet); @@ -1999,7 +1999,7 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion, break; } } - if (glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_6 && needSizeId) { + if (glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_2 && needSizeId) { std::vector dimConstId; for (int dim = 0; dim < 3; ++dim) { bool specConst = (glslangIntermediate->getLocalSizeSpecId(dim) != glslang::TQualifier::layoutNotSet); From f6045255dc5341b829dee15567953c30a826e3c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Tue, 30 Sep 2025 18:35:34 +0200 Subject: [PATCH 4/4] spirv: remove unnecessary assignment Changing the value of `needSizeId` in the loop will have no effect because it is not checked after that anywhere. --- SPIRV/GlslangToSpv.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 3ecf4a1e16..d9c6895328 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -1844,7 +1844,6 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion, if (specConst) { builder.addDecoration(dimConstId.back(), spv::Decoration::SpecId, glslangIntermediate->getLocalSizeSpecId(dim)); - needSizeId = true; } } builder.addExecutionModeId(shaderEntry, spv::ExecutionMode::LocalSizeId, dimConstId);