Skip to content

Commit

Permalink
Remove warning about unspecified vkbinding
Browse files Browse the repository at this point in the history
It is permitted that users omit explicit [vk::binding] settings on
HLSL-style register definitions. They are just opting into the
default implicit mapping, as they would with HLSL->VK in DXC.

That does mean register IDs can clash between resources which have
different register type but the same register number.

Such a problem can be resolved either with explicit bindings
([vk::binding]) in source, or using the "-fvk-binding" slangc options
which match DXC behavior.

Closes issue shader-slang#5938
  • Loading branch information
cheneym2 committed Jan 14, 2025
1 parent 4e62f98 commit 90f8d03
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 45 deletions.
41 changes: 5 additions & 36 deletions source/slang/slang-parameter-binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1060,22 +1060,6 @@ static void addExplicitParameterBindings_HLSL(
}
}

static void _maybeDiagnoseMissingVulkanLayoutModifier(
ParameterBindingContext* context,
DeclRef<VarDeclBase> const& varDecl)
{
// 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>())
{
getSink(context)->diagnose(
registerModifier,
Diagnostics::registerModifierButNoVulkanLayout,
varDecl.getName());
}
}

static void addExplicitParameterBindings_GLSL(
ParameterBindingContext* context,
RefPtr<ParameterInfo> parameterInfo,
Expand Down Expand Up @@ -1212,13 +1196,6 @@ static void addExplicitParameterBindings_GLSL(
return;

auto hlslToVulkanLayoutOptions = context->getTargetProgram()->getHLSLToVulkanLayoutOptions();
bool warnedMissingVulkanLayoutModifier = false;
// 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>());
}

// We need an HLSL register semantic to to infer from
auto hlslRegSemantic = varDecl.getDecl()->findModifier<HLSLRegisterSemantic>();
Expand Down Expand Up @@ -1254,21 +1231,13 @@ static void addExplicitParameterBindings_GLSL(
return;
}

// If inference is not enabled for this kind, we can issue a warning
if (hlslToVulkanLayoutOptions &&
!hlslToVulkanLayoutOptions->canInfer(vulkanKind, hlslInfo.space))
{
if (!warnedMissingVulkanLayoutModifier)
{
_maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl.as<VarDeclBase>());
warnedMissingVulkanLayoutModifier = true;
}
}

if (warnedMissingVulkanLayoutModifier)
if (hlslToVulkanLayoutOptions == nullptr ||
!hlslToVulkanLayoutOptions->canInferBindings() ||
!hlslToVulkanLayoutOptions->canInfer(vulkanKind, hlslInfo.space))
{
// If we warn due to invalid bindings and user did not set how to interpret 'hlsl style
// bindings', we should map `register` 1:1 with equivlent vulkan bindings.
// If the user did not set how to interpret 'hlsl style bindings', we should map
// `register` 1:1 with equivlent vulkan bindings.
if (!hlslToVulkanLayoutOptions || hlslToVulkanLayoutOptions->getKindShiftEnabledFlags() ==
HLSLToVulkanLayoutOptions::KindFlag::None)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
result code = -1
standard error = {
tests/diagnostics/hlsl-to-vulkan-shift-diagnostic.hlsl(11): warning 39013: shader parameter 'c' has a 'register' specified for D3D, but no '[[vk::binding(...)]]` specified for Vulkan
ConstantBuffer<Data> c : register(b2);
^~~~~~~~
tests/diagnostics/hlsl-to-vulkan-shift-diagnostic.hlsl(15): warning 39013: shader parameter 'u' has a 'register' specified for D3D, but no '[[vk::binding(...)]]` specified for Vulkan
RWStructuredBuffer<Data> u : register(u11);
^~~~~~~~
tests/diagnostics/hlsl-to-vulkan-shift-diagnostic.hlsl(15): error 39025: conflicting vulkan inferred binding for parameter 'c' overlap is 0 and 0
RWStructuredBuffer<Data> u : register(u11);
^
Expand Down
3 changes: 0 additions & 3 deletions tests/diagnostics/vk-bindings.slang.expected
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
result code = -1
standard error = {
tests/diagnostics/vk-bindings.slang(6): warning 39013: shader parameter 't' has a 'register' specified for D3D, but no '[[vk::binding(...)]]` specified for Vulkan
Texture2D t : register(t0);
^~~~~~~~
tests/diagnostics/vk-bindings.slang(14): error 39015: shader parameter 'b' consumes whole descriptor sets, so the binding must be in the form '[[vk::binding(0, ...)]]'; the non-zero binding '2' is not allowed
[[vk::binding(2,1)]]
^~~~~~~
Expand Down

0 comments on commit 90f8d03

Please sign in to comment.