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

[WGSL] Proposal: Default Struct Layout #1393

Closed
ben-clayton opened this issue Jan 30, 2021 · 24 comments · Fixed by #1447
Closed

[WGSL] Proposal: Default Struct Layout #1393

ben-clayton opened this issue Jan 30, 2021 · 24 comments · Fixed by #1447
Labels
wgsl WebGPU Shading Language Issues
Projects

Comments

@ben-clayton
Copy link
Collaborator

ben-clayton commented Jan 30, 2021

Internally at Google @dj2, @kainino0x, @dneto0, @alan-baker and I have been discussing struct layout ideas. We have (mostly) converged on this proposal.

If the wider group backs this proposal, next steps would be to agree on the mandatory layout rules for each storage type.


WGSL Struct Alignment Proposal

Design principles

  1. Provides a single default layout for any host-shareable datatype (primitive, structure, array).

    The layout is always valid for storage buffers.
    The rules are practical - not too wasteful.
    The rules are forward-looking - std140 / “extended” is considered legacy.

  2. Structure layout is defined in terms of “size” and “layout” with no additional rules. The offset of any field is always exactly:

    roundUp(alignment field, offset previous field + size previous field).

  3. User defined data types and individual structure fields may diverge from the default layout, by using annotations to override the default alignment, size, and array stride values.

    [[align(n), size(n)]] can be decorated on user-defined types and structure fields.
    [[stride(n)]] on array types.

    In most cases, this is required to make a datatype conform to the restrictions of uniform buffers.
    It can also be used to specify layouts denser than the default, e.g. to use with future optional capabilities (e.g. “scalar” layout).

  4. Rules for valid uniform/storage buffer layouts are defined separately from the algorithm that defines the default layout

    This proposal intentionally does not define the layout rules for the various storage classes, but does demonstrate how more restrictive storage class layouts are supported.

Proposal

Default alignments and sizes

Every data type has a default alignment and size value. When [[size(n)]] or [[align(n)]] decorations aren’t specified for the type, they’re the same as the Align(S, storage) and Extent(V, storage) in the current WGSL draft spec.

Default alignment and size for each WGSL data type:

Type Alignment (bytes) Size (bytes)
f32 4 4
i32 4 4
u32 4 4
vec2 8 8
vec3 16 12
vec4 16 16
[[stride(S)]]
array<T, N>
alignof(T) N * S
[[stride(S)]]
array<T>
alignof(T) undefined
matXxY (col-major) alignof(vecY) sizeof(array<vecY, X>)
mat2x2 8 16
mat3x2 8 24
mat4x2 8 32
mat2x3 16 32
mat3x3 16 48
mat4x3 16 64
mat2x4 16 32
mat3x4 16 48
mat4x4 16 64
struct S max of field alignments roundUp(alignof(S), offsetof(last field) + sizeof(last field))

Note: A matrix is laid out so that a program can form a reference to each of its column vectors, and that reference (pointer) is like a reference to a regular column vector.
In particular, the column vector must be aligned.

Structures and type aliases may have their defaults overridden by using the [[size(n)]] or [[align(n)]] decorations on their declaration:

Example:

// Override default size and alignment
[[size(16), align(32)]]
struct S {
  u32 i;
};

// Override default size, keep default alignment
type RGBX = [[size(16)]] vec3<f32>;

Default array stride

Type Default stride (bytes)
array<T, N> roundUp(alignof(T), sizeof(T))
array<T> roundUp(alignof(T), sizeof(T))

Structure Layouts

The rules for aligning a structure field:

  • A field uses the alignment and size of its type, unless the field is explicitly annotated with [[align(n)]] and / or [[size(n)]] decorations, in which case these field decorations take precedence.
  • The first field is always at offset 0 of the structure.
  • Subsequent fields have an offset that has the byte offset: roundUp(alignment field, offset previous field + size previous field).
  • The default alignment of a structure is the same as the field with the largest alignment.
  • The default size of a structure is equal to: roundUp(alignof(S), offsetof(last field) + sizeof(last field))
  • Structures nested in other structures behave the same as any other field type.

Default Structure Layout Example

/*             align(8)  size(24)  */ [[block]] struct A {
/* offset(0)   align(4)  size(4)   */     u : f32;
/* offset(4)   align(4)  size(4)   */     v : f32;
/* offset(8)   align(8)  size(8)   */     w : vec2<f32>;
/* offset(16)  align(4)  size(4)   */     x : f32;
/* offset(20)  align(1)  size(4)          -- implicit struct size padding -- */
                                      };

/*             align(16) size(160) */ [[block]] struct B {
/* offset(0)   align(8)  size(8)   */     a : vec2<f32>;
/* offset(8)   align(1)  size(8)          -- implicit field alignment padding -- */
/* offset(16)  align(16) size(12)  */     b : vec3<f32>;
/* offset(28)  align(4)  size(4)   */     c : f32;
/* offset(32)  align(4)  size(4)   */     d : f32;
/* offset(36)  align(1)  size(12)         -- implicit field alignment padding -- */
/* offset(40)  align(8)  size(24)  */     e : A;
/* offset(64)  align(16) size(12)  */     f : vec3<f32>;
/* offset(76)  align(1)  size(4)          -- implicit field alignment padding -- */
/* offset(80)  align(8)  size(72)  */     g : [[stride(24)]] array<A, 3>;
/* offset(152) align(4)  size(4)   */     h : 32;
/* offset(156) align(1)  size(4)          -- implicit struct size padding -- */
                                      };

[[group(0), binding(0)]]
var<storage> storage_buffer : B;

Example translated to GLSL

Observe that:

  • struct A's size is padded to 24 bytes (roundUp(20, 8))
  • struct A's alignment is equal to the field with the largest alignment (w)
  • c, packs into end of the vec3 of b.
  • g has the alignment of the array element (A)

Note: Using the fixed type alignment and sizes of this initial proposal, scalars can be automatically packed into the end of a vector, not the front. If layout rules allow for scalar-aligned vectors, then you can pack-front by using an explicit [[align(4)]] decoration on the vector.

The default layout rules will always produce layouts that are valid for storage buffers (without requiring explicit [[align(n)]] / [[size(n)]] decorations).
The same may not be true for non-storage buffers.

Conforming to more constrained storage classes

Some storage classes are (at least initially) likely to impose tighter constraints on structure layouts than the default layout.

It is the developer's responsibility to ensure that structure layouts are valid for all storage class uses.

Field offsets and structure sizes that are invalid for a storage class that use that structure will result in a compilation error.

For example, a uniform storage class might have a stricter set of layout rules to the default layout.
If we used the struct B definition from the above in a uniform buffer, we may encounter the following compilation errors:

/*             align(8)  size(24)  */ [[block]] struct A {
╭╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╮
╎                                     ^^^^^^^^^^^^^^^^^^^^                     ╎
╎ error: struct A size is 24 bytes.                                            ╎
╎ Uniform buffers require structure sizes to be a multiple of 16 bytes.        ╎
╰╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╯
/* offset(0)   align(4)  size(4)   */     u : f32;
/* offset(4)   align(4)  size(4)   */     v : f32;
/* offset(8)   align(8)  size(8)   */     w : vec2<f32>;
/* offset(16)  align(4)  size(4)   */     x : f32;
/* offset(20)  align(1)  size(4)          -- implicit struct size padding -- */
                                      };

/*             align(16) size(160) */ [[block]] struct B {
/* offset(0)   align(8)  size(8)   */     a : vec2<f32>;
/* offset(8)   align(1)  size(8)          -- implicit field alignment padding -- */
/* offset(16)  align(16) size(12)  */     b : vec3<f32>;
/* offset(28)  align(4)  size(4)   */     c : f32;
/* offset(32)  align(4)  size(4)   */     d : f32;
/* offset(36)  align(1)  size(12)         -- implicit field alignment padding -- */
/* offset(40)  align(8)  size(24)  */     e : A;
╭╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╮
╎                                         ^^^^^^                               ╎
╎ error: Structure field has byte offset 40.                                   ╎
╎ Uniform buffers require fields of structure types to be 16 byte aligned.     ╎
╰╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╯
                                          ...
                                      };

[[group(0), binding(0)]]
var<uniform> uniform_buffer : B;

The developer can solve these compilation errors in a number of ways:

(1) Using [[align]] and [[size]] on struct

                                    ╭╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╮
                                    ╎ [[align(16), size(32)]] ╎
                                    ╰╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╯
/*             align(16) size(32)  */ [[block]] struct A {
/* offset(0)   align(4)  size(4)   */     u : f32;
/* offset(4)   align(4)  size(4)   */     v : f32;
/* offset(8)   align(8)  size(8)   */     w : vec2<f32>;
/* offset(16)  align(4)  size(4)   */     x : f32;
/* offset(20)  align(1)  size(12)         -- implicit struct size padding -- */
                                      };

                                    ╭╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╮
                                    ╎ [[align(16), size(208)]] ╎
                                    ╰╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╯
/*             align(16) size(208) */ [[block]] struct B {
/* offset(0)   align(8)  size(8)   */     a : vec2<f32>;
/* offset(8)   align(1)  size(8)          -- implicit field alignment padding -- */
/* offset(16)  align(16) size(12)  */     b : vec3<f32>;
/* offset(28)  align(4)  size(4)   */     c : f32;
/* offset(32)  align(4)  size(4)   */     d : f32;
/* offset(36)  align(1)  size(12)         -- implicit field alignment padding -- */
/* offset(48)  align(16) size(32)  */     e : A;
/* offset(80)  align(16) size(12)  */     f : vec3<f32>;
/* offset(92)  align(1)  size(4)          -- implicit field alignment padding -- */
/* offset(96)  align(16) size(96)  */     g : [[stride(32)]] array<A, 3>;
/* offset(192) align(4)  size(4)   */     h : 32;
/* offset(196) align(1)  size(12)         -- implicit struct size padding -- */
                                      };

[[group(0), binding(0)]]
var<uniform> uniform_buffer : B;

(2) Using [[align]] and [[size]] on fields

/*             align(8)  size(32)  */ [[block]] struct A {
/* offset(0)   align(4)  size(4)   */     u : f32;
/* offset(4)   align(4)  size(4)   */     v : f32;
/* offset(8)   align(8)  size(8)   */     w : vec2<f32>;
                                        ╭╌╌╌╌╌╌╌╌╌╌╌╌╌╌╮
/* offset(16)  align(4)  size(16)   */[[size(16)]] ╎ x: f32;
                                        ╰╌╌╌╌╌╌╌╌╌╌╌╌╌╌╯
                                      };

/*             align(16) size(208) */ [[block]] struct B {
/* offset(0)   align(8)  size(8)   */     a : vec2<f32>;
/* offset(8)   align(1)  size(8)          -- implicit field alignment padding -- */
/* offset(16)  align(16) size(12)  */     b : vec3<f32>;
/* offset(28)  align(4)  size(4)   */     c : f32;
/* offset(32)  align(4)  size(4)   */     d : f32;
/* offset(36)  align(1)  size(12)         -- implicit field alignment padding -- */
                                        ╭╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╮
/* offset(48)  align(16) size(32)  */[[align(16)]] ╎ e : A;
                                        ╰╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╯
/* offset(80)  align(16) size(12)  */     f : vec3<f32>;
/* offset(92)  align(1)  size(4)          -- implicit field alignment padding -- */
/* offset(96)  align(8)  size(96)  */     g : [[stride(32)]] array<A, 3>;
/* offset(192) align(4)  size(4)   */     h : 32;
/* offset(196) align(1)  size(12)         -- implicit struct size padding -- */
                                      };

[[group(0), binding(0)]]
var<uniform> uniform_buffer : B;

(3) Using padding fields

/*             align(8)  size(32)  */ [[block]] struct A {
/* offset(0)   align(4)  size(4)   */     u : f32;
/* offset(4)   align(4)  size(4)   */     v : f32;
/* offset(8)   align(8)  size(8)   */     w : vec2<f32>;
/* offset(16)  align(4)  size(4)   */     x : f32;
                                        ╭╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╮
/* offset(20)  align(4)  size(12)  */   ╎ _ : array<f32, 3>; ╎
                                        ╰╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╯
                                      };

/*             align(16) size(208) */ [[block]] struct B {
/* offset(0)   align(8)  size(8)   */     a : vec2<f32>;
/* offset(8)   align(1)  size(8)          -- implicit field alignment padding -- */
/* offset(16)  align(16) size(12)  */     b : vec3<f32>;
/* offset(28)  align(4)  size(4)   */     c : f32;
/* offset(32)  align(4)  size(4)   */     d : f32;
/* offset(36)  align(1)  size(12)         -- implicit field alignment padding -- */
/* offset(48)  align(16) size(32)  */     e : A;
/* offset(80)  align(16) size(12)  */     f : vec3<f32>;
/* offset(92)  align(1)  size(4)          -- implicit field alignment padding -- */
/* offset(96)  align(8)  size(96)  */     [[stride(32)]] array<A, 3>;
/* offset(192) align(4)  size(4)   */     h : 32;
/* offset(196) align(1)  size(12)         -- implicit struct size padding -- */
                                      };

[[group(0), binding(0)]]
var<uniform> uniform_buffer : B;

Related discussions: #1303, #1310, #1349. #1361

@ben-clayton ben-clayton added the wgsl WebGPU Shading Language Issues label Jan 30, 2021
@kvark
Copy link
Contributor

kvark commented Feb 1, 2021

Thank you for collaborating on this proposal and writing it down in such great detail!
Let me try to dissect this a bit. Please correct if I got anything wrong!

First, the table in "Default alignments and sizes" appears to be an application of the rules we already have in the spec (https://gpuweb.github.io/gpuweb/wgsl.html#memory-layout-intent) to all the types. If I understand correctly, it just spells out the values we already have, not proposes any new logic there.

Secondly, the proposal has 2 structure layouts provided:

  • the "default" layout applicable to storage buffers (only)
  • the "restricted" layout applicable to all buffers

Then, the proposal spells out how the "default" layout can be automatically computed, without the need of [[offset]].
This part appears very similar to #1361, where "general" = "restricted", and "storage" = "default" (in this proposal).

However, there is no help in figuring out the "restricted" layout for uniform buffers. It's as implicit as it is today in the spec (i.e. the rules are spelled out, but there is no assistance from the compiler to get them).

Thirdly, the proposal introduces new type decorations - the [[size]] and [[align]].
Question here - would specifying any of these in shader code allow the user to get any behavior that was inaccessible with padding?
If the answer is "no", then I suggest clearly separating this part of the proposal as an optional addition, so that it doesn't confuse and distract from the core part.

If I got these points right, then my only concern about this is that it makes uniform buffers more complicated for no good reason. It's hard to find a pipeline today without any uniform buffers, they should be easy to construct correctly. The simplest way to solve this would be to make the user to opt into a layout instead of having a default one, like #1361 suggests.

@ben-clayton
Copy link
Collaborator Author

Thank you for collaborating on this proposal and writing it down in such great detail!
Let me try to dissect this a bit. Please correct if I got anything wrong!

First, the table in "Default alignments and sizes" appears to be an application of the rules we already have in the spec (https://gpuweb.github.io/gpuweb/wgsl.html#memory-layout-intent) to all the types. If I understand correctly, it just spells out the values we already have, not proposes any new logic there.

It quite possibly matches perfectly (I'll try and take a deeper look tomorrow). If there are differences, then we can discuss whether we'd want to update this proposal or the existing spec.

Secondly, the proposal has 2 structure layouts provided:

  • the "default" layout applicable to storage buffers (only)
  • the "restricted" layout applicable to all buffers

Not sure I entirely follow.
This proposes one default layout. This is the natural layout for fields if no additional decorations are placed on fields or types. This proposal intentionally does not go into the required layout rules for non-storage buffers. We believe that this default layout will work for most simple uniform buffer structure layouts without explicit decorations, but provides simple ways of adjusting the layouts if the implicit layout cannot be used for the storage class.

I've tried to give examples where the default layout should always work (storage buffers), and examples of where the default layout may give errors for uniform buffer usage, and examples of how you might fix these.

Then, the proposal spells out how the "default" layout can be automatically computed, without the need of [[offset]].
This part appears very similar to #1361, where "general" = "restricted", and "storage" = "default" (in this proposal).

However, there is no help in figuring out the "restricted" layout for uniform buffers. It's as implicit as it is today in the spec (i.e. the rules are spelled out, but there is no assistance from the compiler to get them).

There is help in that if you try to compile an incompatible layout, the compiler can provide assistance with decent compiler error messages by describing which fields are misaligned and/or which structs are incorrectly sized.

Thirdly, the proposal introduces new type decorations - the [[size]] and [[align]].
Question here - would specifying any of these in shader code allow the user to get any behavior that was inaccessible with padding?

Yes - tighter packing if / when we permit such layouts.

If I got these points right, then my only concern about this is that it makes uniform buffers more complicated for no good reason. It's hard to find a pipeline today without any uniform buffers, they should be easy to construct correctly. The simplest way to solve this would be to make the user to opt into a layout instead of having a default one, like #1361 suggests.

I believe for the most part, the default layout rules will allow structs to be used as uniform buffers without significant manual fixups. There are few obviously cases where it would likely fail (vec3 followed by f32), but you're unlikely to port existing shader code with that sort of pattern, and making fixes is simple (e.g. add a [[align(16)]] on the f32).

So comparing against #1361, with this proposal:

  • There is a single default layout that we believe is simple to remember.
  • There's no struct decoration to bulk-change the default layout (we may want to consider a default-to-scalar-packed at a later date, but not for now).
  • The developer has to ensure that all storage class use of the struct have obey that storage classes' layout rules. Compiler errors should help guide the developer to fix their layout.
  • The lack of [[storage]] and [[general]] decorations mean that structs can be shared by any compatible storage classes.
  • If we want to add scalar-block-layout in the future, then this is a pure relaxation on the validation rules. All existing WGSL code will be forward compatible.

@kvark
Copy link
Contributor

kvark commented Feb 1, 2021

Thank you for the answer!

This proposal intentionally does not go into the required layout rules for non-storage buffers.

Sorry, I don't understand this part. How can we not talk about uniform buffer layouts?
Say, I'm writing a shader. How can I write a proper shader if the spec doesn't specify the requirements for the uniform data?

There is help in that if you try to compile an incompatible layout, the compiler can provide assistance with decent compiler error messages by describing which fields are misaligned and/or which structs are incorrectly sized.

I read it as "no help".

Yes - tighter packing if / when we permit such layouts.

If these tighter packing rules aren't in the spec, then I think it only supports the point that [[align]] and [[size]] should be separated out and added later, not needing to be a part of the proposal.

The lack of [[storage]] and [[general]] decorations mean that structs can be shared by any compatible storage classes.

I don't think #1361 puts any more restrictions on this. A [[layout(general)]] struct can be used for anything.

@ben-clayton
Copy link
Collaborator Author

Sorry, I don't understand this part. How can we not talk about uniform buffer layouts?
Say, I'm writing a shader. How can I write a proper shader if the spec doesn't specify the requirements for the uniform data?

The spec will specify the requirements for the uniform data, it's just not defined by this proposal.

See the very top of the proposal: "If the wider group backs this proposal, next steps would be to agree on the mandatory layout rules for each storage type."

There is help in that if you try to compile an incompatible layout, the compiler can provide assistance with decent compiler error messages by describing which fields are misaligned and/or which structs are incorrectly sized.

I read it as "no help".

"error: Uniform buffer layout rules require field 'foo' to be 16 byte aligned, but has offset 20." is a lot more help than none. But it isn't an automatic layout, which I think is what you're asking for.

Yes - tighter packing if / when we permit such layouts.

If these tighter packing rules aren't in the spec, then I think it only supports the point that [[align]] and [[size]] should be separated out and added later, not needing to be a part of the proposal.

I personally think that the (1) Using [[align]] and [[size]] on struct solution to fixing a layout is the cleanest solution out of my three examples above, but I'm not entirely opposed to mandatory padding as a first iteration.

The lack of [[storage]] and [[general]] decorations mean that structs can be shared by any compatible storage classes.

I don't think #1361 puts any more restrictions on this. A [[layout(general)]] struct can be used for anything.

Okay, fair enough.
The argument about having to remember two layout rules still stands. With this proposal, you only need to remember one to understand all the field offsets of a layout.

@kvark
Copy link
Contributor

kvark commented Feb 1, 2021

The spec will specify the requirements for the uniform data, it's just not defined by this proposal.

Ok, so there have to be 2 layouts defined anyway. It's just that the proposal only has one but not the other, gotcha!

"error: Uniform buffer layout rules require field 'foo' to be 16 byte aligned, but has offset 20." is a lot more help than none. But it isn't an automatic layout, which I think is what you're asking for.

That kind of error is the minimum we can do. It's what any proposal would have to do anyway. When I asked for "help", I meant what does it offer on top of the minimum.

The argument about having to remember two layout rules still stands. With this proposal, you only need to remember one to understand all the field offsets of a layout.

I don't quite understand this bit. It's no surprise that this proposal doesn't require me to remember 2 layouts, since it totally excludes the uniform layout from the scope, but assumes that it's going to be there.

@ben-clayton
Copy link
Collaborator Author

Ok, so there have to be 2 layouts defined anyway. It's just that the proposal only has one but not the other, gotcha!

There is one implicit default layout. At least one other layout will be defined in the spec for more restrictive storage classes. None of these non-default layouts are implicit, and are purely used for compile time validation of what you've written.

I don't quite understand this bit. It's no surprise that this proposal doesn't require me to remember 2 layouts, since it totally excludes the uniform layout from the scope, but assumes that it's going to be there.

If you can remember the default layout rules, you can read any WGSL structure and know what the field offsets are. Writing a uniform buffer struct correctly first time would also require knowing other layout rules, but the compiler can provide assistance if you don't know the rules off by heart.

@litherum
Copy link
Contributor

litherum commented Feb 2, 2021

I think this proposal is going in a good direction (full disclosure: I haven't read #1361 yet).

A few questions/statements:

  1. If an application only uses storage buffers (for some reason...), would that application author ever have to write align() or size() or stride()?

[[align(n), size(n)]] can be decorated on user-defined types and structure fields.

I think it would be a mistake to apply this to typedefs. These annotations aren't part of the type, and type definitions define new types.

The default size of a structure is equal to: roundUp(alignof(S), offsetof(last field) + sizeof(last field))

Given the distinction between size, align, and stride, I don't understand why the size of a struct has to be a multiple of its alignment. I thought the whole point of distinguishing alignments from sizes was your alignment could be smaller than your size, and it's up to the next member of the struct (or the next element of the array) to put itself wherever it needs to go given all the members before it.

Put another way, I don't see why Bar couldn't have a size of 16 bytes:

struct Foo {
    a: vec2<f32>;
    b: f32;
}
struct Bar {
    c: Foo;
    d: f32;
}

In this example, the stride of Foo (for arrays) would also be 16 bytes. But its size can totally be 12 bytes.

@kainino0x
Copy link
Contributor

kainino0x commented Feb 2, 2021

Ok, so there have to be 2 layouts defined anyway. It's just that the proposal only has one but not the other, gotcha!

This isn't the way I think about it. There is one layout algorithm defined (now and forever). There is one set of default sizes/alignments which are used by that algorithm if not overridden (now and forever). There are two sets of layout validation rules: those for uniform and those for storage (and more could be added later).

1. If an application only uses storage buffers (for some reason...), would that application author ever have to write align() or size() or stride()?

I believe they would not.

I think it would be a mistake to apply this to typedefs. These annotations aren't part of the type, and type definitions define new types.

Note this just got tweaked to be type ... = [[...]] ...; instead of [[...]] type ...

I don't understand why the size of a struct has to be a multiple of its alignment. ...

This restriction is specific to struct-typed members and comes from underlying layout rules, e.g. Vulkan's Standard Buffer Layout:

The Offset decoration of a member must not place it between the end of a structure or an array and the next multiple of the alignment of that structure or array.

@litherum
Copy link
Contributor

litherum commented Feb 2, 2021

I find the fact that uniform buffers seem to be demoted to second class citizens (because they need manual author fixup) distasteful.

@ben-clayton
Copy link
Collaborator Author

A few questions/statements:

  1. If an application only uses storage buffers (for some reason...), would that application author ever have to write align() or size() or stride()?

Assuming I've done my homework correctly, no, you wouldn't. It might not be the most optimal packing, however.

[[align(n), size(n)]] can be decorated on user-defined types and structure fields.

I think it would be a mistake to apply this to typedefs. These annotations aren't part of the type, and type definitions define new types.

Maybe. ,Decorations on type aliases isn't a highly needed feature (RGBX example seemed quite nice, but I freely admit it's non critical). I'd be more than happy to remove size / align support from type aliases for this proposal.

The default size of a structure is equal to: roundUp(alignof(S), offsetof(last field) + sizeof(last field))

Given the distinction between size, align, and stride, I don't understand why the size of a struct has to be a multiple of its alignment. I thought the whole point of distinguishing alignments from sizes was your alignment could be smaller than your size, and it's up to the next member of the struct (or the next element of the array) to put itself wherever it needs to go given all the members before it.

This was a decision to try and keep the default layout compatible enough so it wouldn't cause backend translation issues. We might be able to relax it further, but this seemed like a good compromise.

I find the fact that uniform buffers seem to be demoted to second class citizens (because they need manual author fixup) distasteful.

They don't always. I suspect needing to fix them up would be less common than you might expect.

@kainino0x
Copy link
Contributor

kainino0x commented Feb 2, 2021

I find the fact that uniform buffers seem to be demoted to second class citizens (because they need manual author fixup) distasteful.

This was done because the more restricted layout for uniform buffers is a legacy constraint. The devices that require it are diminishing over time.

@litherum
Copy link
Contributor

litherum commented Feb 2, 2021

I find the fact that uniform buffers seem to be demoted to second class citizens (because they need manual author fixup) distasteful.

(This is in comparison to #1361)

@dneto0
Copy link
Contributor

dneto0 commented Feb 5, 2021

I find the fact that uniform buffers seem to be demoted to second class citizens (because they need manual author fixup) distasteful.

The future is bright though.

The awkward constraints on uniform buffer is a legacy thing. (D3D even calls it legacy cbuffer layout). Technology is moving such that the constraints will age out. At some point future authors will be able to take the more relaxed rules for granted. (subject to how they declare they are opting in, cf. #600)

@kdashg kdashg added this to Resolved: Needs Specification Work in WGSL Feb 9, 2021
@kainino0x
Copy link
Contributor

kainino0x commented Feb 9, 2021

Resolved to accept something along these lines. Big open questions from meeting:

  • Consider whether it should be on type or member or both
    • Consider whether [[stride(X)]] array<T, N> should become array<[[size(X)]] T, N>
  • Consider [[roundSizeToMultipleOf()]] ("[[round()]]"?) instead of [[size()]]
  • Did I miss anything?

@ben-clayton
Copy link
Collaborator Author

Consider whether it should be on type or member or both

@dneto0 voiced preference for field-only - examples 2 and 3.
Example 3 makes use of the unnamed padding field _ (#1310), which should probably be introduced via a separate PR.

If anyone has preferences for size and alignment decorations on structs (example 1) then please voice your opinions now, otherwise I'll start a PR with just the field and array decorations in the next few days.

@dneto0
Copy link
Contributor

dneto0 commented Feb 10, 2021

I prefer size and alignment on fields because that's where the information will be used. The size and alignment of a struct really only matters when used in context of another struct.

The only net benefit I can see of putting alignment and size on the struct type is reuse. But that may be a false reuse because what really matters is how that struct is embedded in something else. In Ben's examples, the size of struct A only really matters in context of embedding it in struct B.

@dneto0
Copy link
Contributor

dneto0 commented Feb 10, 2021

About array strides.

Array stride is a property of the array type, not the structure field at which the array is placed in the struct.
For example, that avoids weirdness with multidimensional arrays.

Another reason is the array stride is needed for any pointers derived from a stored array.

  type Arr = [[stride(16)]] array<u32,10>;
  [[block]] struct B {
         A : Arr;
  };
  [[group(0), binding(0)]] var<storage> b:  B;


  fn foo() ->void {
        // base pointer to the array.
        const parr : ptr<storage,Arr> = b.A;

        // Compute reference to the i'th element.
        // When counting bytes, it's  i * stride(Arr) bytes after start of parr.
        // This matters conceptually, and when generating code.
        const pelem : ptr<storage,u32> = parr[i];

  }

@kvark
Copy link
Contributor

kvark commented Feb 10, 2021

@dneto0 I share the desire to limit the decorations to places where they are required, i.e. have them on fields since this is a more constrained/simple model to implement and specify.

However, I'm not so convinced that we should make an exception for the stride. I think we could go either way, in which case making it consistent with [[align]] and [[size]] seems more appropriate.

Here is the reasoning:

  1. if [[stride]] is on the field, it would strictly replace[[size]]. It makes is somewhat elegant: arrays use stride, non-arrays use size.
  2. it's still a layout property of host-shareable structures. I.e. for a private array in user code, the stride decoration has no sense.
  3. The weirdness with multi-dimensional arrays is a super niche case. I'd argue that we shouldn't even consider it for the purposes of API design, considering a simple workaround. Whoever wants to have them in host-shareable types, should just put the inner array into a struct:
struct InnerArray {
  [[stride] data: array<Foo, 5>;
};
struct Bar {
  [[stride]] data: array<InnerArray, 10>;
};
  1. For the indexing case you mentioned - const pelem : ptr<storage,u32> = parr[i], today we know statically what each pointer is pointing at, so we have no trouble computing the address of an element.

@ben-clayton
Copy link
Collaborator Author

@dneto0 I share the desire to limit the decorations to places where they are required, i.e. have them on fields since this is a more constrained/simple model to implement and specify.

I'm happy to remove the size and align decorations for type declarations.

However, I'm not so convinced that we should make an exception for the stride. I think we could go either way, in which case making it consistent with [[align]] and [[size]] seems more appropriate.

I sort of buy this argument, but I also see dneto's argument here - especially regarding multidimensional arrays.
Do we ever foresee the use of host-visible arrays outside of a structure (e.g. var<storage> data : array<f32>)? If so, how would you specify array stride here.

On reflection, I actually think [[stride]] is a better name for the field's [[size]] - as a stride is a more precise term for the size + alignment padding, which is precisely why you'd add an explicit [[size]] decoration. However that just makes array fields even more ambiguous - does a [[stride]] refer to the total array stride in bytes, or per-element stride in bytes?

@kvark
Copy link
Contributor

kvark commented Feb 16, 2021

I also see dneto's argument here - especially regarding multidimensional arrays.

This wouldn't block multi-dimensional arrays, it would just require an intermediate structure to be declared. Not a big deal.

Do we ever foresee the use of host-visible arrays outside of a structure (e.g. var data : array)? If so, how would you specify array stride here.

Good point! I'm thinking if we should just make the [[stride]] to be as optional as [[size]] is. I.e. users would only specify it if their desired stride is different from the one we compute using the default struct layout. In this case, they'd be able to use var<storage> data : array<f32> no problem, as f32 stride is well defined. If they need a custom stride, they'd (once again) be able to define a structure and put the array into it.

@kainino0x
Copy link
Contributor

However, I'm not so convinced that we should make an exception for the stride. I think we could go either way, in which case making it consistent with [[align]] and [[size]] seems more appropriate.

IMO, it's not an exception:
[[align]]/[[size]] on members is part of the struct type. They position members relative to the outer (struct) type.
[[stride]] on array is part of the array type. It similarly positions elements relative to the outer (array) type.

On reflection, I actually think [[stride]] is a better name for the field's [[size]] - as a stride is a more precise term for the size + alignment padding

I think of stride as implying it's repeated more than once (e.g. the stride between footsteps when walking), so I wouldn't agree with that. But if stride were to move out to the member, I'd be fine merging the two terms (saying there's only one "footstep" for non-array members, multiple for arrays).

@kvark
Copy link
Contributor

kvark commented Feb 19, 2021

@kainino0x

[[align]]/[[size]] on members is part of the struct type. They position members relative to the outer (struct) type.
[[stride]] on array is part of the array type. It similarly positions elements relative to the outer (array) type.

It's a matter of interpretation. If you think about arrays similar to structures, like I mentioned in #1447 (comment):

// [[stride(S)]] array<T, 3>
struct Array {
  [[size(S)]] _element0: T,
  [[size(S)]] _element1: T,
  [[size(S)]] _element2: T,
};

Then [[stride]] is a property of the members, not the container itself.

That's why I'm saying "we could go either way".
And even in your interpretation the [[stride]] is still and exception, since it becomes the only property of a type itself that only makes sense in host-shareable contexts. All other properties related to host-shareable contexts are on fields (as opposed to types).

So if we are to pick an interpretation among the two, it seems to me that the one with less exceptions is nicer, isn't it?

@kainino0x
Copy link
Contributor

I think anything attached to members is part of the outer type, they just are much more convenient to syntactically put on the members and not outside the type. OTOH with an array there's no other place to put it, and it applies equally to every member.

In that sense, both structs and arrays have "properties of the type" (even though syntactically one is inside and one is outside the definition) that only make sense in host-shareable contexts.

Anyway, I'm only arguing mental model here, and numerous mental models are valid here. So I'm not really strongly any particular way, just looking for what's both clear and convenient.

@ben-clayton
Copy link
Collaborator Author

#1447 has been merged. Closing.

WGSL automation moved this from Resolved: Needs Specification Work to Done Mar 9, 2021
@kainino0x kainino0x linked a pull request Mar 9, 2021 that will close this issue
ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this issue Sep 6, 2022
Also removes need for passing additional information into
`makeBinaryF32Case`

Fixes gpuweb#1391
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wgsl WebGPU Shading Language Issues
Projects
WGSL
Done
Development

Successfully merging a pull request may close this issue.

5 participants