Update to inline SPIR-V

[s-perron]

Some features in the inline SPIR-V are hard to use, and some features are
missing. This proposal suggestes some changes that we can consider.

[danginsburg]

I strongly support this proposal, thank you for putting it together.

[Keenuts]

Some high-level questions, there are probably technical reasons for those decisions, just curious 😊

[s-perron]

@llvm-beanz Can this be reviewed and merged as a proposal, or do I need to do more before we can merge the first part?

There has been a lot of time for the Vulkan community to comment, and there does not seem to be anything too controversial.

[llvm-beanz]

@s-perron, sorry, we’ve had a couple busy weeks and the holiday last week had lots of people out. I’ve assigned this to myself, @tex3d, and @pow2clk so that it shows up on our work queues.

I had some comments that I had started to put together (nothing big), and I’ll get those posted today or tomorrow and we can aim to get this merged soon.

[llvm-beanz]

Mostly nit-picky language comments.

I should add some documentation about this but I strongly recommend using a language linter to help avoid passive language, and to avoid first-person or wishy-washy language.

I use write-good on all my technical writing as a good starting point. It has plugins for lots of common text editors.

[s-perron]

@llvm-beanz I believe I have address all of the issues. Is there anything else before the first version can be merged?

[s-perron]

@llvm-beanz Ping.

[llvm-beanz]

Two last little things from me:

• File an issue on exploring better representation of types in the source.
• Put yourself and myself as "Sponsors".

[s-perron]

Both are done. I opened #76 to follow up on Nathan's idea.

[bogner]

I think we need a little more detail on the semantics of the builtin input and output, since they could potentially have a large effect on implementation strategy of the language.

[bogner]

If I understand correctly this will mean that the compiler will have to parse the body of the function in order to figure out the input storage class, correct? What happens if someone calls the ext_builtin_input function from a function that isn't an entry point, or only conditionally?

[s-perron]

compiler will have to parse the body of the function in order to figure out the input storage class

When the compiler sees the declaration of the function with this attribute, it will create the spir-v variable in the input storage class. It has all of the information that it needed to get the storage class correct.

The compiler will have to do an analysis to know if the variable needs to be added as an operand to the OpEntryPoint instruction (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpEntryPoint). This problem is not unique to these builtins. Starting with version 1.4, all interface variables will have to be added in the same way. This proposal will not add any extra complication.

What happens if someone calls the ext_builtin_input function from a function that isn't an entry point, or only conditionally?

We need to traverse the entire call tree starting that the entry point, and it does not matter if it is conditionally referenced. However, if optimization is able to remove all references to it, then we can remove it from the OpEntryPoint, and we have an optimization in spirv-opt that does that.

I could reuse the language from the SPIR-V spec and say "add it to the OpEntryPoint instruction when it is referenced by the entry point’s call tree." Is that clear enough?

[bogner]

Yes, that language makes it clearer. Thanks for the explanation.

[devshgraphicsprogramming]

As it stands its with the old mechanism impossible to for example, delare a pointer to a struct defined in HLSL. So I could in-fact do this?

template<typename T>
using ptr_t = vk::SpirvType</*OpTypePointer*/ 32, /*PhysicalStorageBuffer*/ 5349, T>;

instead of what I have right now, which is

// 15 just happens to guess typeid(MyStruct) by accident, also its only ever defined if its used somewhere else outside BDA
#define typeid_MyStruct 15

// I need to call it myself in a way reachable from the entry point which is mildly annoying
[[vk::ext_type_def(60,/*SpvOpTypePointer*/ 32)]]
void createPtrType([[vk::ext_literal]] int storageClass, [[vk::ext_literal]] int type);

// Shouldn't I be doing the rest with?
typedef /*vk::ext_result_id<*/vk::ext_type<60> /*>*/ imm_p_MyStruct;

[[vk::ext_capability(/*SpvPhysicalStorageBufferAddresses*/ 5347)]]
[[vk::ext_instruction(/*SpvOpConvertUToPtr*/ 120)]]
imm_p_MyStruct ConvertUToPtr(uint64_t src);

...

void main()
{
    nbl::hlsl::spirv::impl::createPtrType(5349,typeid_MyStruct);

...
}

[devshgraphicsprogramming]

let me show you a vaguely "nuts" usage of SPIR-V intrinsics
https://godbolt.org/z/EEoebnTPK

As you can see I'm a bit impatient for proposals 0006 and 0010 to get ironed out and decided, so I want to make a bit of my own throw away code.

struct MyStruct{
    uint32_t a;
    uint32_t b;
    uint32_t c;
};
// 15 just happens to guess typeid(MyStruct) by accident, also its only ever defined if its used somewhere else outside BDA
#define typeid_MyStruct 15

namespace nbl
{
namespace hlsl
{

namespace spirv
{

namespace impl
{

// I need to call it myself in a way reachable from the entry point which is mildly annoying
[[vk::ext_type_def(60,/*SpvOpTypePointer*/ 32)]]
void createPtrType([[vk::ext_literal]] int storageClass, [[vk::ext_literal]] int type);

// Shouldn't I be doing the rest with?
typedef /*vk::ext_result_id<*/vk::ext_type<60> /*>*/ imm_p_MyStruct;

[[vk::ext_capability(/*SpvPhysicalStorageBufferAddresses*/ 5347)]]
[[vk::ext_instruction(/*SpvOpConvertUToPtr*/ 120)]]
imm_p_MyStruct ConvertUToPtr(uint64_t src);

template<typename T>
[[vk::ext_instruction(/*SpvOpLoad*/ 61)]]
T bda_load(imm_p_MyStruct ptr, [[vk::ext_literal]] int memoryOperands);

template<typename T>
[[vk::ext_instruction(/*SpvOpStore*/ 62)]]
void bda_store([[vk::ext_reference]] T val, imm_p_MyStruct ptr, [[vk::ext_literal]] int memoryOperands);

}

template<typename T>
T bda_load(uint64_t addr)
{
    nbl::hlsl::spirv::impl::createPtrType(5349,typeid_MyStruct);

    return impl::bda_load<T>(impl::ConvertUToPtr(addr),/*None*/0x1);
}

template<typename T>
void bda_store(inout T val, uint64_t addr)
{
    nbl::hlsl::spirv::impl::createPtrType(5349,typeid_MyStruct);

    impl::bda_store<T>(val,impl::ConvertUToPtr(addr),/*None*/0x1);
}

}

template<typename T>
struct ssbo_pointer_t
{
    T deref()
    {
        return spirv::bda_load(value);
    }
    ssbo_pointer_t<T> operator+(const int64_t incr)
    {
        ssbo_pointer_t<T> retval = {value+sizeof(T)*incr};
        return retval;
    }

    uint64_t value;
};

template<typename T, typename U>
ssbo_pointer_t<U> _reinterpret_cast(ssbo_pointer_t<T> ptr)
{
    ssbo_pointer_t<U> retval = {ptr.value};
    return retval;
}

//what can you do when you can't overload operator-> (again not really appropriate, cause its not deref)
//#define MEMBER(t,m) nbl::hlsl::_reinterpret_cast<decltype(decltype(t)::m)>(t.value+offsetof(decltype(t),m))
// Another way to do it is operator-> an `ssbo_reference_t` struct and more intrinsics to construct a whole OpAccessChain via spawning `ssbo_reference_t`

}
}

struct PushConsts {
  nbl::hlsl::ssbo_pointer_t<MyStruct> addr;
  MyStruct dummy;
};
[[vk::push_constant]] PushConsts pushConsts;

[shader("compute")]
[numthreads(1, 1, 1)]
void test( uint3 DTid : SV_DispatchThreadID )
{    
    MyStruct mystruct = nbl::hlsl::spirv::bda_load<MyStruct>(pushConsts.addr.value+4);
    mystruct.a = 6;
    nbl::hlsl::spirv::bda_store(mystruct,pushConsts.addr.value);

//    nbl::hlsl::ssbo_pointer_t<uint> p_a = MEMBER(pushConsts.addr.value,a);
}

if you want to make SPIR-V intrinsics more useful, try to get it to a place where things like this are possible.

I believe everything I need for this (Except for offsetof and decltype I only need for member access) is already throught about (templated custom SPIR-V types from HLSL user-types, no weird function calls to declare types) in this PR. But sharing anyway in-case I'm wrong.

P.S. does it make sense to use [[vk::ext_reference]] on arguments to functions with an actual function body (in other words, without the vk::ext_instruction attribute)?

[s-perron]

@llvm-beanz Anything left?