Remove optionality of support for vendor-specific extension in glslang.

[DominikWitczakAMD]

Right now, in order to be able to access AMD-specific extensions (such as GL_AMD_shader_ballot), ISVs are required to modify CMake configuration and build customized versions of glslang, as CMake's ENABLE_AMD_EXTENSIONS option is disabled by default.

We would like to have that CMake option removed, making extension support always enabled in glslang. We see this problem being frequently raised by ISVs who use Vulkan, and it is also our belief that customized glslang builds are not the right way for providing support for GLSL / SPIR-V extensions in glslang in the foreseeable future.

While I referred to our existing extensions here as an example, we are worried about current approach to adding support for vendor-specific extensions as a whole. We would like to see glslang include support for all vendor-specific extensions in general, in order to avoid fragmenting Vulkan ecosystem.

[baldurk]

:+1: from me, especially since at runtime it's still protected by needing#extension VK_FOO_bar_blah : enable, so having default-on compile-time support won't affect shaders unless people opt-in anyway.

[ghost]

As glslang is being distributed in Vulkan SDKs (Lunar-G SDK, Android NDK, ...) as a prebuild binary, the build-time configuration may be highly uncomfortable for the developers.

Any run-time way for enabling extensions is better. If people are concerned in thight control of the feature set used by SPIR-V output, a commandline or config file can be used to select target SPIR-V extensions (in parallel to GLSL #extension enables in shader source).

[Novum]

Yes from our side as well. I don't think a command line option is necessary because the shader already controls the enabled extensions.

[kayru]

From Frostbite Engine point of view, #extension opt-in mechanism in shaders should be the only requirement for using an extension with vanilla VulkanSDK. We would like to see vendor extensions regularly up-streamed and available to use with minimal setup.

[johnkslang]

This was done to connect the code quality of an extension implementation with the extension. That is, accepting vendor-specific PRs with less code reading and testing when they were being "owned" by a specific vendor, making quality that vendor's responsibility, rather than Khronos's. That is, I was ensuring the quality of what's outside the #ifdef's but not what's inside.

That would be most important for use of glslang in a online environment rather than offline. E.g. vendor A doesn't want vendor B breaking glslang in an online situation.

However, what I think I'm hearing here is if it is offline, and the new code is protected by the #extension in the shader, then vendors are comfortable with that?

[DominikWitczakAMD]

AMD's preference is to see all vendor extensions integrated into glslang for both offline (i.e. using glslangvalidator) and online (i.e. run-time GLSL->SPIR-V conversion) use cases. This is vital for smooth ISV development experience. Maintenance of a number of glslang builds in a toolchain, as is the case right now, is a major burden for the developers

[gwihlidal]

Another vote from the Frostbite Engine side of things!

[Novum]

If a specific SDK doesn't want to include certain vendor extensions (e.g. Android, but there could be others) they could also compile without them if they prefer, but the official Vulkan SDK should include all extensions if possible.

I can see the point that there is a risk of bugs created by extension interaction, but can't judge the actual likeliness of that happening.

[DominikWitczakAMD]

Extension interaction would need to be worked out on a case-by-case basis. I'm afraid it's a prerequisite for a tool like glslang in this day and age.

[kholtnv]

It would be nice, just as a coding convention to continue to bracket vendor specific code with some "ifdef" (like #ifdef AMD_EXTENSION ... #endif) (and just define AMD_EXTENSION somewhere) so that it is clear that the bracketed code is from a vendor and not "core".

[dgkoch]

Yes NVIDIA's preference is definitely to have vendor extensions enabled by default as well. We don't think ISVs want to have multiples tools or versions of glslang for each IHV configuration that they are targeting.

[DominikWitczakAMD]

@johnkslang : It seems like people are in favor of the proposal. Following @kholtnv's suggestion, would you be OK with:

1. having the ENABLE_AMD_EXTENSIONS option renamed to AMD_EXTENSION
2. having AMD_EXTENSION enabled by default.
3. enabling support for future vendor-specific extensions in glslang from the moment they are merged into master, provided all relevant modifications are wrapped inside {vendor_name}_EXTENSION.

I can propose a MR for steps 1-2.

[johnkslang]

@DominikWitczakAMD yes that is fine, thanks.

[johnkslang]

So, expecting now that extension implementation can be enabled by default.