-
Notifications
You must be signed in to change notification settings - Fork 12
A proposal to deprecate APIs we never intended #31
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
base: main
Are you sure you want to change the base?
A proposal to deprecate APIs we never intended #31
Conversation
There's a lot of cruft in `slang.h` and the `slangc` command-line options that was never really meant to be used, and a lot of other cruft that we knew was a liability from the moment it landed. This is a proposal to mark as much stuff deprecated as currently seems possible, with the hope that over time we can clean up the implementation by not needing to support too many legacy features and in-retrospect-poor design choices.
This is intended to be the implementation PR for shader-slang/spec#31
This is intended to be the implementation PR for shader-slang/spec#31
Renumber coherent pointer proposal to shader-slang#31.
| * option `-source-embed-style` should be deprecated | ||
|
|
||
| * option `-source-embed-name` should be deprecated | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am actively using these options, they're useful for small applications that want to embed SPIR-V in the executable so that they don't need to care about a filesystem. They also mirror equivalent glslc / glslang functionality, making slangc a drop-in replacement in an existing CMake shader compilation setup.
I have no issues with the other deprecations listed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'm also not the only person using this functionality, here's someone recently on Discord asking how to do exactly what these options do: https://discord.com/channels/1303735196696445038/1397550571321299006
|
Tagging @danginsburg for input and awareness as well |
The only ones that would impact our usage are:
|
|
|
||
| * option `-msvc-style-bitfield-packing` should be marked as undesirable (and subject to future deprecation/removal) | ||
|
|
||
| * option `-fspv-reflect` should be marked as undesirable (and subject to future deprecation/removal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn’t this required to support Khronos’s SPIRV reflection library? https://github.com/KhronosGroup/SPIRV-Reflect if I understand right, then IIRC Valve was a proponent that we support SPIRV-Reflect decorators.
If this is true, then I’d probably object to removing this command line argument.
|
|
||
| * option `-reflection-json` should be marked as undesirable (and subject to future deprecation/removal) | ||
|
|
||
| * option `-msvc-style-bitfield-packing` should be marked as undesirable (and subject to future deprecation/removal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may be a need for the functionality this option provides, it was brought up in this issue: shader-slang/slang#8112
|
|
||
| * `ISlangWriter` | ||
|
|
||
| * The ability for users to override the "prelude" code for particular target languages and/or downstream compilers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are currently using a custom build of Slang that uses our own C++ prelude in addition to some other changes. This feature could be a potentially useful way for us to use the official build of Slang.
Our intention is to contribute our changes back, but some of our prelude changes rely on a LLVM compiler plugin so I'm not sure if all our prelude changes will make it in.
There's a lot of cruft in
slang.hand theslangccommand-line options that was never really meant to be used, and a lot of other cruft that we knew was a liability from the moment it landed. This is a proposal to mark as much stuff deprecated as currently seems possible, with the hope that over time we can clean up the implementation by not needing to support too many legacy features and in-retrospect-poor design choices.