Add requirement for mandating non empty structs to GL_ARB_gl_spirv

[pmistryNV]

GLSL spec doesn't support empty structs. GLSL 4.6 section 4.1.8 has following statement to restrict the same: "Structures must have at least one member declaration". SPIR-V specification however allows empty structs.

GL_ARB_gl_spirv doesn't explicitly mention this restriction against GLSL frontend's to not allow empty structs in SPIRV. This restriction should be added to the extension to align it with GLSL specification.

[pdaniell-nv]

For additional reference for Khronos folks, this issue was discussed here https://gitlab.khronos.org/vulkan/vulkan/-/issues/2174 for Vulkan and here https://gitlab.khronos.org/spirv/SPIR-V/-/issues/163 for SPIR-V. Both of which decided to allow empty structs. That doesn't mean we should or need to for OpenGL, but some useful background reference material.

[seanbaxter]

I filed this for a bug fix and am getting feature removal instead. The comments describe the situation in detail: https://github.com/seanbaxter/segfault_nvidia2

C++ mandates true empty structs (ones with zero size) through the empty base optimization. It's used throughout the STL. In LLVM you just don't emit the empty base, because you can Bitcast the object's address to a pointer to the empty base class. But Logical addressing prohibits these bitcasts. I think the only way to support it on SPIR-V is by declaring an empty struct member for OpAccessChain to cling onto.

I could elide empty base classes entirely and error when an lvalue is requested. But in general, not supporting this layout breaks the C++ object model. It's hard to anticipate how much would break. Using templates to generate classes creates a lot of empty structs. Are we all cool with very common C++ idioms not being supported by OpenGL?

@dneto0 @johnkslang have weighed in here: https://github.com/gpuweb/gpuweb/issues/1222

I rely on OpenGL for compiler frontend work. Vulkan is way too unproductive for the prototyping and testing I do. These APIs were supposed to co-exist. I really can't use Amber because the single-source nature of the compiler makes its UBO and SSBO types opaque to the outside. NVIDIA is deprecating OpenGL when there's no suitable alternative. I'd roll with whatever changes they made if there was a spot to roll to, but there isn't right now.

[seanbaxter]

I changed the default behavior to emit a uint8_t member for all empty classes. Empty base class optimization bases are now elided entirely. Asking for an empty base class optimized subobject now returns the pointer to a special Private Variable. There's one instance per class type, created when that subobject access is made. If after all the CFG passes are don and there are still uses of this Private Variable, the compiler throws an error when creating the SPIR-V binary stream. This gives the compiler a lot more leeway in optimizing out the empty base class entirely. It can exist semantically, just not in the SPIR-V binary.

Calls to member functions on empty base classes should also be okay. The compiler already has to inline through functions that take non-Function implicit objects (due to Logical requirements). As long as the this pointer isn't actually stored in memory in a way that sroa/mem2reg can't free up, all uses of that poison Private Variable are likely removed prior to SPIR-V serialization.

In the general, non-C++ context, you can populate an empty struct with a uint8_t (except in this empty base class optimization context) to avoid this issue altogether. That should cover the scenario where you're using macros to conditionally populate a structure.

I can usually find workarounds within a few hours. I don't want to keep waiting a month or more after filing a bug to hear that engineering has decided on removal rather than a fix.

[pdaniell-nv]

We discussed this issue in the OpenGL/ES working group. It is unclear whether others support empty structs in SPIR-V for OpenGL and there don't appear to be any tests for it. It appears there are SPIR-V tests for empty UBO and SSBO structs in Vulkan CTS, added in https://github.com/KhronosGroup/VK-GL-CTS/commit/f18008951765ce4c12ee43f1efb6b0023607119b. It would be ideal if those tests can get ported to OpenGL.

[pdaniell-nv]

NVIDIA is no longer planning to push for this spec clarification. Supporting empty structs for SPIR-V is an important use case and we're planning to support it. There is an open issue to add OpenGL CTS coverage for it here: https://gitlab.khronos.org/Tracker/vk-gl-cts/-/issues/2768.