Amendments to "017-shader-record" #33
Conversation
…scussions regarding updated type wrapper syntax
natevm
requested review from
csyonghe,
expipiplus1 and
tangent-vector
as code owners
|
|
| * For Vulkan, the compiler can remap annotations to the necessary `[[vk::push_constant]]` or | ||
| `[[vk::shader_record]]` attributes under the hood. | ||
| 2. **IR and Reflection Impact** | ||
| * For D3D, the compiler would still produce a `ConstantBuffer<T>` or resource |
There was a problem hiding this comment.
This section needs to be updated. I'm not sure of what changes need to be made to slang's reflection. What's here now was originally just placeholder text, from before the proposal was moved over to the spec repo.
| (push constants, root constants, or a small constant buffer) regardless of stage/backend. | ||
| * Parameters typed as `ShaderRecord<T>` map to per-shader-record data (Vulkan/OptiX SBT; | ||
| D3D local root signature). | ||
| * Parameters typed as `Payload<T>` map to payload registers. `in`, `out`, and `inout` apply, |
There was a problem hiding this comment.
My understanding of these wrapper types is that they should be treated as if they are pointer types. So Payload<T> is effectively a T*. It is not meaningful to have inout T* or out T* because that just means the pointer itself is mutable. Instead, we probably want to have RWPayload<T>, WPayload<T> to represent payload pointers with read/write or write-only access.
There was a problem hiding this comment.
We should also just make it invalid to use in inout or out on ray tracing entrypoint parameters.
There was a problem hiding this comment.
Agreed. I was running into this today with the refactor. I can make it invalid too use in/out/inout on RT entrypoint parameters.
There was a problem hiding this comment.
I’m not sure I’m aligned on the RWPayload though. With PAQs, users annotate individual fields within the structure passed as T as being read, write, or read write. It generally never makes sense to make the entire payload read-only.
My thought was to instead look at the T being passed into the payload type and assume that that’s where PAQs information comes from.
Then otherwise drop in/out semantics entirely
| * If either `[[vk::shader_record]]` or `[[vk::push_constant]]` or `[[shader_record]]` or `[[push_constant]]` | ||
| appear on entrypoint parameters, these will become "syntax sugar" which is transformed by slang into | ||
| the appropriate wrapper type. `[[hit_attribute]]` might be considered for addition to allow for consistency. | ||
| * Legacy usage of bare (non-`uniform`) parameters in ray tracing entry points should result in a warning, telling |
There was a problem hiding this comment.
This should be an error after slang 2026.
3 participants
Main change is to have the proposal language use
Type<T>rather than[[attribute]](the later could either be syntax sugar, or could result in a diagnostic)Trimmed down some of the text regarding rates to help keep discussion focused.