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

Support for 8 bit storage extension #4637

Closed
TheArheus opened this issue Sep 3, 2022 · 12 comments
Closed

Support for 8 bit storage extension #4637

TheArheus opened this issue Sep 3, 2022 · 12 comments
Labels
spirv Work related to SPIR-V

Comments

@TheArheus
Copy link

That seems there is no 8 bit data storage extension right now. Maybe there are plans to add support for VK_KHR_8bit_storage extension, so in shader I could write something like this:

struct vertex
{
    uint8_t vx, vy, vz;
    uint8_t nx, ny, nz;
};

If so, then maybe add flags like -fspv-extension=SPV_KHR_8bit_storage and -enable-8bit-types. Thanks

@natevm
Copy link
Contributor

natevm commented Sep 9, 2022

+1. It would be really helpful to have an official means of representing 8-bit types.

@pow2clk pow2clk added the spirv Work related to SPIR-V label Sep 12, 2022
@natevm
Copy link
Contributor

natevm commented Jan 14, 2023

Finding a couple more use cases for this feature.

With hardware accelerated ray tracing, ray visibility flags are 8 bit integers. The TraceRay call in HLSL has users pass a full 32-bit integer, but counterintuitively discards the top 24 bits. This is kinda confusing to new HLSL users, since they might assume those top 24 bits do something when really they don't...

That also affects the readability and efficiency trying to use these masks. I'd like to create a buffer of these visibility masks and expose that buffer to my users, but it's a little odd and wasteful asking users for full 32 bit integers when just the first 8 bits are used. At the same time, I can't directly load 8-bit integers, making it difficult to setup VkAccelerationStructureInstanceKHR structures on the device in parallel.

More broadly, many structures shared between the CPU and the GPU these days use 8-bit integer fields, and many GPU architectures support 8-bit integers through API like GLSL/OpenCL/SPIRV, CUDA, SYCL, HIP...

@sudonatalie
Copy link
Collaborator

As a general rule, if a feature is not supported by HLSL->DXIL, it won't be supported by the SPIR-V backend either. However, if you're interested in this for targeting SPIR-V only you may be able to accomplish it with the inline SPIR-V intrinsics.

Requests for changes to HLSL are now handled by the microsoft/hlsl-specs repo, so a proposal for 8-bit scalar types could be directed there.

@natevm
Copy link
Contributor

natevm commented Aug 23, 2023

Closing all these issues with the argument they should go to the HLSL spec—without actually bringing them over yourself to the HLSL spec repo—honestly comes off as lazy and dishonest.

I’ve had about 10-20 issues I’ve been tracking be closed in a half-arsed way, without any effort to migrate those issues to this spec repo, and with seemingly only a glance at the actual issues to find some excuse to close.

Maybe you’re being pushed by some manager to close this massive set of issues in any way possible, but that currently appears to be by pulling a rug over the issues rather than carrying them over to the appropriate channels.

Clearly you can see how that might be a problem?…

@natevm
Copy link
Contributor

natevm commented Aug 23, 2023

Somewhat related, more of a complaint about Microsoft’s DX platform… The number of times DXIL gets in the way of progress in consumer-facing high performance computing these days is insane. Frustrating how seemingly far behind DXIR is compared to Vulkan. Even more frustrating that HLSL suffers glaring issues as a consequence, and we’re all just expected to match the pace of Microsoft dragging their feet.

Yes, my priorities are probably different in research. I need a platform though that will support my efforts, and I’m getting the sense that HLSL isn’t it.

I’m honestly tempted to just fork DXC at this point and add these features myself to circumvent all the bureaucracy. Like, what’s the point of SPIRV if no language can actually use it effectively?… HLSL is so close to something amazing, but DXIR is its Achilles heel. If according to my gamedev colleagues DX is just as buggy on AMD as VK is, and DXIR is way less feature complete as Vulkan, why continue to use it in research?

I know at this point I’m just talking to myself, and at this point I know any response by HLSL core devs is likely going to be disappointing, more sweeping issues under the rug.

I’ll likely either be switching back to Slang and pushing for these features there or just forking DXC and removing the DXIR generation altogether.

@sudonatalie
Copy link
Collaborator

To be clear, we are focused on expanding coverage of Vulkan-specific features from HLSL through better support for inline SPIR-V. Using VK_KHR_8bit_storage types should already be possible with the current set of inline SPIR-V intrinsics, however if not, the improvements proposed in Update to inline SPIR-V #59 will put us in a better position to support the use of types and extensions like this in a much more user-friendly way (ack that the current set of attributes are hard to use) and without requiring code changes to the compiler.

My pointer to the hlsl-spec repo was only to note that there is now a path for requesting HLSL feature changes, if the desire was for a first-class uint8_t type to be supported by both DXIL and SPIR-V.

@s-perron
Copy link
Collaborator

@natevm I can understand your frustration. We are trying to deal with the backlog of issues now that we have more resources. We choose to apply those resources in the area that are that are inline with our goals, which are different from yours. One of the important design goals, which you would disagree with, is that we want to keep the same HLSL for DXIL and Vulkan as much as possible. Fragmenting the language will not be good in the long term. I believe this is what is leading to a lot of the friction, but it is something that we will not waiver on. This forces us to move more slowly or even, as in this case, say that we will not do something because DXIL does not support it.

We are not opening the issues in HLSL specs because we do not want it to appear as if the request is coming from us, or that we will be putting a lot of resources toward designing it. We will only do that for features that are important to our customers. If you feel like you really want the feature, you can open the issue in HLSL specs on your own.

@devshgraphicsprogramming
Copy link

devshgraphicsprogramming commented Oct 6, 2023

As a general rule, if a feature is not supported by HLSL->DXIL, it won't be supported by the SPIR-V backend either. However, if you're interested in this for targeting SPIR-V only you may be able to accomplish it with the inline SPIR-V intrinsics.

The syntax for [[vk::ext_type_def(UUID,...)]] doesn't let DXC know the sizeof(vk::ext_type<UUID>) nor the alignof(vk::ext_type<UUID>) so it will get in the way of reading/storing them from byte offsets, I think?

To be clear, we are focused on expanding coverage of Vulkan-specific features from HLSL through better support for inline SPIR-V. Using VK_KHR_8bit_storage types should already be possible with the current set of inline SPIR-V intrinsics, however if not, the improvements proposed in microsoft/hlsl-specs#59 will put us in a better position to support the use of types and extensions like this in a much more user-friendly way (ack that the current set of attributes are hard to use) and without requiring code changes to the compiler.

even the new HLSL Lang proposal 0011 syntax

typedef vk::SprivType</* OpTypeAvcMcePayloadINTEL */ 5704> AvcMcePayloadINTEL;

// Requires HLSL2021
template<typename ImageType>
using VmeImageINTEL = vk::SpirvType</* OpTypeVmeImageINTEL */ 5700, Imagetype>;

doesn't have enough parameters in vk::SpirvType to let us know size, alignment and whether the type is opaque or not.
I'm actually commenting on microsoft/hlsl-specs#97 about that.

Furthermore, its not like declaring a builtin uint8_t and int8_t type will magically give me a type/class that can do all the arithmetic, or which I can slam into vector<> or matrix<> templates.

I’m honestly tempted to just fork DXC at this point and add these features myself to circumvent all the bureaucracy.

Please ping me if you do, we're interested in adding/fixing a few things ourselves too, like #5675

I’ll likely either be switching back to Slang and pushing for these features there or just forking DXC and removing the DXIR generation altogether.

I've investigated Slang, no templates and its not like you can share much code between that and C++. don't give up hope yet.

@devshgraphicsprogramming

Btw even if I successfully managed to declare a vk::SpirvType for a uint8_t or similar, I wouldn't be able to create a wrapper class that satisfies the arithmetic or scalar concept around it, due to other issues like #4133

@natevm
Copy link
Contributor

natevm commented Feb 19, 2024

If any future dxc users finding their way here, unlike DXC, Slang has full support for uint8_t types!

So if 8 bit types are a feature you value, I highly recommend giving slang a try.

http://shader-slang.com/slang/user-guide/conventional-features.html

@devshgraphicsprogramming

As a general rule, if a feature is not supported by HLSL->DXIL, it won't be supported by the SPIR-V backend either. However, if you're interested in this for targeting SPIR-V only you may be able to accomplish it with the inline SPIR-V intrinsics.

The syntax for [[vk::ext_type_def(UUID,...)]] doesn't let DXC know the sizeof(vk::ext_type<UUID>) nor the alignof(vk::ext_type<UUID>) so it will get in the way of reading/storing them from byte offsets, I think?

To be clear, we are focused on expanding coverage of Vulkan-specific features from HLSL through better support for inline SPIR-V. Using VK_KHR_8bit_storage types should already be possible with the current set of inline SPIR-V intrinsics, however if not, the improvements proposed in microsoft/hlsl-specs#59 will put us in a better position to support the use of types and extensions like this in a much more user-friendly way (ack that the current set of attributes are hard to use) and without requiring code changes to the compiler.

even the new HLSL Lang proposal 0011 syntax

typedef vk::SprivType</* OpTypeAvcMcePayloadINTEL */ 5704> AvcMcePayloadINTEL;

// Requires HLSL2021
template
using VmeImageINTEL = vk::SpirvType</* OpTypeVmeImageINTEL */ 5700, Imagetype>;

doesn't have enough parameters in vk::SpirvType to let us know size, alignment and whether the type is opaque or not. I'm actually commenting on microsoft/hlsl-specs#97 about that.

Furthermore, its not like declaring a builtin uint8_t and int8_t type will magically give me a type/class that can do all the arithmetic, or which I can slam into vector<> or matrix<> templates.

I’m honestly tempted to just fork DXC at this point and add these features myself to circumvent all the bureaucracy.

Please ping me if you do, we're interested in adding/fixing a few things ourselves too, like #5675

I’ll likely either be switching back to Slang and pushing for these features there or just forking DXC and removing the DXIR generation altogether.

I've investigated Slang, no templates and its not like you can share much code between that and C++. don't give up hope yet.

Checked back almost exactly a year later and tried to make uint8_t, that bug is still an issue
https://tinyurl.com/mv3kehwp

@natevm
Copy link
Contributor

natevm commented Oct 12, 2024

Now that DX is officially moving away from DXIR and to SPIR-V, uint8_t should become an official language type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spirv Work related to SPIR-V
Projects
None yet
Development

No branches or pull requests

6 participants