KhronosGroup/SPIRV-Registry

Inconsistency regarding OpPtrAccessChain and storage classes

Hugobros3 opened this issue · 3 comments

There seems to be conflicting information regarding what pointers OpPtrAccessChain may be applied to:

The spec classifies values obtained from that instruction as "variable pointers" (2.2.2, page 19), and later in 2.16.1 it says variable pointers may only point to StorageBuffer and Workgroup storage classes.

But in the definition of OpPtrAccessChain, it is explicitly specified what the behavior is when using the Element operand to offset a pointer in the PushConstant storage class, and the behavior is also implied to be valid (but the offset impl-defined) for other storage classes too.

To compute the new element address, Element is treated as a signed count of elements E, relative to the original Base element B, and the address of element B + E is computed using enough precision to avoid overflow and underflow. For objects in the Uniform, StorageBuffer, or PushConstant storage classes, the element’s address or location is calculated using a stride, which will be the Base-type’s Array Stride if the Base type is decorated with ArrayStride. For all other objects, the implementation calculates the element’s address or location.

If I am reading your question correctly, why do you think you can't have a OpPtrAccessChain that is not a "variable pointer" and then it doesn't have the restrictions

variable pointers are putting the restrictions in place (on other parts of the spec as well), which I think is different from having "inconsistent OpPtrAccessChain information"

Section 2.2.2 defines a variable pointer as:

Variable pointer: A pointer of logical pointer type that results from one of the following instructions:
• OpSelect
• OpPhi
• OpFunctionCall
• OpPtrAccessChain // !
• OpLoad
• OpConstantNull

There are no further restrictions given, so that implies all results of OpPtrAccessChain are variable pointers, regardless of operands.

Furthermore, if the variable ptr capability is not declared, 2.16 excludes a logical pointer (which pointers within the Push Constant storage classe are) from being the result of OpPtrAccessChain:

If neither the VariablePointers nor VariablePointersStorageBuffer capabilities are declared, the following rules apply
to logical pointer types:

[...]

It is invalid for a pointer to be the Result <id> of any instruction other than:
* OpVariable
* OpAccessChain
* OpInBoundsAccessChain
* OpFunctionParameter
* OpImageTexelPointer
* OpCopyObject

This is a problem for at least one implementation: Mesa assumes this all means OpPtrAccessChain cannot operate on pointers into push constants, a while back I talked to people on IRC and they confirmed that was their interpretation too. I can also point to that assumption being baked in the code here, as nir_deref_instr_get_variable will return null when the deref chain is obtained from translating an OpPtrAccessChain operation to NIR.

My understanding is to get this fixed in Mesa will require clarification from the specification on where exactly OpPtrAccessChain is valid.

Khronos is discussing a clarification. OpPtrAccessChain with push constants is never intended to be a valid usage.