slang icon indicating copy to clipboard operation
slang copied to clipboard

Cannot use different push constants for different entry points

Open georgeouzou opened this issue 7 months ago • 5 comments

In the following shader file there is the following problem: The user can have a different buffer for binding 1 per entry point, but not different push constants. Instead the push constants buffer is "translated" as a constant buffer with binding 2.

georgeouzou avatar May 17 '25 13:05 georgeouzou

@georgeouzou we don't understand the issue. Can you help to clarify what the behavior you're expecting is?

bmillsNV avatar May 22 '25 22:05 bmillsNV

Hello @bmillsNV, the push contant annotated constant buffers are compiled to uniform blocks with a specific binding number instead of push_constant blocks. This happens only if they are declared as local entry point arguments.

georgeouzou avatar May 24 '25 18:05 georgeouzou

Hi. I'm not 100% sure why the version of the code you posted isn't getting translated as you want/expect. I'm not sure when we'll have time to fully debug and fix the issue, but I wanted to show you some viable workarounds that you can use in the interim.

We have logic in the Slang compiler that automatically translates ordinary uniform entry-point parameters into a push-constant buffer, so that there is no need to use [[vk::push_constant]] nor a ConstantBuffer<> in order to get the result you want. Here is a playground that I believe gives you the output code you wanted, simply by removing extraneous layout annotations.

To go further down this road: the compiler will automatically gather multiple uniform parameters on an entry point into a single push-constant buffer, so there is no need to have the distinct PC1 and PC2 types; it's fine to just declare the parameters on the entry point, like one would do for a more-or-less ordinary function.

I went ahead and put together another playground that further simplifies things so that the file just contains the two entry point function declarations.

The streamlined versions of your example that I've posted still yield exactly the same layout in the generated GLSL/SPIR-V that you seemed to be trying for. Our experience from having worked with many shader codebases over many years is that manual binding annotations like [[vk::...]] are, more often than not, a liability for the long-term maintainability, portability, and overall quality of a shader codebase. The Slang compiler strives to support manual annotations, for the sake of developers who insist on using them, but the language design and compiler architecture prioritizes supporting developers in writing simpler code that is easier to maintain, while still achieving high performance.

tangent-vector avatar Jun 05 '25 19:06 tangent-vector

@tangent-vector thanks for the detailed tips and explanation! In the case of a more complex code-base, i think this style would require the need for shader reflection and would lead to an OpenGL-glGetUniformLocation kind of solution.

georgeouzou avatar Jun 14 '25 08:06 georgeouzou

As for ray tracing shaders, can a user add local entry-point variables that compile to push constants instead of shader records? example

georgeouzou avatar Jun 20 '25 16:06 georgeouzou

On your last point, there has been some discussion within the dev team of changing the default handling on uniform entry-point parameters for ray-tracing shaders so that they would directly match the behavior implemented for all the other stages (mapping to push constants). The current behavior was implemented at a point where the ray-tracing support in Vulkan and D3D was quite new and the overall developer consensus on how to best drive those APIs had not yet formed. If we were implementing ray-tracing support in Slang today, we would almost certainly treat uniform entry-point parameters the same for ray tracing as for any other stages.

The catch is that, while there may not be much code that uses the current functionality, changing the default behavior for uniform entry-point parameters would constitute a breaking change and, under our current plan for how to handle language versions and breaking changes, we would only be able to implement that new default starting in the Slang 2026 language version.

We haven't gone through the full evolution proposal/review process yet, but it is likely that we would tackle this issue in the short term by having a family of wrapper types like ShaderRecord<T> and PushConstant<T> that would stand in for a T (much like how a ConstantBuffer<T> does) and make the intended mapping to API-specific resources explicit. Thus a codebase that wants to declare push constants on ray-tracing entry points while staying on the current language version (Slang 2025) could use PushConstant<T> to get that behavior.

tangent-vector avatar Jul 14 '25 20:07 tangent-vector

@tangent-vector thanks for the info! The PushConstant<T> solution seems ok for all the mentioned use cases.

georgeouzou avatar Jul 15 '25 15:07 georgeouzou

Posting notes from debugging.

The original shader provided via playground defines the entry points with attributes on the parameters, e.g.,:

[shader("compute")]
[numthreads(64, 1, 1)]
void main1(
    uint tidx : SV_DispatchThreadID,
    [[vk::binding(1, 0)]] StructuredBuffer<float> input_buf,
    [[vk::push_constant]] ConstantBuffer<PC1> pc)
{ ... }

It looks like Slang currently ignores these attributes for entry point parameters. If the binding number is adjusted, one can see that it's a coincidence that the input_buf binding is ending up with the value requested. Likewise, the request for pc to be made a push constant buffer is also ignored, and it just assigns it a binding as if it were a ConstantBuffer.

I note this comment in the implementation of RefPtr<ProgramLayout> generateParameterBindings(TargetProgram* targetProgram, DiagnosticSink* sink) at slang-parameter-binding.cpp:4251

    // Note that we do *not* support explicit binding annotations
    // on entry point parameters, so we only consider global shader
    // parameters here.

@tangent-vector Do you know if these comments are just out of date? I'm wondering if the current behavior is considered broken, or if I should instead be adding a warning for the presence of these particular [[vk::]] annotations when detected on entry point parameters. (It's specific to entry points, they work correctly on global parameters)

jhelferty-nv avatar Sep 05 '25 18:09 jhelferty-nv

Based on internal discussion, I'll add a diagnostic for now.

jhelferty-nv avatar Sep 09 '25 14:09 jhelferty-nv

Going to dig a bit more into this today. We have a proposal to correct this issue here, but it needs updating to accommodate the newer Type<T> pattern: https://github.com/shader-slang/spec/blob/main/proposals/017-shader-record.md

natevm avatar Sep 09 '25 16:09 natevm

@natevm I've created PR #8487 adding a diagnostic for now. Do you have a separate task to implement the longer-term solution from the proposal?

jhelferty-nv avatar Sep 19 '25 05:09 jhelferty-nv

@jhelferty-nv I did get a working version of the proposal, however there was relatively strong pushback from Tess and Yong. The mixed reaction signals that the current proposal isn’t the right solution to the problem.

natevm avatar Sep 19 '25 05:09 natevm