| Age | Commit message (Collapse) | Author |
|---|
|
* Parameter binding and gfx fixes.
* Add diagnostics on entry point parameters.
* Fix.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
The reflection API had a bit of DWIM (Do What I Mean) logic in that a client could query the resource usage/bindings of a `ParameterBlock<X>` and see not only the register `space` or descriptor `set` for the block itself, but also the constant buffer `register` or `binding` for its default constant buffer (if any).
The reason for this behavior was that there was existing client code in Falcor that relied on that behavior for parameter blocks, and even after changing the way that parameter block layouts were computed and stored we sought to maintain backwards compatibility with that client code. The trouble is that the weird behavior then goes on to cause confusion for other clients of the Slang reflection API.
This change removes the special-case logic, and fixes up our reflection tests to mirror the new (correct) information that we return.
When this change is released, it will be a breaking change for any client code that still relies on the old behavior. We will need to coordinate with client application developers to fix their reflection logic. Note that all the same information can still be accessed, simply by using new reflection API that we have added.
|
|
This change adds some new entry points to the reflection API to cover corner cases that a majority of applications won't care about.
These are most likely to come up for users who want to make a complete copy of the Slang reflection information into a data format of their own design.
All of the information is stuff that we already computed as part of layout, and just hadn't exposed:
* Alignment information for type layouts. This is only useful for ordinary/uniform data; in all other cases alignment is always one. Even for uniform/ordinary data, it is unlikely that any application would actually make use of it.
* Layout information for the result of an entry point function. This would be useful for applications that need to enumerate the varying outputs (user- or system-defined) of a shader. Having information available for `out` parameters but not the function result was inconsistent.
* The "element type" of a parameter block type (e.g., going from `ParameterBlock<X>` to `X`). This seems to have been an oversight since `ConstantBuffer<X>` appears to have been implemented, and the case for a type *layout* was handled.
* The "container" variable layout for a parameter block or constant buffer. It took a while for us to arrive at the current representation of layout for parameter groups, and most client code continues to use the original API that requires us to generated kludged "do what I mean" data. However, if we don't expose the more useful new representation fully, there is no way for users to take advantage of it!
The reflection test tool has been updated to print the new information where it makes sense, which provides us some level of coverage for the new code.
Unfortunately, this led to some cascading changes:
* First, a bunch of the tests had their output changed since they include new information. That's the easy bit.
* Next, the "container" and "element" var layouts don't actually have names (because there is no actual variable underlying them), which means that the code to emit variable names in the JSON dump needed to be condition.
* Making the `"name"` output conditional messed up a lot of the delicate logic that had been dealing with when to emit commas for the output JSON (JSON uses commas as separators, and doesn't allow trailing commas). I added a bit of new infrastructure to make it simple(-ish) to track when a comma actually needs to be output.
|
|
* Start exposing a new COM-lite API
This change is mostly about exposing a new API to the Slang compiler that allows more fine-grained control over the compilation flow. The basic concepts in the new API are:
* An `IGlobalSession` is the granularity at which we load/parse the Slang stdlib, and therefore gives applications a way to amortize startup cost for the library across multiple compiles. This is a concept that might be able to go away in a future version of Slang.
* An `ISession` owns all the code that gets loaded/compiled/generated. Any `import`ed modules are shared across everything in a session (we don't re-parse/-check the code when we see another `import` for the same module). Any generic- or interface-based code in the session can be specialized using types from the same session (but not necessarily across sessions).
* An `IModule` is the unit of code loading and scoping. It doesn't expose any API in this change, but would be the right scope for looking up types or entry points by name.
* An `IProgram` is a "linked" combination of modules and entry points from which code can be generated and reflection information queried.
This change re-uses the existing reflection API types, rather than introduce a new API that duplicates that functionality. That will probably change in a future revision.
There are two major pieces of functionality added here that aren't related to the new API:
* We now have an API concept of "entry point groups" which are one or more entry points that are intended to be used together so that they need to have non-overlapping parameters. For now this is being used to handle "hit groups" and local root signatures for ray tracing, but I'm not sure this is a concept we will keep in the long run.
* We have a very special-case (client-application-specific) flag that ascribes special meaning to the `shared` keyword, so that it can be attached to global parameters to indicate that they are actually to be part of the local root signature rather than the global one for DXR.
None of the API design (including naming) here is finalized; the only reason to check in the changes at this point to avoid having a long-running branch that leads to merge pain. Clients should *not* try to depend on the new API just yet, since it is still a work in progress.
* fixup: clang warning
* fixup: try to detect clang C++11 support
* fixup
* fixup
* fixup
* fixup
* fixup: review feedback
|
|
* Allow plugging in types with resources for interface parameters
The key feature enabled by this change is that you can take a shader declared with interface-type parameters:
```hlsl
ConstantBuffer<ILight> gLight;
float4 myShader(IMaterial material, ...)
{ ... }
```
and specialize its interface-type parameters to concrete type that can contain resources like textures, samplers, etc.
The hard part of doing this layout is that we need to support signatures that include a mix of interface and non-interface types. Imagine this contrived example:
```hlsl
float4 myShader(
Texture2D diffuseMap,
ILight light,
Texture2D specularMap)
{ ... }
```
We end up wanting `diffuseMap` to get `register(t0)` and `specularMap` to get `register(t1)`, so that they have the same location no matter what we plug in for `light`.
But if we plug in a concrete type for `light` that needs a texture register, we need to allocate it *somewhere*.
We handle this by having the `TypeLayout` for `light` come back with a "primary" type layout that doesn't have any texture registers, but with a "pending" type layout that includes the texture register requirements of whatever concrete type we plug in.
This split between "primary" and "pending" layout then needs to work its way up the hierarchy, so that an aggregate `struct` type with a mix of interface and non-interface fields (recursively), needs to compute an aggregate "primary type layout" and an aggregate "pending type layout," and then each field needs to be able to compute its offset in the primary/pending layout of the aggregate.
A large chunk of the work in this PR is then just implementing the split between primary and pending data, and ensuring that layouts are computed appropriately.
The next catch is that when a "parameter group" (either a parameter block or constant buffer) contains one or more values of interface type, then we can allow the parameter group to "mask" some of the resource usage of the concrete types we plug in, but others "bleed through."
For example, if we have:
```hlsl
struct MyStuff { float3 color; ILight light; }
ConstantBuffer<MyStuff> myStuff;
struct SpotLight { float3 position; Texture2D shadowMap; }
``
If we plug in the `SpotLight` type for `myStuff.light`, then the `float3` data for the light can be "masked" by the fact that we have a constant buffer (we can just allocate the `float3` `position` right after `color`), but the `Texture2D` needed for `shadowMap` needs to "bleed through" and become "pending" data for the `myStuff` shader parameter.
Adding support for that detail more or less required a full rewrite of the logic for allocating parameter group type layouts.
The next detail is that when we go to legalize a declaration like the `myStuff` buffer, we will end up with something like:
```hlsl
struct MyStuff_stripped { float3 color; }
struct Wrapped
{
MyStuff_stripped primary;
SpotLight pending;
}
ConstantBuffer<Wrapped> myStuff;
```
This "wrapped" version of the buffer type more accurately reflects the layout we need/want for the uniform/ordinary data, but in order to further legalize it and pull out the resource-type fields like `shadowMap` we need to have accurate layout information, and the problem is that layout information for the original buffer can't apply to this new "wrapped" buffer.
The last major piece of this change is logic that runs during existential type legalization to compute new layouts for "wrapped" buffers like these that embeds correct offset/binding/register information for any resources nested inside them. A key challenge in that code is that existential legalization needs to erase any "pending" data from the program entirely, so that offset information that used to be relatie to the "pending" part of a surrounding type now needs to be relative to the primary part.
The work here may not be 100% complete for all scenarios, but it does well enough on the new and existing tests that I want to checkpoint it. Note that a few other tests have had their output changed, but in all cases I've reviewed the diffs and determined that the change in observable behavior is consistent with what we intened Slang's behavior to be.
Note that there is still one major piece of support for interface-type parameters that is missing here, and which might force us to revisit some of the decisions in this code: we don't properly support user-defined `struct` types with interface-type fields.
* fixup: typos
|
|
* Don't look at VK bindings when compiling for D3D and vice versa
The compiler had been looking at all the modifiers on a declaration when piecing together binding information, whether or not those modifiers should apply on the chosen target API. This was working in practice because the "layout resource kinds" used by each API target were disjoint, for the most part.
This change ensures that we don't even look at modifiers that don't apply on the chosen target, and furthermore adds a new warning that applies if the user is compiling a shader with explicit `register` bindings for Vulkan, if there are no corresponding `[[vk::binding(...)]]` attributes (under the assumption that if they want to be explicit in one case, they probably want to be explicit in all cases).
* Allow explicit space/set bindings on parameter blocks
The syntax for the D3D case is to specify a `space` in a `register` modifier, without any other register class:
```hlsl
ParameterBlock<X> myBlock : regsiter(space999);
```
In the Vulkan case, the user must apply the `[[vk::binding(...)]]` attribute and is expected to use a `binding` of zero:
```hlsl
[[vk::binding(0,999)]]
ParameterBlock<X> myBlock;
```
This change includes a reflection test for the new capability (where we also confirm that it produces the expected output when compared with fxc), and a test for the diagnostic messages when the user messes up bindings for Vulkan.
The implementation itself is fairly straightforward, since the compiler already treats registe spaces/sets as a resource that parameters can consume directly.
Note: the test case for explicit parameter block space/set bindings includes some commented out code that lead to a compiler crash. I would like to fix the underlying issue, but it seemed sensible to keep the bug fix out of a change like this that is adding functionality.
|
