spirv-val: Add Vulkan Location VUID #4608
Conversation
a62967c to
afbea88
Compare
|
Something definitely seems wrong with this change. I haven't dug into it completely, but one of the errors being flagged in the glslang test suite is:
This shouldn't require a Location decoration. Only Input, Output and some ray tracing stages should use location. |
Agreed. Those VUIDs should be limited to applying to variables in certain storage classes (Input/Output is what I'm familiar with.) See Khronos-internal Vulkan issue 2737 |
|
SPV_NV_ray_tracing allows / requires Location decorations for variables in certain storage classes. |
| for (size_t j = 3; j < inst->operands().size(); ++j) { | ||
| const auto id = inst->word(inst->operand(j).offset); | ||
| desc.interfaces.push_back(id); | ||
| vstate->registerInterfaceVariable(id); |
There was a problem hiding this comment.
This is also capturing interface variables in raytracing-related storage classes.
SPV_NV_ray_tracing allows Location decorations on certain storage classes:
Add RayPayloadKHR, IncomingRayPayloadKHR, CallableDataKHR, and IncomingCallableDataKHR to the list of Storage Classes that Location is valid on.
The OpTraceNV instruction references payloads by numeric location. That's the clear motivation for decorating variables there with Location.
There are no Location-related validation rules described in the extension, or that I can find in the Vulkan spec related to the extension.
- the mechanism is changed: the payload is referenced by pointer instead of integer location
- there are no mentions of variables having Locations.
So I think the validator should only check locations for Input and Output variables.
| "of a structure type"; | ||
| } | ||
|
|
||
| if (spvIsVulkanEnv(vstate.context()->target_env)) { |
There was a problem hiding this comment.
This should only check location if the variable is Input or Output.
| %v4float = OpTypeVector %float 4 | ||
| %ptr_v4float = OpTypePointer Output %v4float | ||
| %fragCoord = OpVariable %ptr_v4float Output | ||
| %non_interface = OpVariable %ptr_v4float Output |
There was a problem hiding this comment.
This variable is unused. So it's not part of the interface of any entry point, and so it should not produce an error because it's not in scope of the rules for "user defined variable" shader interface.
| %fragCoord = OpVariable %outvar_ptr Output | ||
| %block = OpTypeStruct %vec3 | ||
| %invar_ptr = OpTypePointer Input %block | ||
| %non_interface = OpVariable %invar_ptr Input |
There was a problem hiding this comment.
Similarly, this is not used.
|
Replaced by #4664 |
Labeled and added tests for
Added logic to track interface variables to check for