Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PtrAccessChain with a PhysicalStorageBuffer Base #152

Open
nanokatze opened this issue Jul 2, 2022 · 1 comment
Open

PtrAccessChain with a PhysicalStorageBuffer Base #152

nanokatze opened this issue Jul 2, 2022 · 1 comment

Comments

@nanokatze
Copy link

nanokatze commented Jul 2, 2022

Description of PtrAccessChain seems to say nothing about the case when the Base belongs to PhysicalStorageBuffer class.

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.

I believe the intent was to use ArrayStride with PhysicalStorageBuffer Base as well? In practice spirv-val seems to be happy with PhysicalStorageBuffer Base that doesn't have ArrayStride decoration. Of drivers' compilers, NIR expects an explicit stride, blowing up on an assert otherwise, and Nvidia's seems to work as if stride was size of the pointee or 1 (the pointee size this was tested with was int8, so it's unknown whether it's the former or the latter.)

@jeffbolznv
Copy link
Contributor

I think this was probably an oversight and OpPtrAccessChain should include PhysicalStorageBuffer in its list.

For NVIDIA's implementation, I believe we use the array stride if it's present, otherwise we fall back to the size of the element type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants