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

SPV_EXT_physical_storage_buffer aliasing decorations #93

Open
seanbaxter opened this issue Jan 14, 2021 · 5 comments
Open

SPV_EXT_physical_storage_buffer aliasing decorations #93

seanbaxter opened this issue Jan 14, 2021 · 5 comments
Labels
question Further information is requested

Comments

@seanbaxter
Copy link

Why are the aliasing/restrict decorations supposed to go on OpVariable, and how does that work? You can eliminate all your local OpVariables through mem2reg. My pointers are loaded from a PushConstant, and don't even have an OpVariable. What is their aliasing behavior?

It makes more sense to make aliasing a storage class: PhysicalStorageRestrict and PhysicalStorageAliased. This way it won't fall off under CFG transformations.

@johnkslang johnkslang added the question Further information is requested label Jan 19, 2021
@Hugobros3
Copy link

Hugobros3 commented Mar 5, 2021

My pointers are loaded from a PushConstant, and don't even have an OpVariable

How does this work though ? The way you declare push constants is by making them a global OpVariable which is a logical pointer to your push constant data, using the PushConstant storage class. Mem2Reg obviously can't touch those as the implication is those are "externally" provided.

Edit: I got that bit wrong, see later posts

I see the issue you're getting at though, and it's pretty bad: the memory models available to shaders do not allow expressing potential aliasing outside of global inputs without having to go through an unnecessary OpVariable store/load.

It makes more sense to make aliasing a storage class: PhysicalStorageRestrict and PhysicalStorageAliased. This way it won't fall off under CFG transformations.

What about just having the Alias/Restrict decorations on pointer types themselves ? That would make most sense since the decorations are already present on pointer types and just have this now irrelevant restriction to be used only in the context of memory object declarations.

With physical storage buffers we can effectively reach arbitrary memory regions beyond simply the shader inputs, so the whole concept of "memory object declaration" falls apart. The issue seems to extend to OpenCL flavours too, this seems to me like a very messy part of the spec.

@seanbaxter
Copy link
Author

seanbaxter commented Mar 5, 2021

The way you declare push constants is by making them a global OpVariable which is a logical pointer to your push constant data, using the PushConstant storage class. Mem2Reg obviously can't touch those as the implication is those are "externally" provided.

The OpVariable is for the push constant or UBO, not for the pointer member. You need to do a Function Variable, copy AccessChain and load the pointer into that, and mark that Function Variable with Restrict. But the goal of M2R is to reduce all Function Variables. This is worse when casting longs to pointers, because there is no Variable in that picture.

@Hugobros3
Copy link

Hugobros3 commented Mar 5, 2021

Still I don't see the point of another 2 storage classes when we can already decorate pointer types to encode this info, the issue of aliasing is orthogonal to the storage class, any storage class with physical pointers has the potential for aliasing. In OpenCL kernels, the issue would persist as the storage classes are different, for example.

I was confused - see later post

@seanbaxter
Copy link
Author

But we can't decorate pointer types to encode this information. I wish we could! We can only decorate Variables. That's weird.

@Hugobros3
Copy link

Hugobros3 commented Mar 5, 2021

Ah you're right - I misread your post

In the issue I linked, glslang decorates struct type members with Restrict (illegally, this is out of spec, but spirv-val doesn't seem to catch puting Restrict/Aliasing decorations on random variables). Apparently that confused me into thinking we could decorate the types themselves. I maintain this would be the cleaner solution though.

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

No branches or pull requests

3 participants