diff options
| -rw-r--r-- | source/slang-glslang/slang-glslang.cpp | 4 | ||||
| -rw-r--r-- | source/slang/slang-parameter-binding.cpp | 33 | ||||
| -rw-r--r-- | tests/diagnostics/vk-binding-with-register-no-warning.slang | 35 |
3 files changed, 61 insertions, 11 deletions
diff --git a/source/slang-glslang/slang-glslang.cpp b/source/slang-glslang/slang-glslang.cpp index e830fb15a..56be8b042 100644 --- a/source/slang-glslang/slang-glslang.cpp +++ b/source/slang-glslang/slang-glslang.cpp @@ -189,7 +189,7 @@ extern "C" #ifdef _MSC_VER _declspec(dllexport) #else -__attribute__((__visibility__("default"))) + __attribute__((__visibility__("default"))) #endif bool glslang_disassembleSPIRVWithResult( const uint32_t* contents, @@ -237,7 +237,7 @@ extern "C" #ifdef _MSC_VER _declspec(dllexport) #else -__attribute__((__visibility__("default"))) + __attribute__((__visibility__("default"))) #endif bool glslang_disassembleSPIRV(const uint32_t* contents, int contentsSize) { diff --git a/source/slang/slang-parameter-binding.cpp b/source/slang/slang-parameter-binding.cpp index 86d76fc6d..4ba9e4e03 100644 --- a/source/slang/slang-parameter-binding.cpp +++ b/source/slang/slang-parameter-binding.cpp @@ -1069,7 +1069,7 @@ static void addExplicitParameterBindings_HLSL( } } -static void _maybeDiagnoseMissingVulkanLayoutModifier( +static bool _maybeDiagnoseMissingVulkanLayoutModifier( ParameterBindingContext* context, DeclRef<VarDeclBase> const& varDecl) { @@ -1077,7 +1077,7 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier( if (varDecl.getDecl()->hasModifier<PushConstantAttribute>() || varDecl.getDecl()->hasModifier<ShaderRecordAttribute>()) { - return; + return false; } // If the user didn't specify a `binding` (and optional `set`) for Vulkan, @@ -1085,6 +1085,11 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier( // oversight on their part. if (auto registerModifier = varDecl.getDecl()->findModifier<HLSLRegisterSemantic>()) { + // Don't warn if the declaration already has a vk::binding attribute + if (varDecl.getDecl()->findModifier<GLSLBindingAttribute>()) + { + return false; + } auto varType = getType(context->getASTBuilder(), varDecl.as<VarDeclBase>()); if (auto textureType = as<TextureType>(varType)) { @@ -1095,7 +1100,7 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier( registerModifier, Diagnostics::registerModifierButNoVulkanLayout, varDecl.getName()); - return; + return true; } } @@ -1111,7 +1116,9 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier( Diagnostics::registerModifierButNoVkBindingNorShift, varDecl.getName(), registerClassName); + return true; } + return false; } static void addExplicitParameterBindings_GLSL( @@ -1273,8 +1280,16 @@ static void addExplicitParameterBindings_GLSL( // If we are not told how to infer bindings with a compile option, we warn if (hlslToVulkanLayoutOptions == nullptr || !hlslToVulkanLayoutOptions->canInferBindings()) { - warnedMissingVulkanLayoutModifier = true; - _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>()); + warnedMissingVulkanLayoutModifier = + _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>()); + // For behavioral compatibility, if we didn't warn due to a push constant or shader record, + // we still need to set the flag to true to maintain the same binding logic + if (!warnedMissingVulkanLayoutModifier && + (varDecl.getDecl()->hasModifier<PushConstantAttribute>() || + varDecl.getDecl()->hasModifier<ShaderRecordAttribute>())) + { + warnedMissingVulkanLayoutModifier = true; + } } // We need an HLSL register semantic to to infer from @@ -1301,8 +1316,8 @@ static void addExplicitParameterBindings_GLSL( { if (!warnedMissingVulkanLayoutModifier) { - _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>()); - warnedMissingVulkanLayoutModifier = true; + warnedMissingVulkanLayoutModifier = + _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>()); } return; } @@ -1323,8 +1338,8 @@ static void addExplicitParameterBindings_GLSL( { if (!warnedMissingVulkanLayoutModifier) { - _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>()); - warnedMissingVulkanLayoutModifier = true; + warnedMissingVulkanLayoutModifier = + _maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>()); } } diff --git a/tests/diagnostics/vk-binding-with-register-no-warning.slang b/tests/diagnostics/vk-binding-with-register-no-warning.slang new file mode 100644 index 000000000..3bbbcc1bd --- /dev/null +++ b/tests/diagnostics/vk-binding-with-register-no-warning.slang @@ -0,0 +1,35 @@ +// vk-binding-with-register-no-warning.slang + +//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK):-target metallib -matrix-layout-row-major -entry main + +// Test that no spurious warnings are generated when vk::binding is present alongside register +// This addresses issue #7553 where the compiler incorrectly warned about missing vk::binding + +// Should NOT generate warning - has both vk::binding and register +// CHECK-NOT: ([[# @LINE+1]]): warning 39029 +[[vk::binding(0, 0)]] cbuffer ConstantBuffer : register(b0) +{ + float4 testValue; +}; + +// Should NOT generate warning - has both vk::binding and register +// CHECK-NOT: ([[# @LINE+1]]): warning 39029 +[[vk::binding(1, 0)]] Texture2D tex : register(t1); + +// Should NOT generate warning - has both vk::binding and register +// CHECK-NOT: ([[# @LINE+1]]): warning 39029 +[[vk::binding(2, 0)]] SamplerState sampler : register(s2); + +// Should generate warning - has register but no vk::binding +// CHECK: ([[# @LINE+1]]): warning 39029 +Texture2D texNoBinding : register(t3); + +// Should generate warning - has register but no vk::binding +// CHECK: ([[# @LINE+1]]): warning 39029 +ConstantBuffer<float4> cbNoBinding : register(b4); + +[shader("compute")] +[numthreads(1, 1, 1)] +void main() +{ +}
\ No newline at end of file |