[wgsl] Explicit layout needs more than Offset: ArrayStride and matrix attributes

[dneto0]

This issue is a blocker for meaningful support of compute shaders.

We have agreed that buffer resource data needs to be explicitly laid out.
That was resolved in the discussion of #561 where we talked about the Offset attribute.

However, Offset alone is not enough to explictly lay out data.

Explicit layout in SPIR-V shaders (for graphics APIs) also uses:

• ArrayStride
• RowMajor, ColMajor: for matrices
• MatrixStride: for matrices
  See [https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_shadervalidation_a_validation_rules_for_shader_a_href_capability_capabilities_a](validation rules for Shader modules) for the requirement that structured data passing between host and device must be explicitly laid out. Start at "Composite objects in the StorageBuffer,..."

The ArrayStride is the most pressing one to have and which is missing from the main text of the current WGSL draft. There is an example in 3.16 Type Aliases showing an example usage:

type RTArr = [[stride 16]] array<vec4<f32>>;

type S = [[block]] struct {
  [[offset 0]] a : f32;
  [[offset 4]] b : f32;
  [[offset 16]] data : RTArr;
};

ArrayStride is the number of bytes between two adjacent elements in the array.
E.g. This attribute is required to capture the difference between the natural layouts for a std140 and a std430 buffer. (In std140, array stride must be a multiple of 16; std430 doesn't have that rule.)

Matrix stuff:

Vulkan supports both row-major and column-major matrices. So we need to apply RowMajor or ColMajor on each thing that ends up being a matrix in the structured data interface.

Additionally, for a row-major matrix the matrix-stride is the number of bytes between adjacent rows; for a column-major matrix the matrix-stride is the number of bytes between adjacent columns. Vulkan supports multiple matrix-strides, i.e. more than the minimum natural stride.

Now, where to apply these. SPIR-V has the rule that effectively you don't decorate a type with attributes (there at most one type definition of a matrix with a given number of rows, columns, and scalar component type). So instead the matrix-related layout attributes are applied to the struct member of matrix type or struct member of any level of nested-array-of-matrix type.
This is sufficient for several reasons, including the fact that a single descriptor binding maps to a block-decorated struct.

So the proposal is:

1. ArrayStride:

   • 1a. Add (some spelling of) ArrayStride as a possible attribute of an array type, applied via a type-alias, as in the example currently in 3.16.
   • 1b. Spell out the rules of when it's needed: on any array type used as the member of a struct that describes the content of a single buffer-like binding.

2. Spell out matrix layouts. I'm happy to attack this separately, but it's still needed for MVP.

[grorg]

Discussed at the 2020-05-19 meeting.

[grorg]

Related to #561

[damyanp]

D3D's packing rules:

Everything is aligned to the maximum of per-component natural alignment or 4. "per-component" here means the scalar component - so for a float4 the component type is float and so it must be 4-byte aligned.

There's an additional nuance here with constant buffers. The above describes the "new" layout, but constant buffers may also use a "legacy". "legacy" packing rules are documented here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-packing-rules. fxc only supports legacy layout (this is relevant to any WebGPU-compat mode goals), so any pipeline that relies on fxc (eg D3D11) will need to work within those constraints.

[dneto0]

fxc only supports legacy layout (this is relevant to any WebGPU-compat mode goals)

To be explicit about it, Chrome definitely wants to support D3D11 targets.

[kainino0x]

But we may take additional restrictions to do so.

[dneto0]

Vulkan's rules are here:

https://www.khronos.org/registry/vulkan/specs/1.1/html/vkspec.html#interfaces-resources-layout

I adapted them for WebGPU here (when SPIR-V was our proposal):
https://github.com/gpuweb/spirv-execution-env/blob/master/execution-env.md#layout

I think that's a clear set of rules. It has different rules for UBOs and SSBOs; UBOs have additional requirements that the base alignment of structure members, and of array strides be a multiple of 16.
Like D3D11, there's a requirement to avoid straddling a 16byte boundary.

I've mentioned in meetings that, in order to help port D3D content to Vulkan, Vulkan has added extensions to relax rules over time. These have produced "relaxed" block layout, and then later "scalar" block layout.
There's also now a "uniformBufferStandardLayout" Vulkan feature which is required to be supported in Vulkan 1.2. That allows a UBOs to use the same layouts as SSBOs. Vulkan 1.2 was introduced in January; it's not on any mobile devices that I know of.

[kainino0x]

There's also now a "uniformBufferStandardLayout" Vulkan feature which is required to be supported in Vulkan 1.2.

The feature flag is now core, but can't the feature itself can still be unavailable?

[litherum]

Add (some spelling of) ArrayStride as a possible attribute of an array type, applied via a type-alias, as in the example currently in 3.16.

Does that mean it would be impossible to add an array to a uniform block without making a dummy type alias? 😕

[dneto0]

Does that mean it would be impossible to add an array to a uniform block without making a dummy type alias? 😕

Yes, you understood correctly.

The reasoning is that you want the array layout to be part of the type, so that with a type-correct program address calculations (called sub-indexing in the draft) can be done correctly, even if you break up the sub-indexing into intermediate steps

Example, where I've added an unusual stride:

type MyArr =  [[stride 16]] array<f32>;     # unusual stride
type S = [[block]] struct {
   [[offset 0]] data : MyArr;
};

var<storage_buffer> my_s : S;

fn main() -> void {
  const sptr : ptr<storage_buffer, S> = my_s;
  const arr_ptr : ptr<storage_buffer, MyArr> = my_s.data;

  const i : u32  = 12;                           #  many intervening steps ...
  const elem_ptr : ptr<storage_buffer, f32> = arr_ptr[i];    # address calculation
  elem_ptr = 3.25;         # store
  return;
}

The stride is an intrinsic part of the array type. The benefit is you don't have to go decorating all the intervening address calculations with the stride. Having to decorate the intermediate calculations is tedious and error prone.

[litherum]

It's unfortunate to require typedefs for all host sharable arrays. How about new syntax instead?

Like...

• array<i32, 5, stride=14>
• [[stride 16]]array<i32, 5>
• i32[5, 16]
• i32[5]|16
• i32[5]16
• i32[5]stride=16
• i32[5, stride=16]

Pick your favorite :)

[kvark]

@litherum pretty much all of these variants rely on syntax that is not consistent with the current grammar. I.e. stride=14 as a named generic parameter, while all the other ones are not named. Or [[stride 16]] being decorations outside of top-level items (or their fields). Or i32[5]|16 which is just new syntax, etc. The only one that doesn't introduce new syntax is i32[5, 16], and IMHO it looks the worst, since it's confusing to have two separate numbers together.

Saying that, it seems to me that the original PR is most consistent with the grammar, and would make our lives easier.

[dj2]

Do we know how common host shareable arrays are in existing corpora?

[dneto0]

Do we know how common host shareable arrays are in existing corpora?

Any bulk data in a uniform buffer or in a storage buffer is going to be array-based. It's either going to be an array or an unspecified-sized array (what SPIR-V calls a runtime array).

e.g. in GLSL:

// storage buffer
layout(set=0,binding=0) buffer BufferName {
   int count;
   vec4 u[2]; // sized array
   vec4 v[];   // unsized array; can only be last element in a storage buffer's struct.
} buffer_var;

void main() {
   buffer_var.u[0] = vec4(0.0);
   buffer_var.v[i] = vec4(1.0);
}

[dneto0]

I'm with @kvark on this one.
The original suggestion is the most regular. Furthermore, it's already in a the "Type Alias" example in the spec.
Putting the stride attributes in the type alias declaration for the array is a very isolated change to the grammar, and 100% clear.

Of @litherum's suggestions, the most regular one and least-deviating from existing WGSL syntax is:

[[stride 16]]array<i32, 5>

But type names appear all over the place in WGSL, so the burden is on @litherum to argue that there are no weird cases introduced. For example, from the interactions with other attributes that might be at play. Does this restrict further growth?

I think for MVP the original suggestion of using the type alias is the winner here.

[dneto0]

Discussed in WGSL meeting 2020-07-21

Action: @dj2 will change the grammar to allow [[stride 16]]array<i32,5> syntax anywhere an array type is permitted.

[litherum]

It is not possible today to give an explicit stride in MSL or explicitly change the majorness (row-major or column-major) of a standard MSL matrix.

[dj2]

Hm, that means for MSL we'll need to change the element type to match the stride value, possibly turning it into a struct with padding at the end?