Add 'Memory Size' field to DebugTypeVector

[LU-JOHN]

Add 'Memory Size' field to DebugTypeVector

[MrSidims]

Since it will affect OpenCL compilation flow and with a further extend SPIR-V to LLVM IR translator, @svenvh and @StuartDBrady may I ask you to take a look?

[alan-baker]

@baldurk would RenderDoc handle seeing this unexpected operand without issue?

A concern here is version information is not conveyed in the SPIR-V binary (nor does this change increment the revision of the spec). So it would be reasonable for a tool to reject this module until it is updated. Have you tried parsing/disassembling a module with this extra operand in an un-updated SPIRV-Tools?

[Keenuts]

@alan-baker: just tried, patched spirv-as to assemble a SPIR-V module with the new operand to DebugTypeVector, and captured a trace using renderdoc. Seems like renderdoc accepeted the shader and doesn't crash.

[baldurk]

This should be fine for RenderDoc at least, since my reader would allow any instruction to have extra words in its wordcount - I don't think that could safely be assumed to be generally true of all consumers though. Note that to fully test a change like this you'd need to debug the shader not just load a capture as the debug info isn't processed until that point.

That said, I don't expect RenderDoc would ever make use of this information since it seems to be a quirk of CL where vectors are not just aligned but actively sized to 16 bytes with padding bytes, so this property is to represent that when it's otherwise not surfaced?

[bashbaug]

Discussed in the May 8th teleconference. Are there any cases where the size of a three-component vector is NOT padded to a size that is a power of two? If not, do we really need to add an explicit "memory size" field, or can this be assumed implicitly?

[LU-JOHN]

Discussed in the May 8th teleconference. Are there any cases where the size of a three-component vector is NOT padded to a size that is a power of two? If not, do we really need to add an explicit "memory size" field, or can this be assumed implicitly?

For clang at least it looks like this can be assumed. From https://clang.llvm.org/docs/LanguageExtensions.html#vectors-and-extended-vectors:

The size and alignment are both the number of bits rounded up to the next power of two

The testcase that motivated this had 832 elements of 32-bits that was rounded up to 32768.

[MrSidims]

Are there any cases where the size of a three-component vector is NOT padded to a size that is a power of two

I believe not.

or can this be assumed implicitly

From the translator and OpenCL runtime perspective it should work. The real question would be: do we want to put a note about padding in SPIR-V Debug Info specification and if no - why? :)

[LU-JOHN]

In https://docs.vulkan.org/guide/latest/shader_memory_layout.html, a vec3 can take 12 bytes of memory with "scalar alignment". This can be seen in "Alignment Example 3" and "Alignment Example 4".

@baldurk Do the differences in alignment std140/std430 and scalar make a difference to RenderDoc? Currently the translator automatically pads 3-element vectors to have a memory size of 4-elements, which is a mismatch with Vulkan's scalar alignment.

[baldurk]

vec3s in vulkan (and opengl for that matter) always take 12 bytes no matter what, even with plain std140. The only thing that changes is the alignment requirement for them (being either 4 or 16 bytes). This issue is about an OpenCL behaviour where vec3s actually take 16 bytes from what I understand above.

RenderDoc cares about the alignment of values but that information doesn't come from the debug info since it's part of the SPIR-V module's external interface. None of this information is used by RenderDoc and I wouldn't expect it to be used in future since the memory layout is specified by SPIR-V directly already. I don't expect RenderDoc will ever support OpenCL so any changes that are only for kernel SPIR-V modules shouldn't affect it.

[LU-JOHN]

"Memory Size" can be calculated with (element size)*(bit_ceil(# elements)). Adding 'Memory Size' field to DebugTypeVector not needed.