[SPIR-V] Scalar layout to follow C structure layout #7996
Conversation
|
Thanks for the contribution. That is great. We'll have to see if this causes regressions for anyone. It could be that people depend on the old behaviour, and this could break them. If that is significant, then we will have to add a new option. @gjaegy, @devshgraphicsprogramming, @sajjadmirzanv, @LukasBanana, @EpicJeanNoeMorissette If any of you use the scalar layout option in DXC, let me know if this will cause problems for you. I think this is the right direction in spirit, but it might not be pragmatic. |
|
We don't use the "vk-use-scalar-layout" option, so not a problem for us, thanks for asking ! |
It actually fixes stuff for us, LGTM |
|
Following the description in #7894, this makes sense to me. If the intention was to have C-like structures, this sounds like a reasonable fix. We do use this layout in UE5 as the extension |
|
/AzurePiplines run |
|
@microsoft-github-policy-service agree company="BAE Systems OneArc" |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
s-perron
left a comment
s-perron
left a comment
There was a problem hiding this comment.
Looks good. just a minor issue, and then I will be able to merge it.
| // CHECK: [[buf_0:%[0-9]+]] = OpAccessChain %_ptr_Uniform_ulong %buffer %int_0 %uint_0 %int_1 | ||
| // CHECK-NEXT: OpStore [[buf_0]] %ulong_56 | ||
| buffer[0].c = sizeof(Bar); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Can you add a new line here?
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
5 participants
See #7894 for context.
Note that this is a backwards incompatible change and may break code that relies on the current odd behavior of
-fvk-use-scalar-layout. It also fixes a case found by @s-perron that currently generates invalid SPIR-V (scalar.layout.struct.array test) because structs don't get properly aligned inside arrays.