-
Notifications
You must be signed in to change notification settings - Fork 17
Add a proposal for how to explicitly specify struct layouts #171
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
Changes from 1 commit
e785c3c
29fd8d3
f442a3c
ddfd3d6
53c33d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,187 @@ | ||
<!-- {% raw %} --> | ||
|
||
# Types with explicit layouts in DXIL and SPIR-V | ||
|
||
* Proposal: [NNNN](NNNN-explicit-layout-struct.md) | ||
* Author(s): [bogner](https://github.com/bogner) | ||
* Status: **Design In Progress** | ||
|
||
## Introduction | ||
|
||
This introduces the `dx.Layout` and `spirv.Layout` target extension types, | ||
which can be used to represent HLSL structures that need explicit layout | ||
information in LLVM IR. | ||
|
||
## Motivation | ||
|
||
Some HLSL types have layout that isn't practical to derive from the module's | ||
DataLayout. This includes all kinds of `cbuffer`, but especially those that use | ||
`packoffset`, and also applies to structs that use the vulkan `[[vk::offset()]] | ||
extension and possibly objects with specific alignment specified on subobjects. | ||
|
||
We need to be able to represent these types in IR so that we can generate | ||
correct code in the backends. | ||
|
||
## Proposed solution | ||
|
||
We should implement a target type that includes a struct type, total size, and | ||
offsets for each member of the struct. This type can then be used in other | ||
target types, such as CBuffer or StructuredBuffer definitions, or even in other | ||
layout types. We need target types for DirectX and for SPIR-V, but the types | ||
can and should mirror each other. | ||
|
||
``` | ||
target("[dx|spirv].Layout", %struct_type, <size>, [offset...]) | ||
``` | ||
|
||
### Examples | ||
|
||
In the examples below we generally use "dx.Layout", since the "spirv.Layout" | ||
variants would be identical. | ||
|
||
While these aren't necessarily needed for types that don't have explicit layout | ||
rules, some examples of "standard" layout objects represented this way are | ||
helpful: | ||
|
||
```llvm | ||
; struct simple1 { | ||
; float x; | ||
; float y; | ||
; }; | ||
%__hlsl_simple1 = { i32, i32 } | ||
target("dx.Layout", %__hlsl_simple1, 8, 0, 4) | ||
; struct simple2 { | ||
; float3 x; | ||
; float y; | ||
; }; | ||
%__hlsl_simple2 = { <3 x float>, float } | ||
target("dx.Layout", %__hlsl_simple2, 16, 0, 12) | ||
; struct nested { | ||
; simple2 s2; | ||
; simple1 s1; | ||
; }; | ||
%__hlsl_nested = type { target("dx.Layout", %__hlsl_simple2, 16, 0, 12), | ||
target("dx.Layout", %__hlsl_simple1, 8, 0, 4) } | ||
target("dx.Layout", %__layout_nested, 24, 0, 16) | ||
``` | ||
|
||
Objects whose layout differs in cbuffers than in structs: | ||
|
||
```llvm | ||
; struct array_struct { | ||
; float x[4]; | ||
; float y; | ||
; }; | ||
%__hlsl_array_struct = type { [4 x float], float } | ||
target("dx.Layout", %__hlsl_array_struct, 20, 0, 16) | ||
; cbuffer array_cbuf1 { | ||
; float x[4]; | ||
; float y; | ||
; }; | ||
target("dx.Layout", %__hlsl_array_struct, 56, 0, 52) | ||
; cbuffer array_cbuf2 { | ||
; array_struct s; | ||
; }; | ||
target("dx.Layout", %__hlsl_array_struct, 56, 0, 52) | ||
; struct nested2 { | ||
; simple1 s1; | ||
; simple2 s2; | ||
; }; | ||
%__hlsl_nested2 = type { target("dx.Layout", %__hlsl_simple1, 8, 0, 4), | ||
target("dx.Layout", %__hlsl_simple2, 16, 0, 12) } | ||
target("dx.Layout", %__hlsl_nested2, 24, 0, 8) | ||
; cbuffer nested_cbuf { | ||
; simple1 s1; | ||
; simple2 s2; | ||
; }; | ||
target("dx.Layout", %__hlsl_nested2, 32, 0, 16) | ||
``` | ||
|
||
Simple usage of packoffset: | ||
|
||
```llvm | ||
; cbuffer packoffset1 { | ||
; float x : packoffset(c1.x); | ||
; float y : packoffset(c2.y); | ||
; }; | ||
target("dx.Layout", { i32, i32 }, 40, 16, 36) | ||
``` | ||
|
||
packoffset that only specifies the first field: | ||
> note: This emits a warning in DXC. Do we really want to support it? | ||
```llvm | ||
; cbuffer packoffset1 { | ||
; float x : packoffset(c1.x); | ||
; float y; | ||
; }; | ||
target("dx.Layout", { i32, i32 }, 24, 16, 20) | ||
``` | ||
|
||
packoffset that only specifies a later field: | ||
> note: This behaves differently between DXIL and SPIR-V in DXC, and the DXIL | ||
> behaviour is very surprising. Do we want to allow this? | ||
bogner marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
```llvm | ||
; cbuffer packoffset1 { | ||
; float x; | ||
; float y : packoffset(c1.x); | ||
; }; | ||
target("dx.Layout", { i32, i32 }, 24, 20, 16) | ||
target("spirv.Layout", { i32, i32 }, 20, 0, 16) | ||
``` | ||
|
||
packoffset that reorders fields: | ||
> note: This fails to compile for SPIR-V in DXC. Is this worth handling? | ||
|
||
```llvm | ||
; cbuffer packoffset1 { | ||
; float x : packoffset(c2.y); | ||
; float y : packoffset(c1.x); | ||
; }; | ||
target("dx.Layout", { i32, i32 }, 40, 36, 16) | ||
``` | ||
|
||
Use of `[[vk::offset()]]`: | ||
|
||
```llvm | ||
; struct vkoffset1 { | ||
; float2 a; | ||
; [[vk::offset(8) float2 b; | ||
bogner marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
; } | ||
%__hlsl_vkoffset1 = { <2 x float>, <2 x float> } | ||
target("spirv.Layout", %__hlsl_vkoffset1, 12, 0, 8) | ||
; struct complex { | ||
; float r; | ||
; float i; | ||
; }; | ||
; struct vkoffset2 { | ||
; float2 a; | ||
; [[vk::offset(8) complex b; | ||
; } | ||
%__hlsl_vkoffset2 = { <2 x float>, { float, float } } | ||
target("spirv.Layout", %__hlsl_vkoffset1, 16, 0, 8) | ||
``` | ||
|
||
## Open questions | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there are other open question. How and when do you convert from the But we still have the question of what should the type of the store. How should the GEP look? At first the store becomes:
Note that the GEPs currently act on the struct type. We cannot change the GEPs to use the target extension type, because they are not allowed in GEPs. We could leave it implicit in some ways, but then the optimizer will assume it knows the layout and optimize accordingly:
Note that the opimizer modified the GEPs assuming it knows the offsets, even though it does not. How will you stop the optimizer from making assumptions about the layout of the struct, since we are representing it differently than llvm-ir usually expects? This is less of a problem for cbuffers because we expect all access to the cbuffer to be through an intrinsic. There are a few options for structrued buffers: I have a couple ideas on how to fix this, but only one that seems reasonable. Add an intrinsic that does a GEP of the We might also need an intrinsic that will do a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also expand the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be hard to allow target types into a GEP instruction? Because I feel this would be the best option vs adding another intrinsic:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding target type handling to the GEP instruction sounds nicer than needing a parallel set of operations, but that's definitely a larger change to LLVM. I'll add some notes to the open questions to capture these ideas - we'll need to answer this in some satisfying way in order to handle vk::offset properly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just to be pedantic, Y'all already know all that, its just that if the meaning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we try to use a target type in the existing GEP instruction, I believe we will have many places we will have to add special cases. Any code that tries to optimize a GEP will have have a special case for the target extension type, even if it does nothing with it. That defeats the purpose of using an opaque type. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added some text to try to capture the questions here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like I indeed missed an important aspect (Thanks DevSH) (This doesn't solve the issue of GEP being allowed to use target types still)
Keenuts marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
- Should we also add a `target("dx.CBufArray", <type>, <size>)` type, rather | ||
than having the CBuffer logic need to be aware of special array element | ||
spacing rules? | ||
- Should reordering fields actually be allowed here? | ||
|
||
## Acknowledgments | ||
|
||
s-perron marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another open issue. How will member functions work? This is related to how the |
||
This proposal is expanded from comments in [llvm/wg-hlsl#94] and follow up | ||
conversations. | ||
|
||
[llvm/wg-hlsl#94]: https://github.com/llvm/wg-hlsl/pull/94 | ||
|
||
<!-- {% endraw %} --> |
Uh oh!
There was an error while loading. Please reload this page.