Support dynamic indexing of by-value matrices and arrays, per WGSL

gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.

https://wgpu.rs

Apache License 2.0

12.03k stars 873 forks source link

Support dynamic indexing of by-value matrices and arrays, per WGSL #4337

Open jimblandy opened 3 years ago

jimblandy commented 3 years ago

WGSL permits dynamically indexing matrices that are not stored in variables:

A function parameter may be a matrix.
A formal parameter expression does not evaluate to a pointer, but rather to the passed value.
A matrix access expression does not require a pointer to the matrix.

At present, Naga does not permit this: typifier::ResolveContext::resolve only permits indexing matrices behind pointers. This is in part because the only SPIR-V instructions that can take dynamic indices operate on pointers, not values. (The WGSL spec description of matrix access expressions mentions the SPIR-V OpCompositeAccess, which isn't sufficient for the job; filed as gpuweb/gpuweb#1782.)

We solved the analogous problem for arrays in gfx-rs/naga#723 by moving the value to a temporary variable, obtaining a pointer to that, and then indexing the pointer. It seems like the same tactic would work here.

JCapucho commented 2 years ago

https://github.com/gpuweb/gpuweb/pull/1801 removed this, so all dynamic accesses from wgsl must happen from a reference now which means that no local will be needed, closing.

jimblandy commented 1 year ago

The committee reversed its position, and the ability to index matrices and arrays dynamically was re-added in gpuweb/gpuweb#2427.

cshenton commented 8 months ago

Is there a time frame on fixing this? It is an enormous performance killer. Without this, we have to mark dynamically indexed arrays as var, which pulls them into per-thread registers, causing register pressure which kills SM occupancy.

cwfitzgerald commented 8 months ago

Are you seeing different codegen from this? I wouldn't have expected the keyword you use to affect anything once the underlying compiler got to it.

nical commented 7 months ago

This blocks a fair amount of the shaders at https://webgpufundamentals.org/ Bugzilla entry: https://bugzilla.mozilla.org/show_bug.cgi?id=1830763

ErichDonGubler commented 2 months ago

Another bug blocked by this on the Firefox side: https://bugzilla.mozilla.org/show_bug.cgi?id=1878323

jimblandy commented 1 month ago

I've increased this issue's stack rank.

We'll want to crib from Dawn for this.

ErichDonGubler commented 1 week ago

Enough things are getting reported as blocked on the Firefox side that I made a bug entry to track it there, too: https://bugzilla.mozilla.org/show_bug.cgi?id=1913424