Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions source/slang-glslang/slang-glslang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
{
Expand Down
33 changes: 24 additions & 9 deletions source/slang/slang-parameter-binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1069,22 +1069,27 @@ static void addExplicitParameterBindings_HLSL(
}
}

static void _maybeDiagnoseMissingVulkanLayoutModifier(
static bool _maybeDiagnoseMissingVulkanLayoutModifier(
ParameterBindingContext* context,
DeclRef<VarDeclBase> const& varDecl)
{
// Don't warn if the declaration is a vk::push_constant or shaderRecordEXT
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,
// but they *did* specify a `register` for D3D, then that is probably an
// 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))
{
Expand All @@ -1095,7 +1100,7 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier(
registerModifier,
Diagnostics::registerModifierButNoVulkanLayout,
varDecl.getName());
return;
return true;
}
}

Expand All @@ -1111,7 +1116,9 @@ static void _maybeDiagnoseMissingVulkanLayoutModifier(
Diagnostics::registerModifierButNoVkBindingNorShift,
varDecl.getName(),
registerClassName);
return true;
}
return false;
}

static void addExplicitParameterBindings_GLSL(
Expand Down Expand Up @@ -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
Expand All @@ -1301,8 +1316,8 @@ static void addExplicitParameterBindings_GLSL(
{
if (!warnedMissingVulkanLayoutModifier)
{
_maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
warnedMissingVulkanLayoutModifier = true;
warnedMissingVulkanLayoutModifier =
_maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
}
return;
}
Expand All @@ -1323,8 +1338,8 @@ static void addExplicitParameterBindings_GLSL(
{
if (!warnedMissingVulkanLayoutModifier)
{
_maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
warnedMissingVulkanLayoutModifier = true;
warnedMissingVulkanLayoutModifier =
_maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
}
}

Expand Down
35 changes: 35 additions & 0 deletions tests/diagnostics/vk-binding-with-register-no-warning.slang
Original file line number Diff line number Diff line change
@@ -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()
{
}
Loading