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

Storage buffers in binding arrays aren't writable #1864

Closed
jimblandy opened this issue Apr 26, 2022 · 4 comments · Fixed by #2282
Closed

Storage buffers in binding arrays aren't writable #1864

jimblandy opened this issue Apr 26, 2022 · 4 comments · Fixed by #2282
Labels
area: middle Intermediate representation kind: bug Something isn't working

Comments

@jimblandy
Copy link
Member

Since indexing a BufferArray never produces a pointer, it's impossible to write to the contents of buffers accessed through a buffer array.

Permitting pointers to buffer arrays seems like a bad idea. Perhaps indexing a BufferArray whose base type is not Image or Sampler should just always produce a pointer to the buffer's value, as if it were an ordinary var<storage> or var<uniform>, even when the base operand to Access or AccessIndex is not a pointer.

cc @cwfitzgerald

@cwfitzgerald
Copy link
Member

Yeah I agree with the "just pointerize the result" solution. I was intentionally trying to avoid funky pointer stuff (like a pointer to an image in a binding array might lead to some funky consequences I don't understand).

@jimblandy
Copy link
Member Author

I agree we want to avoid introducing pointers to new things. I think you'd say, for binding_array<texture> and binding_array<sampler>, indirection doesn't produce a pointer, but for everything else, it does.

@teoxoy teoxoy added kind: bug Something isn't working area: middle Intermediate representation labels Apr 29, 2022
@jimblandy
Copy link
Member Author

There's also a language-level problem here. For buffers, we distinguish between read-only and read/write storage variables with:

var<storage> read_only_storage: i32;
var<storage, read> more_read_only_storage: i32;
var<storage, read_write> writable_storage: i32;

But since all buffer_array<Data> variables must go in the handle address space, there's no syntactic opportunity to say whether the variable is writable or not. (There's also no such opportunity in the IR.)

@kvark
Copy link
Member

kvark commented Apr 12, 2023

Note that this problem doesn't exist today because you can't actually use uniform/storage buffers in binding arrays.
#2282 makes it possible, and it uses the same address spaces as you'd use for non-arrays. So the problem is solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: middle Intermediate representation kind: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants