[hlsl-out] Insert padding between struct members

[teoxoy]

fixes #1783

With this fix WGSL's align & size attributes and also spir-v's Offset decoration are respected since the offset in the IR is affected by those.

[teoxoy]

I noticed half is supported by the hlsl backend but the issue with using half as the padding type is that its size can be either 2 or 4 bytes. How should we deal with this?

[kvark]

but the issue with using half as the padding type is that its size can be either 2 or 4 bytes

Is that not defined in the spec? we shouldn't expose f16 (or half) until we figure out the details on how it maps to HLSL.
Note that fp16 extension has still not landed on WebGPU - gpuweb/gpuweb#2647

[kvark]

Thank you for the PR!

[teoxoy]

half - 16-bit floating point value. This data type is provided only for language compatibility. Direct3D 10 shader targets map all half data types to float data types.

reference

DXC requires the -enable-16bit-types flag to be passed as described here in order for half to be a true 16bit type otherwise it's 32bit

Maybe a better way to do this would be to check if the struct contains a member with a half type and only then use half for padding since at this point it would be safe to assume that the half type is 16bit. Also the padding only needs to be as small as the smallest struct member.

What do you think?

[kvark]

@teoxoy sounds like a reasonable proposal!

[teoxoy]

Maybe a better way to do this would be to check if the struct contains a member with a half type and only then use half for padding since at this point it would be safe to assume that the half type is 16bit. Also the padding only needs to be as small as the smallest struct member.

sounds like a reasonable proposal!

so, should I open a new PR implementing that logic?

[kvark]

If you feel that the FP16 spec has issues with half, please open an issue. Suggesting or writing a PR with a solution is greatly appreciated, obviously!
I suspect we are the only ones detecting it because Google virtualized uniform buffers on DX12, so they aren't actually using half in them. So your worries are likely right.

[teoxoy]

I looked into this some more and it looks like the WGSL FP16 extension will require generation of DXIL (comment, slide). So I guess when we start implementing the extension, we will just fail if we try to target the HLSL backend when the extension is required.

[kvark]

we'll just not expose it on DX12 until we are able to generate DXIL