Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specialization constants #16

Closed
RandomShaper opened this issue Oct 20, 2022 · 6 comments
Closed

Specialization constants #16

RandomShaper opened this issue Oct 20, 2022 · 6 comments
Labels
needs-triage Requires Shader Model Feature requests that require DXIL and Shader Model updates

Comments

@RandomShaper
Copy link

RandomShaper commented Oct 20, 2022

Is your feature request related to a problem? Please describe.
Yes. Currently the whole HLSL to DXIL to PSO flow must be re-run whenever certain key values that need to vary among variants of a given shader change.

Describe the solution you'd like
I'd like that there was a syntactic way to declare specialization constants in HLSL, similarly as it can be done in GLSL. For PSO creation non-default values could be specified, much like it currently works in Vulkan.

UPDATE: @reduz's comment reminded me that there are some compatibility annotations (https://github.com/microsoft/DirectXShaderCompiler/blob/main/docs/SPIR-V.rst#specialization-constants) that make it possible to have concepts from the Vulkan realm in HLSL. However, for first class support, a more native syntax, like register() in resource declarations would be what I'm suggesting; e.g.:

// A contrived example, just to illustrate
bool lighting_enabled = false : constant(0);
int shadow_passes = 4 : constant(1);

Describe alternatives you've considered

Additional context
I'm aware that this change would span across the whole ecosystem, from HLSL parsing up to IHV drivers, but it would be worthy to do.

It was also requested in the DXC repo: #71.

@reduz
Copy link

reduz commented Oct 21, 2022

Just for the record, HLSL does support specialization constants (As it can be compiled to SPIRV by both Microsoft compiler and GLSLang). The support is entirely lacking in DXIL.

There are two main uses for specialization constants. The lack of them makes these use cases cumbersome:

  1. Massively reduce the amount of shader variants precompiled or cached: The GPU driver will still interpret DXIL as source code and then compile to native. The driver will still do dead code elimination either case, because LLVM IR does not suffice for this end.
  2. Allow the possibility to have proper ubershaders, where the PSO of a main shader containing all possible features first is compiled (which will be less efficient due to generally bad occupancy), and then use specialization constants to compile on threads the PSO versions that the game actually needs (and swap them by the specialized versions on demand). This is super useful for modern forward+ renderers.

There is zero effort in supporting them, since it should just be an extra argument to the pipeline compilation, like in Vulkan.

@llvm-beanz
Copy link
Collaborator

@RandomShaper, thank you very much for filing this request!

This request is not new to us. We've heard it and it is on our minds. Unfortunately this is not a feature we can develop publicly. As we state in the process docs:

This process cannot be used for features that require Shader Model changes because Shader Model collaboration with hardware vendors occurs in private under NDA.

Adding specialization constants requires altering the DXIL specification, the DirectX runtime, and the driver interface, so this feature would need to be part of a new Shader Model.

We don't really have a good way to track that publicly, so I'll take an action item to discuss with my team how to best manage this so that we don't leave you completely hanging.

@reduz, thank you for the extra context. The more context we have the easier it is for us to prioritize requests. I also would ask that you refrain from comments like:

There is zero effort in supporting them, since it should just be an extra argument to the pipeline compilation, like in Vulkan.

This kind of generalization isn't constructive, and undermines the work of our team and our partners both inside Microsoft and across the industry. Your statement assumes that we would adopt the Vulkan syntax (which isn't a given), and then ignores that we would need to support code generation, runtime integration and work with hardware vendors to update drivers.

I assure you if this required "zero effort", the feature would be available already.

@reduz
Copy link

reduz commented Oct 24, 2022

This kind of generalization isn't constructive, and undermines the work of our team and our partners both inside Microsoft and across the industry. Your statement assumes that we would adopt the Vulkan syntax (which isn't a given), and then ignores that we would need to support code generation, runtime integration and work with hardware vendors to update drivers.

Sorry Chris, this is not what I intended to say and I apologize if it came through that way (I'm not a native English speaker), as I fully understand the effort and cost involved (I take part in such working groups too).

My comment makes reference to the fact that, unlike other driver features, we guess that this one should not require doing significant driver modifications (bureaucracy aside). We experimented with @RandomShaper on patching DXIL code (in a manner similar of how the driver would do so) and observing native bytecode generated by the drivers afterwards, and all of them do the proper dead code removal.

@reduz
Copy link

reduz commented Oct 24, 2022

Pinging @jenatali as potential stakeholder within Microsoft.

@llvm-beanz llvm-beanz added the Requires Shader Model Feature requests that require DXIL and Shader Model updates label Oct 27, 2022
@llvm-beanz
Copy link
Collaborator

As discussed in two DXC issues DXC/4731 & DXC/4290, we should also extend specialization constants to driver-defined constants and for use as array size parameters.

@llvm-beanz
Copy link
Collaborator

We moved the original DXC issue over to this repository and will track the feature request using the original issue:
#71

@llvm-beanz llvm-beanz closed this as not planned Won't fix, can't repro, duplicate, stale Aug 6, 2024
@github-project-automation github-project-automation bot moved this to Triaged in HLSL Triage Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Requires Shader Model Feature requests that require DXIL and Shader Model updates
Projects
Archived in project
Development

No branches or pull requests

3 participants