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

[0004] Are struct and array layouts specified by HLSL? #50

Open
s-perron opened this issue Apr 26, 2023 · 7 comments
Open

[0004] Are struct and array layouts specified by HLSL? #50

s-perron opened this issue Apr 26, 2023 · 7 comments
Assignees
Labels
active proposal Issues relating to active proposals
Milestone

Comments

@s-perron
Copy link
Collaborator

Which proposal does this relate to?
Unions

Describe the issue or outstanding question.

To implement unions, SPIR-V will have to know the size of the union. I do not know if that is specified by HLSL or if the compiler can decide for itself. Depending on which features are going to be used, the entire layout must be specific.

Additional context
See #49 for the limitations on how to represent unions in spir-v.

To create a variable that is large enough to hold the entire union, the size of the union has to be known by the compiler.

Related to this, consider the example from the proposal:

struct vector {
  union {
    struct {
      float x;
      float y;
      float z;
      float w;
    };
    struct {
      float r;
      float g;
      float b;
      float a;
    };
    float f[4];
  };
};

vector v;

Can the user assume that v.x, v.r, and v.f[0] are the same memory location? If so, where in the HLSL spec does it say that the members of the struct and the elements of the array must be at the same offset? I cannot find it, but my search was not extensive.

@s-perron s-perron added the active proposal Issues relating to active proposals label Apr 26, 2023
@s-perron
Copy link
Collaborator Author

FYI: @llvm-beanz

@llvm-beanz
Copy link
Collaborator

The Unions spec is not fully specified, but it does say:

Following C++ Unions are always sized large enough to contain their largest member type.

In practice, the sizeof(T) for a union is the size of the largest union member.

We do need to specify that and the memory layout in the spec. Each union member's layout starts at the start of the union, but that doesn't necessarily guarantee strict overlapping. In the example you provided in the issue the fields all overlap, but padding, alignment or other constraints can impact how they overlap.

@s-perron
Copy link
Collaborator Author

Based on what you said above, I would expect the following to be undefined:

float func() {
  vector v;
  v.a = 1.0;
  return v.f[3]; // Undefined because it is not specified how the struct and the array overlap.
}

However, this statement in the proposal (emphasis mine) seems to suggest that you expect to be able to do this.

One common case is when the layout of data matches between two data types and the user wants to be able to access the data interchangeably as the two types.

@llvm-beanz
Copy link
Collaborator

I think you're misunderstanding (or I'm mis-speaking). First we need to answer #43 to define whether or not punning is allowed. By C++ rules, punning is always undefined behavior regardless of the layouts (which are fairly well defined).

I think the layout rules from C that we'll inherit do define the memory overlap. In HLSL, a struct of 4 floats has no padding between members and an array of floats had no padding between members. If the union members are defined to start at the same offset, the overlap is well defined and understood.

Where it gets complicated is if you have structures of different members where padding gets involved or you had more unusual data packing.

We do still need to explore #43 to figure out the answer and behavior there.

@s-perron
Copy link
Collaborator Author

I think the layout rules from C that we'll inherit

thanks. That was the part I was missing. I did not see where that was said.

@llvm-beanz llvm-beanz self-assigned this May 8, 2023
@llvm-beanz llvm-beanz changed the title [004] Are struct and array layouts specified by HLSL? [0004] Are struct and array layouts specified by HLSL? Jun 29, 2023
@llvm-beanz llvm-beanz added this to the HLSL 202x milestone Jun 29, 2023
@jeremyong
Copy link

jeremyong commented Jul 21, 2023

To piggy-back on this issue, it'd be helpful if layouts were specified with respect to inheritance. AFAICT, HLSL restricts inheritance such that derived records may only have one concrete base (concrete meaning a record with fields, i.e. not an interface). However, I wasn't able to find information on:

  1. What alignment is stipulated on the first field of deriving records
  2. How alignments are adjusted for cbuffer records compared to default layouts
  3. If the layout order can be expected to be linear in inheritance declaration order (assumed to be so, but unspecified as of yet)

Technically, I'm not sure where the "one concrete base" rule comes from either, but I remember this was a restriction back in SM5 also, so I assume it's a carryover from there (and would be probably useful to spell out as a rule).

@devshgraphicsprogramming
Copy link

devshgraphicsprogramming commented Sep 6, 2023

I'd be fine with unions being only usable with ByteAddressBuffer.Load<T> and vk::BufferPointer<T,A> and banning them from local/private variables and constant buffers (UBOs) for the time being.

The only "useful" place I'd want to see unions is input/output variables, but I have no clue if in/out attribute locations can be aliased with SPIR-V.

P.S. ARB_enhanced_layouts in OpenGL's GLSL did use to allow some aliasing btw, but not certain types of type-punning (same offset couldn't have a float and an int over it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
active proposal Issues relating to active proposals
Projects
Status: Backlog
Development

No branches or pull requests

4 participants