search

KhronosGroup / Vulkan-Docs

The Vulkan API Specification and related tools
Other
2.74k stars 460 forks source link

Problematic VK_AMDX_shader_enqueue VUIDs #2303

Open Rua opened 6 months ago

Rua commented 6 months ago

The spec contains the following four VUIDs:

VUID-RuntimeSpirv-maxExecutionGraphShaderPayloadSize-09193 Variables declared in the NodePayloadAMDX storage class must not be larger than the maxExecutionGraphShaderPayloadSize limit

VUID-RuntimeSpirv-maxExecutionGraphShaderPayloadSize-09194 Variables declared in the NodeOutputPayloadAMDX storage class must not be larger than the maxExecutionGraphShaderPayloadSize limit

VUID-RuntimeSpirv-maxExecutionGraphShaderPayloadSize-09195 For a given entry point, the sum of the size of any variable in the NodePayloadAMDX storage class, and the combined size of all statically initialized variables in the NodeOutputPayloadAMDX storage class must not be greater than maxExecutionGraphShaderPayloadSize

VUID-RuntimeSpirv-maxExecutionGraphShaderPayloadCount-09196 Shaders must not statically initialize more than maxExecutionGraphShaderPayloadCount variables in the NodeOutputPayloadAMDX storage class

Statically initialized

VUIDs 09195 and 09196 make mention of statically initialized variables. However, the SPIR-V spec for the NodeOutputPayloadAMDX storage class states:

Variables declared with this storage class are read-write, must not have initializers, and must be initialized with OpInitializeNodePayloadsAMDX before they are accessed.

Therefore, it seems impossible to have a variable in this storage class that is statically initialized. Alternatively, if an initialization call to OpInitializeNodePayloadsAMDX is what's really intended here (which I would not call static, i.e. compile time!), it would be clearer if these VUIDs said "initialized by OpInitializeNodePayloadsAMDX" instead of "statically initialized".

Redundancy

VUID 09193 is completely redundant to 09195. If the sum of all NodePayloadAMDX variable sizes is within the limit, then that implies that any single NodePayloadAMDX variable size must also be within the limit. 09194 may also be redundant, if 09195 is taken to cover all NodeOutputPayloadAMDX variables and not only the initialized ones (whatever that means, after the first point is resolved).

oddhack commented 6 months ago

@tobski assigning to you as a vendor-specific issue (unless @spencer-lunarg wants to tap in as a VVL issue).