-
Notifications
You must be signed in to change notification settings - Fork 193
[spv-out] implement array value indexing #723
Conversation
Here is RDNA2 assembly for the
|
@acowley it's a bit hard to read (for me). It appears that the driver has put the array into internal memory ( |
That’s great! How do nested accesses look? Is using |
I tried nested accesses on the following WGSL: [[stage(vertex)]]
fn foo([[builtin(vertex_index)]] vi: u32) -> [[builtin(position)]] vec4<f32> {
let array = array<array<i32, 2>, 2>(array<i32, 2>(1, 2), array<i32, 2>(3, 4));
let value = array[vi][vi];
return vec4<f32>(vec4<i32>(value));
} It generates the following RDNA2 assembly:
Can't say it makes sense to me, but the assembly has exactly 2 memory loads, as we'd optimally expect in this case. Edit; what's most confusing to me is that these accesses don't look dependent on each other. |
The PR is now updated to have our snapshot test |
@cwfitzgerald noted that there is only vi=0 and vi=1 cases. I modified the test to have 9 different paths now // This snapshot tests accessing various containers, dereferencing pointers.
[[stage(vertex)]]
fn foo(
[[builtin(vertex_index)]] vi: u32,
[[builtin(instance_index)]] ii: u32,
) -> [[builtin(position)]] vec4<f32> {
let array = array<array<i32, 3>, 3>(
array<i32, 3>(1, 2, 3),
array<i32, 3>(4, 5, 6),
array<i32, 3>(7, 8, 9)
);
let value = array[ii][vi];
return vec4<f32>(vec4<i32>(value));
} RDNA2 assembly:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Some minor suggestions.
gfx-rs/naga#723 back from dead Co-authored-by: Dzmitry Malyshau <[email protected]> Signed-off-by: sagudev <[email protected]>
gfx-rs/naga#723 back from dead Co-authored-by: Dzmitry Malyshau <[email protected]> Signed-off-by: sagudev <[email protected]>
gfx-rs/naga#723 back from dead Co-authored-by: Dzmitry Malyshau <[email protected]> Signed-off-by: sagudev <[email protected]>
gfx-rs/naga#723 back from dead Co-authored-by: Dzmitry Malyshau <[email protected]> Signed-off-by: sagudev <[email protected]>
Bring gfx-rs/naga#723 back from the dead. Signed-off-by: sagudev <[email protected]> Co-authored-by: Dzmitry Malyshau <[email protected]> Co-authored-by: Jim Blandy <[email protected]>
Bring gfx-rs/naga#723 back from the dead. Signed-off-by: sagudev <[email protected]> Co-authored-by: Dzmitry Malyshau <[email protected]> Co-authored-by: Jim Blandy <[email protected]>
Fixes #635
Alternative to #669
This is a quick and dirty solution for array indexing. It unconditionally creates a function-local variable, stores the whole array in there, then indexes with
OpAccessChain
, and then loads the result back. This is likely not the most efficient way to go in a lot of situations, but it works. Also, there is a fairly good chance that LLVM optimizer in the driver will see the variable and eliminate it if the HW supports indexing directly. It would be amazing if we could verify this.