[SPIRV] SPIR-V module layout doesn't respect the spec.

[Keenuts]

The layout of a SPIR-V module is very strict (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_logical_layout_of_a_module).

Example:

• An OpVariable instruction cannot be present outside of the first block of a function.

As-is, the backend doesn't respect those rules, but lower IR instructions to SPIR-V in place.
This means the following LLVM IR won't build to a valid SPIR-V module:

define void @main() #1 {
entry:
  %0 = alloca <2 x i32>, align 4
  %1 = getelementptr <2 x i32>, ptr %0, i32 0, i32 0
  %2 = alloca float, align 4
  ret void
}

attributes #1 = { "hlsl.numthreads"="4,8,16" "hlsl.shader"="compute" }

[Keenuts]

@michalpaszkowski FYI

[Sh0g0-1758]

Hey @Keenuts, I can work on this. In case you have not worked on this, can you give me some pointers to get started.

[Keenuts]

Hi,

Sure, I haven't started working on it, so feel free to start!
The first step might be to look at the SPIR-V instructions that needs ordering (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_logical_layout_of_a_module), and check which ones are already ordered.

• The SPIRVAsmPrinter already has some logic to correctly order the module sections.
• I identified OpVariable as a candidate, but I don't know if that's the only one.

As for how to fix the ordering, I haven't thought about it much, but top of my head, we have 2 options:

• reorder the LLVM-IR to move every alloca to the start of the first BB.
• modify the printer to output all variables first, then the other instructions.
  I'd prefer to reorder the LLVM-IR, so the serializer remains simply a serializer, not something reordering instructions.

But we we reorder the IR, we need to handle VLAs:

• if alloca uses a register for the size, we cannot simply move it to the top.

For HLSL, that's fine, as VLA are not allowed.
For OpenCL, It looks like VLA are not allowed either.

But since the backend has support for this SPV_INTEL_variable_length_array extension, there might be a case where the IR could have a valid VLA using a register.
In such case, I'm not sure what the SPIR-V should look like.
@michalpaszkowski do you happen to know in what scenario is SPV_INTEL_variable_length_array used? I don't see it in https://github.com/KhronosGroup/SPIRV-Registry/tree/main/extensions/INTEL

[michalpaszkowski]

Thank you @Keenuts and @VyacheslavLevytskyy for the fix! I see the solution in PR87554 handles the problem in the selector.

@michalpaszkowski do you happen to know in what scenario is SPV_INTEL_variable_length_array used? I don't see it in https://github.com/KhronosGroup/SPIRV-Registry/tree/main/extensions/INTEL

You can find the spec for SPV_INTEL_variable_length_array in the Intel's DPC++ repository. I am not very much familiar with the use of this extension outside of SYCL/DPC++ (if there is any).

This extension allows to allocate local arrays whose number of elements is unknown at compile time. This is useful for implementing high-level language features like C99 variable length arrays.

[Keenuts]

Fixed with #66261

[Keenuts]

You can find the spec for SPV_INTEL_variable_length_array in the Intel's DPC++ repository. I am not very much familiar with the use of this extension outside of SYCL/DPC++ (if there is any).

Thanks! Looks like this doesn't use OpVariable, hence seems like VLA won't be an issue for OpVariable, so this fix should be enough 😊