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

Add packed 8bit builtin types #5939

Merged
merged 3 commits into from
Dec 27, 2024

Conversation

fairywreath
Copy link
Contributor

@fairywreath fairywreath commented Dec 26, 2024

Adds packed 4x 8 bit values that are stored in 32 bit integers specified in SM 6.6.

These types are added as builtin types where it is internally (somewhat) represented as an unsigned 32 bit type. These types do not have arithmetic and logical(bitwise) operators. Explicit casting is required to cast a variable of another builtin integer variable type to/from these packed types and also for casting between the signed and unsigned packed types - current behavior emits a warning for cases of implicit casts.

For code generation on all targets except HLSL, these types resolve to an unsigned 32-bit integer type.

csyonghe
csyonghe previously approved these changes Dec 26, 2024
Copy link
Collaborator

@csyonghe csyonghe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is OK, but we should really just support vector<uint8, 4> and use these intrinsics to implement the buffer load/store operations when generating HLSL code.

@fairywreath
Copy link
Contributor Author

fairywreath commented Dec 26, 2024

Can you infer further on what you mean by support vector<uint8, 4> and implement the buffer load/store operations? Do you mean something like typealiasing these new types to vector<uint8, 4>?

Also some additional notes:

  • DXC implements these new types as unsigned builtins. They are treated the same way as any unsigned integer type with arithmetic and logical operators, integer literals can also be assigned to them. The individual components(i.e. y,z,w) of the type cannot be accessed.
  • I personally find the SM 6.6 unpack/pack specs a bit awkard to use. I do not see a point of these new types unless they want to enforce strong typing so users cannot modify these types' variables(eg. addition of two of them) freely, but DXC does not enforce this. All other shading languages with unpack intrinsics accept u32, and if users want to work with an actual 8bit vector they can just use vector<uint8, 4> . There is also no unpacking to float vectors, only integers. Anways I still want to have these intrinsics implemented so we have more feature parity and compatibility with HLSL.
  • I originally implemented these type as __magic_type but had too much trouble working around the integer casting so I used builtin here instead.

@csyonghe
Copy link
Collaborator

First of all, the implementation here is good as is.

What I was talking about is something we should consider adding as a separate PR. This is similar to how we are handling column major matrix or integer typed matrices on spirv/metal: conceptually every type has a "register" type and a "storage type", where the former represents the type of such a variable when defined in "local register" space, and the latter represents how the type should be declared inside a constant/storage buffer.

In the case of column_major float4x4, it will be represented as just a float4x4 in register and a array<vec4, 4> in a buffer.

The handling of storage type packing/unpacking is done in the lowerBufferElementStorageType pass.

If we are to support vector<int8, 4> on HLSL where there is no native int8 support, we can uultize the same infra structure to implement it: int8 always translates to int, and vector<int8,4> will have its storage type defined as packed_int8_4 and register type defined as vector<int,4>. By doing so we can just allow the user to use vector<int8,4> like all other vectors, but generate the necessary pack and unpack code using the HLSL intrinsics after loading from memory or before storing into memory.

@csyonghe csyonghe added the pr: non-breaking PRs without breaking changes label Dec 26, 2024
@csyonghe csyonghe merged commit 7cecc51 into shader-slang:master Dec 27, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: non-breaking PRs without breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants