-
Notifications
You must be signed in to change notification settings - Fork 17
[NNNN] New proposal for feature profiles #337
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?
Conversation
This proposal suggests a different approch for handling target feature profiles which is both more flexible and more meaningful across APIs. The goal is to redefine the concept of target profiles as DXC represented them to extend more clearly across the whole ecosystem.
I like this direction, personally. I think there's 3 interesting axes that shader models have historically meant:
Right now a shader model controls both the set of features you can use, and the interchange format that will be produced. With (very) few exceptions, in DXC, enabling a more capable shader model doesn't allow you to disable any of the optional parts of it. So I'm very much in favor of separating out compiler enablement of functionality to a per-feature basis rather than a bundle of features. I would also argue that in general, developers don't care about the interchange format, as long as it is sufficient to express their shader code. There might be exceptions to this: for example, DXIL 1.9's vectorization may provide performance or size benefits with no new features being used in the shader source. I think what developers will generally want is a way to enable the set of features that are supported by their target devices, with the "oldest" interchange format that can support those features, and have the compiler error if they attempt to use a feature that's not explicitly enabled, and then also have a way to optionally select a specific bytecode format that's newer. |
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.
A couple of comments, but this looks ready to be merged and discussed further to me.
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.
LGTM. I like this direction. It was confusing using the shader models because they do not map nicely to when features were made available on Vulkan.
ProfileName: Android15 | ||
TargetDefinitions: | ||
- BaseTriple: spirv-vulkan1.2 | ||
Stages: [pixel, vertex, compute] # Not really sure what all Android supports... |
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 think this is the minimum, but in practice many more are supported. This would be a good place to bring up another issue with Vulkan. Not all features are enabled by extensions. Some features are enabled by feature bits. We might want a way to describe features bits as well. Note that when a feature becomes core, it gets a feature bit and does not use the extensions.
The second issue is that the SPV_* extensions that get enabled in Vulkan do not necessarily map to a SPIR-V feature. For the backend, we'll have to map the vulkan extensions to available spir-v extensions.
With that said, I think that using the vulkan extensions is the right direction. It will be easier for Vulkan developers because that is what they are using.
Features marked "Required" are enabled in the compiler and available to use | ||
without triggering diagnostics when the target profile is specified. Features | ||
not explicitly required are treated as unavailable unless they have been | ||
explicitly enabled by the user. |
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.
Do you need to call out that features can be explicitly disabled, even if they are "Required".
Co-authored-by: Steven Perron <[email protected]>
Co-authored-by: Steven Perron <[email protected]>
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.
This is in good shape to be merged and have further consideration in tree.
This proposal suggests a different approch for handling target feature profiles which is both more flexible and more meaningful across APIs. The goal is to redefine the concept of target profiles as DXC represented them to extend more clearly across the whole ecosystem.