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

Add deprecatedVersion field to annotate phase-out features #249

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PENGUINLIONG
Copy link
Contributor

The current grammar JSON lacks a mechanism to annotate phase-out features that are not currently removed from the specification, which currently include OpDecorationGroup, OpGroupDecorate, and OpGroupMemberDecorate as in SPIR-V 1.5, Revision 5.

This PR introduces a deprecatedVersion field to show that such features are not recommended to be emitted and the SPIR-V consumers can decide not to support them, if it's preferred.

deprecatedVersion gives the version of SPIR-V specification, in which a feature is deprecated in any of its revisions. It interacts with existing fields version and lastVersion, where version is the first version a feature is supported; and lastVersion the version a feature is completely removed from the specification. The lifetime of a feature can thus be bound as following: version < deprecatedVersion <= lastVersion.

@CLAassistant
Copy link

CLAassistant commented Oct 23, 2021

CLA assistant check
All committers have signed the CLA.

@raunraun
Copy link
Contributor

The SPIR working group likes the idea, but wants to do some internal testing to ensure the various tools that consume the grammar are working with this change.

@PENGUINLIONG
Copy link
Contributor Author

Any update on this?

@raunraun
Copy link
Contributor

the group is looking at this right now and will likely have a slight counter proposal that includes multiple fields in the deprecatedVersion tag (such as a link/reference to the replacement functionality the deprecated feature). Adding fields is usually safe, but there are many consumers of the json that all need testing - which is happening now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants