| Commit message (Collapse) | Author | Age |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Fixes related to combined texture/sampler types
Work on #1891
Our intention has always been to support combined texture/sampler types in Slang, both targets like OpenGL where that is the only option available and for targets like Vulkan where it can be beneficial to performance. Because Slang's current users mostly focus on D3D12+Vulkan codebases, they strongly prefer separate textures and samplers, and the relevant support code in Slang has "bit-rotted" over many releases until the functionality that was there isn't useful any more.
This change significantly overhauls the implementation of combined texture/sampler types and adds a test that uses them in the hopes of avoiding future regressions.
The new combined texture/sampler types use the prefix `Sampler`, so where there is an existing standalone `Texture2D` type the equivalent combined texture/sampler type will be `Sampler2D`. The intention is that this naming mirrors the GLSL conventions (where the type is `sampler2d`) while following HLSL naming precedent (to the extent it exists).
The operations available on a `Sampler2D` are intended to be those that are available on a `Texture2D`, and it is just that in cases where the `Texture2D` operation would take a separate sampler argument:
Texture2D t = ...;
SamplerState s = ...;
float4 result = t.Sample(s, uv);
the equivalent `Sampler2D` operation just elides that argument:
Sampler2D s = ...;
float4 result = s.Sample(uv);
In terms of implementation, there are a lot of subtle details here:
* I've tried to use the same metaprogramming logic that generates all the stdlib declarations for `Texture*` to also generate `Sampler*` in the hopes that this helps keep them in sync.
* The big catch to the above is that it means that for certain operations the indidces of parameters depend on whether or not an explicit sampler parameter is used. Rather than try to tweak the indices in the stdlib generation logic (which is already complicated) I went and added Yet Another Hack to the logic that handles intrinsic definition strings. Basically, the special-case handling of `$p` has been modified so that it *also* applies a negative offset to future parameter references in the same intrinsic string.
* Trying to actually bring this up in our test framework revealed that the "flat" reflection API was seemingly not reflecting combined texture/sampler types correctly at all (it was reflecting them as just plain textures). Other than that issue, the Vulkan path seems to work fine with combined texture/samplers.
* I also had to add logic to the `TEST_INPUT` parsing to re-introduce handling of the combined types (that was something I consciously left out to reduce the amount of code in the earlier refactor there). Luckily, the architecture is such that a combined texture/sampler can leverage most of the existing logic for the separate cases.
* fixup: reveiw feedback
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Various Fixes to gfx, reflection and emit.
- Fix GLSL emit to properly output `*bitsTo*` functions for `IRBitCast` insts.
- Add line directive mode setting for `ISession`.
- Extend `TypeLayout::getElementStride` to handle `VectorType` case.
- Fix `IDevice::readBufferResource` 's D3D12 implementation to copy only the requested bytes out.
- Fix `render-test` to use the `ISession` from `gfx` instead of creating its own `ISession` to make sure `gfx` and `render-test` agree on WitnessTable and RTTI IDs.
- Extend `render-test` to support filling vector and matrix values in the new `set x = ...` TEST_INPUT syntax.
- Add a `dynamic-dispatch-15` test case to make sure packing / unpacking works correctly across all targets, and to make sure render-test's RTTI/WitnessTable ID filling logic is correct for non-trivial cases.
* Remove default-major test
* Fix cyclic reference in `ExtendedTypeLayout`.
* Move `lineDirectiveMode` setting to `TargetDesc`.
Add `structureSize` to `TargetDesc` and `SessionDesc` for future binary compatibility.
* Cleanup.
Co-authored-by: Yong He <yhe@nvidia.com>
|
| | |
|
| |
|
| |
Co-authored-by: T. Foley <tfoleyNV@users.noreply.github.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change applies to the case where we have a sub-object range with more than one object in it (so arrays of constant buffers, parameter blocks, or existentials). It is worth noting that we don't really seem to have any tests covering this case right now, so it is entirely possible that things are busted already, and/or that this change doesn't work the way I think it does.
When we go to actually bind the state from a sub-object range into the pipeline, we care about:
* The offset to where the first object should be written
* The "stride" between consecutive objects
We were already capturing the offset information from Slang reflection data, and the same was true for the part of the offset that pertains to "pending" ordinary data. For other cases, though, we were manually incrementing the offset by values computed manually in `gfx` for Vulkan, and we were just skipping the offsetting step entirely for D3D11.
With this change we extract more complete stride information from the Slang reflection data and use that instead of ad hoc computations. Hopefully this data is correct, and if it isn't we can consider whether it needs fixing at the Slang reflection level rather than in `gfx`.
This change doesn't apply to the OpenGL back-end (which hasn't been updated to match the new approach to static specialization at all) or to the D3D12 back end (which has been updated a bit, but probably needs other cleanup work to be done first to bring it in line).
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Update gfx back-ends to handle static specialization
The main goal here is to make the D3D11, D3D12 and Vulkan back-ends support static specialization of interface types in the case where the data for the type won't "fit" in the pre-allocated space for existential values. This includes all cases where the concrete type being specialized to has resources/samplers/etc., as well as any cases where its ordinary/uniform data exceeds the space available.
(Note that the CPU and CUDA targets don't need this work since they can (in theory) support arbitrary-size data in the fixed-size existential payload by using pointer indirection. Actually supporting indirection in those cases should be a distinct change)
The Slang compiler already performs layout for programs that have this kind of data that doesn't "fit," and it lays them out using an idea of "pending" type layouts. Basically, a type that contains some amount of specialized interface-type fields will produce both a "primary" type layout that just covers the data for the unspecialized case, as well as "pending" type layout that describes the layout for all the extra data needed by specialization.
When laying out a `ConstantBuffer<X>` or `ParameterBlocK<X>` ("CB" or "PB"), the front-end will try to place as much of that "pending" data into the layout of the buffer/block itself as is possible. That means that both CBs and PBs will be able to allocate trailing bytes for any ordinary data in the "pending" layout. PBs will be able to allocate any trailing resources/samplers into their layout, but for CBs they will spill out to be part of the pending layout for the buffer itself.
In order for the back-ends to properly handle pending data, they need to *either* assume the exact layout rules used by the front-end and try to reproduce them (e.g., by iterating over binding ranges and sub-objects in the exact same order that front-end layout would enumerate them), *or* they need to respect the reflection information produced by the front-end. This change takes the latter approach, trying to make only minimal assumptions about the layout rules being used. This choice is motivated by wanting to decouple the `gfx` implementation from the compiler front-end, especially insofar as this work has made me question whether the current layout rules are the best ones possible.
A common theme across all the implementations is to have a fixed-size type that can represent "binding offsets" for the chosen back-end. The offset type has fields that depend on the API-specific way bindings are indexed; e.g., for D3D11 it has offsets for CBV, SRV, UAV, and sampler bindings. This fixed-size offset type can be filled in based on Slang reflecton information, and then used to compute derived offsets with just a few add operations.
The simple offset type for each API is then extended to produce an offset type that includes both the offsets for "primary" data and also the offsets for "pending" data. Most logic that traffics in offsets doesn't have to know about this more complicated representation.
Making consistent use of these offsets required that I pretty much rewrite the logic that actually applies shader objects to the API state. Doing so might be lowering the efficiency of the system in the near term, but the increase in clarity was important for getting the work done, and it seems like it will also be important if/when we start trying to perform special-case optimizations around root and entry-point parameter setting.
While there are many API-specific differences, we can identify a repeated pattern where many steps, whether applying parameters to the pipeline stage or constructing signatures / layouts, can be broken down into three main operations on `ShaderObject`s or their layouts:
* `*AsValue()` is the core operation, and is the one used for the `ExistentialValue` case most of the time. It ignores the ordinary data in the object, and instead processes all nested binding ranges (for resources/smaplers) and sub-objects.
* `*AsConstantBuffer()` handles the `ConstntBuffer<X>` case, by dealing with the implicit buffer for ordinary data (if it is needed) and then delegates to the `*AsValue()` case.
* `*AsParameterBlock()` handles the `ParameterBlock<X>` case, by allocating/preparing/etc. any descriptor tables/sets that would be required for the current object/layout and then delegating to `*AsConstantBuffer()` to do the rest
The idea is that by having the parameter block case delegate to the constant buffer case, which delegates to the value/existential case, we can streamline a lot of the logic so that it doesn't seem quite as full of special cases.
Note: When preparing this pull request I spent a reasonable amount of time trying to clean up the D3D11 and Vulkan implementations, so they are probably the easiest to read and understand when it comes to the new code. Doing the cleanup work also helped to work out some weird corner case bugs/issues. In contrast, the D3D12 path hasn't had as much attention given to cleanliness and comments, so it really needs some attention down the line to get things into a state that is easier to understand.
* fixup: remove debugging code spotted in review
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Fixing `PseudoPtr` legalization and `gfx` lifetime issues.
* Fixing `model-viewer` example.
This change contains various fixes to bring `model-viewer` example to fully functional. These fixes include:
1. Add `spReflectionTypeLayout_getSubObjectRangeSpaceOffset` function to return the space index for a sub object referenced through a `ParameterBlock` binding.
2. Make sure `D3D12Device` specifies column major matrix order creating a Slang session.
3. Fix `platform::Window::close()` and `platform::Application::quit()`.
4. Fix memory leak during `model-viewer''s model loading.
5. Fix command buffer recording in `model-viewer`.
With these changes, model viewer can now produce an image with a gray cube. The lighting is still incorrect becuase the `gfx` shader object implementation still does not handle "pending layout" resulting from global existential parameters.
* Fix d3d12 root signature creation.
* Use row-major matrix layout in model-viewer
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change originated as an attempt to re-enable a test case, but it has ended up disabling more tests (for good reasons) than it re-enables.
The main change here is a significant overhaul of the way that the D3D12 render path extracts information from the Slang reflection API to produce a root signature.
There were also some supporting fixes in the reflection information to make sure it returns what the D3D12 back-end needed.
The big picture here is that the D3D12 path now uses the descriptor ranges stored in the reflection data more or less directly.
It still needs to use register/space offset information queried via the "old" reflection API, but it only does so at the top level now, for the program and entry points themselves.
All other layout information is derived directly from what Slang provides.
Smaller changes:
* The "flat" reflection API was expanded to include `getBindingRangeDescriptorRangeCount()` which was clearly missing.
* The "flat" reflection results for a constant buffer or parameter block that didn't contain any uniform data and was mapped to a plain constant buffer needed to be fixed up. That logic is still way to subtle to be trusted.
* Several additional tests were disabled that relied on static specialization, global/entry-point generi type parameters, structured buffers of interfaces or other features we don't officially support with shader objects right now. All of the affected tests were somehow passing by sheer luck and because they often passed in specialization arguments via explicit `TEST_INPUT` lines.
* The `inteface-shader-param` test is re-enabled now that we can properly describe its input with the new `set` mode on `TEST_INPUT`
* `ShaderCursor::getElement()` can now be used on structure types (in addition to arrays) to support by-index access to fields
* The `TEST_INPUT` system was expanded to support both by-name and by-index setting of structure fields for aggregates
* The `TEST_INPUT` system was expanded to allow an `out` prefix to mark parts of an expression as outputs on a `set` lines
* The `TEST_INPUT` system was expanded so that anything that would be allowed on a `TEST_INPUT` line by itself (like `ubuffer(...)`) can now be used as a sub-expression on a `set` line
Co-authored-by: Yong He <yonghe@outlook.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* Improve Vulkan shader-objects implementation.
1. Null bindings no longer crashes.
2. No longer copies push constants to staging CPU buffer before setting it into command buffer. The entry-point shader object now directly sets it into command buffer upon `bindObject` call.
* Update comments
* Fix
* Re-enable 3 tests.
Improved vulkan implementation so that each shader object is responsible for creating descriptor sets on-demand.
Fixed slang reflection to correctly report `ParameterBlock` binding.
* Fix gcc compile error.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
* Add `SampleGrad` overload for lod clamp.
* Fix gfx to run the test on vulkan.
* Whitespace change to trigger CI build
* remove presentFrame call in render-test
Co-authored-by: Yong He <yhe@nvidia.com>
Co-authored-by: Tim Foley <tfoleyNV@users.noreply.github.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The purpose of these changes is to make the `shader-object` example work correctly on CUDA.
Originally I had tried to add changes to the "flat" reflection information so that it introduced descriptor ranges to match the binding ranges it added for interface/existential-type fields. This approach helped the CUDA code that was using that information to try and compute uniform offsets for those fields, but it broke most of the other renderer back-ends. Instead, I removed the relevant asserts from `CUDAShaderObject::setObject()`.
Note taht there are leftover changes from my edits to the flat reflection information, around how it handles "leaf" fields that consume multiple resource kinds. I believe that those changes are, on balance, "more correct" now than they were before, so I decided to leave them in.
The other major fix here is to specialize the `CUDAShaderObject::setObject()` logic to handle the case of setting a shader object for a parameter that has interface type instead of a constant-buffer or parameter block. Mostly I just copy bytes from the child object into the parent object. There are a few caveats, though:
* I am not writing the RTTI or witness-table information, so dynamic dispatch won't work.
* I am assuming a hard-coded offset of 16 bytes for the any-value, which will work for now but is a bit too "magical" and might also break once we support conjunctions of interfaces with dynamic dispatch
* I am assuming that the child value to be writen into the field will "fit" into the any-value area. We need some way to determine whether or not things fit dynamically (ideally using the reflection data), and adapt accordingly.
* I had to add another method on the base CUDA shader object type to handle setting data using a device-memory pointr instead of a host-memory pointer
* There's not a lot we can do about it, but in the case of assigning an ordinary `CUDAShaderObject` into an interface-type field of a `CUDAEntryPointShaderObject` we end up needing to perform a device->host memory copy, because the bytes of the value will have already been written to GPU memory, but need to be in GPU memory for the dispatch call.
* The implementation I'm using here basically assumes that the child shader object must have been finalized before it gets plugged into the parent shader object. We haven't yet made a policy decision about that bit.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* #include an absolute path didn't work - because paths were taken to always be relative.
* WIP: First pass in supporting output of line error information.
* Add support for lexing to better be able to indicate SourceLocation information.
* Fix lexer usage in DiagnosticSink in C++ extractor.
* Update diagnostics tests to have line location info.
* Fixed test expected output that now have source location information in them.
* Better handling of tab.
* Fix test expected results for tabbing change.
* DiagnosticLexer -> DiagnosticSink::SourceLocationLexer
Added line continuation tests.
* Fix typo.
* Added String::appendRepeatedChar
* Change to rerun tests.
Co-authored-by: Tim Foley <tfoleyNV@users.noreply.github.com>
|
| |
|
|
| |
of existential types. (#1667)
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change converts a large number of our existing tests to use the `ShaderObject` support that was added to the `gfx` layer.
In many cases, tests were just updated to pass `-shaderobj` and the result Just Worked.
In other cases, a `name` attribute had to be added to one or more `TEST_INPUT` lines.
For tests that did not work with shader objects "out of the box," I spent a little bit of time trying to get them work, but fell back to letting those tests run in the older mode.
Future changes to the infrastructure will be needed to get those additional tests working in the new path.
Along with the changes to test files, the following implementation changes were made to get additional tests working:
* Because the shader object mode uses explicit register bindings (from reflection), the hacky logic that was offseting `u` registers for D3D12 based on the number of render targets gets disabled (by another hack).
* The "flat" reflection information coming from Slang was not correctly reporting "binding ranges" for things that consumed only uniform data (which would be everything on CUDA/CPU), so it was refactored to properly include binding ranges for anything where the type of the field/variable implied a binding range should be created (even if the `LayoutResourceKind` was `::Uniform`).
* A few fixes were made to the CUDA implementation of `Renderer`, in order to get additional tests up and running. Most of these changes had to do with texture bindings, which hadn't really been tested previously.
In addition, a few changes were made that were attempts at getting more tests working, but didn't actually help. These could be dropped if requested:
* As a quality-of-life feature (not being used) the `object` style of `TEST_INPUT` line is upgraded to support inferring the type to use from the type of the input being set.
* Any `object` shader input lines get ignored in non-shader-object mode.
|
|
|
* #include an absolute path didn't work - because paths were taken to always be relative.
* Move reflection to reflection-api.
* Slight reorg to pull out potentially Slang internal functions from the reflection API impls.
* Remove visual studio projects
* Fix for slang-binaries copy.
* Add the visual studio projects in build/visual-studio
* Remove miniz project.
* Differentiate the linePath from the filePath.
* Improve comment in premake5.lua + to kick of CI.
* Kick CI.
|