Validate location assignments

[alan-baker]

Validate that location, component and index decorations do not cause an overlap in any interface.

Checks:

• location can only be applied to variables or structure members
• variable that require a location have one
• index can only be on fragment outputs
• no conflicting location, component, index assignments
  • utility functions calculate the consumed locations and components

Tests include common corners such as a scalar being packed after 3-component 64-bit vector.

Tested against CTS.

[alan-baker]

The glslang tests appear to because the common test runner in glslang doesn't perform the IO mapping to actually assign auto-mapped locations.

[dneto0]

Looks really good.
I think the main issue is stripping away the arrayness needs to be conditional on stage and some other decorations.

[dneto0]

Cache the ... result?

[dneto0]

Should return an error if is not a constant?

[alan-baker]

That should be validated elsewhere. Also, the validator only really evaluates 32-bit integer constants so is_int would be false for other integer sizes.

[dneto0]

Should return an error here? E.g. pointer type.

[alan-baker]

That validation belongs on the variable, but it might currently be missing.

[dneto0]

nit: Could exit as soon as you see OpTypeVoid. That way we don't scan the entire module.

[dneto0]

Or should the validator error out if builtins have any kind of location annotation?

Vulkan 14.1.1: "Built-ins must not have any Location or Component decorations."

[alan-baker]

I thought about that, but I'm not sure this is the best place to do it. Ideally the validator would check for all levels of variable/struct and I didn't want to do all that searching here.

[dneto0]

This only applies to the ones that are required to be arrays:
tessellation control and mesh per-vertex shader outputs
tessellation evaluation, tessellation control, geometry shader per-vertex inputs.

See https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#interfaces-iointerfaces-matching

[dneto0]

For "mesh per-vertex output", that means on Task shader the variables with PerTaskNV decoration.
http://htmlpreview.github.io/?https://github.com/KhronosGroup/SPIRV-Registry/blob/master/extensions/NV/SPV_NV_mesh_shader.html

For the inputs cases noted above, see GLSL 4.5:

Tessellation control, evaluation, and geometry shader input variables get the per-vertex values written out by output variables of the same names in the previous active shader stage. For these inputs, centroid and interpolation qualifiers are allowed, but have no effect. Since tessellation control, tessellation evaluation, and geometry shaders operate on a set of vertices, each input variable (or input block, see interface blocks below) needs to be declared as an array.

[dneto0]

For tessellation control outputs, outputs are partitioned into per-vertex and per-patch. Per-patch variables are decorated with Patch. Per-vertex variables are always arrayed, and an invocation can only write to its own lane, i.e. with index gl_InvocationID.

Example:

#version 460

layout(vertices=5) out;

layout(location=0) out uint arr[];
layout(location=1) patch out uint count;

void main() {
  const uint c = arr.length();
  count = c;
  arr[gl_InvocationID] = c;
}

In the SPIR-V. arr is an array and c is not:

               OpCapability Tessellation
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint TessellationControl %main "main" %count %arr %gl_InvocationID
               OpExecutionMode %main OutputVertices 5
               OpSource GLSL 460
               OpName %main "main"
               OpName %count "count"
               OpName %arr "arr"
               OpName %gl_InvocationID "gl_InvocationID"
               OpDecorate %count Patch
               OpDecorate %count Location 1
               OpDecorate %arr Location 0
               OpDecorate %gl_InvocationID BuiltIn InvocationId
       %void = OpTypeVoid
          %3 = OpTypeFunction %void
       %uint = OpTypeInt 32 0
%_ptr_Output_uint = OpTypePointer Output %uint
      %count = OpVariable %_ptr_Output_uint Output
     %uint_5 = OpConstant %uint 5
%_arr_uint_uint_5 = OpTypeArray %uint %uint_5
%_ptr_Output__arr_uint_uint_5 = OpTypePointer Output %_arr_uint_uint_5
        %arr = OpVariable %_ptr_Output__arr_uint_uint_5 Output
        %int = OpTypeInt 32 1
%_ptr_Input_int = OpTypePointer Input %int
%gl_InvocationID = OpVariable %_ptr_Input_int Input
       %main = OpFunction %void None %3
          %5 = OpLabel
               OpStore %count %uint_5
         %16 = OpLoad %int %gl_InvocationID
         %17 = OpAccessChain %_ptr_Output_uint %arr %16
               OpStore %17 %uint_5
               OpReturn
               OpFunctionEnd

[alan-baker]

Yes, what I implemented is slightly more permissive (since variables will consume fewer locations).

[alan-baker]

I've made a bunch of changes related to this. The validator now accounts for these implicitly arrayed types and also accounts for component decorations getting used on arrayed types to interleave variables in the location component space. @dneto0 PTAL.

[dneto0]

Thanks for your patience.
LGTM

[alan-baker]

@dneto0 this latest patch fixes up some arrayed interfaces checks after digging into the remaining glslang failures.