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

Surround C++ operator overloads in hpp and hpp11 with an #ifndef SPV_DISABLE_ENUM_OPERATORS #385

Open
devshgraphicsprogramming opened this issue Oct 19, 2023 · 5 comments · May be fixed by #386

Comments

@devshgraphicsprogramming
Copy link

devshgraphicsprogramming commented Oct 19, 2023

Hi, we're doing a slightly weird thing, which is we're including the SPIR-V Header in our HLSL code to make the writing on SPIR-V intrinsics easier (don't need to remember the opcodes)

However HLSL doesn't support global operator overloads, so we need a define similar to SPV_ENABLE_UTILITY_CODE (although a negative one so backwards compatibility it preserved)

alternative is #ifndef __HLSL_VERSION which checks for the HLSL compiler.

PR incoming.

@devshgraphicsprogramming
Copy link
Author

@dneto0 is that acceptable?

@dneto0
Copy link
Contributor

dneto0 commented Oct 25, 2023

Hi. It would be better to generate a new file specific for HLSL inclusion.

@devshgraphicsprogramming
Copy link
Author

@dneto0 actually if we were to make a HLSL header, would you accept something that actually declares all the SPIR-V functions and types using DXC's inline SPIR-V, like so?

https://github.com/Devsh-Graphics-Programming/Nabla/blob/spirv_intrinsics/include/nbl/builtin/hlsl/spirv_intrinsics/core.hlsl

We're kinda thinking of generating them from the JSONs now because its a hassle to maintain them by hand, and since they'd be generated from the Header JSONs it seems like this repo is the place to keep them.

@dneto0
Copy link
Contributor

dneto0 commented Oct 30, 2023

@dneto0 actually if we were to make a HLSL header, would you accept something that actually declares all the SPIR-V functions and types using DXC's inline SPIR-V, like so?

https://github.com/Devsh-Graphics-Programming/Nabla/blob/spirv_intrinsics/include/nbl/builtin/hlsl/spirv_intrinsics/core.hlsl

That blob has gone missing.

We're kinda thinking of generating them from the JSONs now because its a hassle to maintain them by hand, and since they'd be generated from the Header JSONs it seems like this repo is the place to keep them.

I would say no. Use the core grammar file to generate what you want in your own repo.
This is what SPIRV-Tools does.
It's why the grammar file is in JSON.

@devshgraphicsprogramming
Copy link
Author

@dneto0 actually if we were to make a HLSL header, would you accept something that actually declares all the SPIR-V functions and types using DXC's inline SPIR-V, like so?
https://github.com/Devsh-Graphics-Programming/Nabla/blob/spirv_intrinsics/include/nbl/builtin/hlsl/spirv_intrinsics/core.hlsl

That blob has gone missing.

Sorry about that, here's a permalink:
https://github.com/Devsh-Graphics-Programming/Nabla/blob/6253ed92aa0c979bec75235c6fa7b0d3ac33c959/include/nbl/builtin/hlsl/spirv_intrinsics/core.hlsl#L21

We're kinda thinking of generating them from the JSONs now because its a hassle to maintain them by hand, and since they'd be generated from the Header JSONs it seems like this repo is the place to keep them.

I would say no. Use the core grammar file to generate what you want in your own repo. This is what SPIRV-Tools does. It's why the grammar file is in JSON.

My original request/sugestion was to make the C++ (and 11) headers shareable with HLSL by conditionally disabling the global operator overloads (since DXC's modified Clang disabled that).

Then I realized that since you'd want a separate header for HLSL, I could provide the intrinsics to go along with it:

template<typename T>
T atomicAdd([[vk::ext_reference]] T ptr, uint32_t memoryScope, uint32_t memorySemantics, T value);
template<>
[[vk::ext_instruction(spv::OpAtomicIAdd)]]
int32_t atomicAdd([[vk::ext_reference]] int32_t ptr, uint32_t memoryScope, uint32_t memorySemantics, int32_t value);
template<>
[[vk::ext_instruction( spv::OpAtomicIAdd )]]
uint32_t atomicAdd([[vk::ext_reference]] uint32_t ptr, uint32_t memoryScope, uint32_t memorySemantics, uint32_t value);

Then this not only gives you the SPIR-V enum tokens but also actual intrinsic entry points to use in your HLSL.

P.S. This is non-invasive because DXC will not emit any capabilities or extensions if they're not in the entry point's call tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants