Skip to content

Commit a3babf1

Browse files
committed
Add warning for ignored binding attributes on uniforms
When binding attributes (like [[vk::binding]]) are specified on uniforms that get packed into the default constant buffer, these binding attributes are effectively ignored since the uniform will always be placed at descriptor set 0, binding 0. This can be confusing for users who expect their explicit bindings to take effect. This change adds a new warning (71) that informs users when their binding attributes on uniforms will be ignored, and suggests declaring the uniform inside a constant buffer to preserve the explicit binding. The warning helps users understand: 1. Why their binding attribute isn't having the expected effect 2. That the uniform is being packed into the default constant buffer 3. How to fix it by using a constant buffer declaration Added test case in tests/bugs/binding-attribute-ignored.slang to verify the warning behavior. (cherry picked from commit 82f66e7)
1 parent e043428 commit a3babf1

File tree

3 files changed

+33
-26
lines changed

3 files changed

+33
-26
lines changed

source/slang/slang-diagnostic-defs.h

+6
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,12 @@ DIAGNOSTIC(
248248
"the output path '$0' is not associated with any entry point; a '-o' option for a compiled "
249249
"kernel must follow the '-entry' option for its corresponding entry point")
250250

251+
DIAGNOSTIC(
252+
71,
253+
Warning,
254+
bindingAttributeIgnoredOnUniform,
255+
"binding attribute on uniform '$0' will be ignored since it will be packed into the default constant buffer at descriptor set 0 binding 0. To use explicit bindings, declare the uniform inside a constant buffer.")
256+
251257
DIAGNOSTIC(
252258
80,
253259
Error,

source/slang/slang-parameter-binding.cpp

+9-26
Original file line numberDiff line numberDiff line change
@@ -3511,34 +3511,17 @@ static void collectParameters(ParameterBindingContext* inContext, ComponentType*
35113511
/// Emit a diagnostic about a uniform/ordinary parameter at global scope.
35123512
void diagnoseGlobalUniform(SharedParameterBindingContext* sharedContext, VarDeclBase* varDecl)
35133513
{
3514-
// This subroutine gets invoked if a shader parameter containing
3515-
// "ordinary" data (sometimes just called "uniform" data) is present
3516-
// at the global scope.
3517-
//
3518-
// Slang can support such parameters by aggregating them into
3519-
// an implicit constant buffer, but it is also common for programmers
3520-
// to accidentally declare a global-scope shader parameter when they
3521-
// meant to declare a global variable instead:
3522-
//
3523-
// int gCounter = 0; // this is a shader parameter, not a global
3524-
//
3525-
// In order to avoid mistakes, we'd like to warn the user when
3526-
// they write code like the above, and hint to them that they
3527-
// should make their intention more explicit with a keyword:
3528-
//
3529-
// static int gCounter = 0; // this is now a (static) global
3530-
//
3531-
// uniform int gCounter; // this is now explicitly a shader parameter
3532-
//
3533-
// We skip the diagnostic whenever the variable was explicitly `uniform`,
3534-
// under the assumption that the programmer who added that modifier
3535-
// knew what they were opting into.
3536-
//
3537-
if (varDecl->hasModifier<HLSLUniformModifier>())
3538-
return;
3539-
35403514
getSink(sharedContext)
35413515
->diagnose(varDecl, Diagnostics::globalUniformNotExpected, varDecl->getName());
3516+
3517+
// Check if the variable has a binding attribute
3518+
if (varDecl->findModifier<GLSLBindingAttribute>())
3519+
{
3520+
sharedContext->m_sink->diagnose(
3521+
varDecl,
3522+
Diagnostics::bindingAttributeIgnoredOnUniform,
3523+
varDecl->getName());
3524+
}
35423525
}
35433526

35443527

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Test that binding attributes on uniforms that get packed into the default uniform buffer trigger a warning
2+
// GH: 4251
3+
// Test Vulkan binding syntax
4+
[[vk::binding(1, 2)]]
5+
uniform float4 g_position; // This should trigger a warning since it's a uniform that will be packed
6+
7+
[[vk::binding(3, 1)]]
8+
uniform float4x4 g_transform; // This should also trigger a warning
9+
10+
// This won't trigger a warning because it's a texture (not packed into default uniform buffer)
11+
[[vk::binding(0, 0)]]
12+
Texture2D g_texture;
13+
14+
[shader("vertex")]
15+
float4 main(float4 pos : POSITION) : SV_POSITION
16+
{
17+
return g_position;
18+
}

0 commit comments

Comments
 (0)