What is the way to deprecate preview extensions?

[MrSidims]

I'd like to discuss, what should be the way to deprecate extensions previously added to the translator as preview extension. A good example would be SPV_INTEL_token_type, which specification has never left intel/llvm#3788 . We (Intel SYCL compiler team) are quite sure, that we won't have problems with the deprecation, but we can't say for sure if there are no pre-generated SPIR-V modules 'in the wild' which would use the extension (some time ago I've noted, that there are several projects, that use llvm-spirv with --spirv-ext=+all , for example https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/HIPAMD.cpp#L178 ).

Right now I've created #3025 that would still support the extension during reverse translation (just in case), but will start emit an error in case if LLVM token type present in LLVM IR module during forward translation. Should the later be changed to a deprecation warning (note, right now the translator never emits warnings)?

[MrSidims]

Would like to hear an opinion from @bashbaug @svenvh and also from @whitneywhtsang @AlexVlx (who are potential users of the feature, would deprecation like this break anything on your side?).

[AlexVlx]

This looks good to me, we do not use it in our temporary fork, thank you for asking!

[whitneywhtsang]

Triton doesn't currently use it, verified all Triton test cases still pass with SPV_INTEL_token_type extension disabled.

[bashbaug]

In general, I am very supportive of deprecating (and ultimately removing) "preview extensions" to keep the codebase tidy.

Some things to consider, though:

• How long should we continue to maintain reverse translation support "just in case"? Is there some point when we can remove support entirely?
• Should we backport the deprecation changes to the different release branches? If we don't, there is a chance some SPIR-V LLVM Translator users wouldn't notice that a preview extension is deprecated until moving to a newer release branch.
• Should we backport any deprecation changes to the different release branches and then remove support entirely in the main branch? This is more work, but I think it's less likely that the deprecation or the removal slips through the cracks.

[svenvh]

No major concerns from our side regarding preview extensions.

I mostly agree with @bashbaug's considerations. We should maintain reverse translation support for a sufficiently long time; I'd suggest we aim for at least one release cycle in which the extension is deprecated but not removed on main (unless that is getting in the way of development on main or there are good reasons to do the removal earlier). I would encourage backporting the deprecation as it may help speed up extinction of the extension 'in the wild'.

[bashbaug]

Trying to summarize discussion to move forward. Does this sound right?

1. As the first step towards deprecating and ultimately removing support for a "preview extension", we will disable support for forward translation of the extension (this will prevent new SPIR-V modules from being generated using the extension), though we will continue to support reverse translation for the extension (this retains compatibility with existing SPIR-V modules "in the wild").
   • We will not emit a deprecation warning during reverse translation, though we may investigate this in the future.
   • We will make these changes in the main branch.
   • We encourage backporting the changes to other branches to speed up removal, but this is not required.
2. After at least one release cycle, we will remove support for reverse translation as well, at which point support for the "preview extension" is considered removed.
3. These are guidelines, not requirements, and we will consider exceptions on a case-by-case basis.

I don't mind adding this to the README if desired, or we can just refer back to this issue.

If all of this sounds correct, then I think we can proceed with reviewing and merging #3025, since it follows the guidelines described in step (1).

[MrSidims]

@bashbaug I agree with the proposed plan. Regarding backports: I find removal of the features from released branches to be a bit odd. May be it's still fine as we have continues releases from these branches.

[MrSidims]

Created #3094 to update README. Also backported deprecation in SPIR-V Writer of SPV_INTEL_token_type. Leaving issue open as I need to remove reverse translation support in the future.