| Age | Commit message (Collapse) | Author |
|---|
|
Fixes issue #6533
This patch updates handling of Array and ConstantBuffer types for WGSL
transpiling, giving correct syntax for arrays of buffers in WGSL.
|
|
* Add GetDimensions support for CUDA
This CL adds GetDimensions support for cuda by using the PTX
instructions. Currently, PTX only supports getting width, height and
depth.
This CL also adds a new test to test this support.
Fixes #5139
* format code
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
# Make `IRWitnessTable` Hoistable
## Intention of the PR
This commit makes `IRWitnessTable` Hoistable so that we can avoid duplicated `IRWitnessTable`.
## Problems
This commit tries to address the following issues arise after turning `IRWitnessTable` into Hoistable:
1. A Hoistable instance is immutable.
2. When tries to create a duplicated child, you will get a previously created instance of `IRWitnessTable`, instead of a new one.
3. We don't actually want to hoist `IRWitnessTable`.
4. There can be only one instance of Hoistable and it cannot appear as childs multiple times.
5. Different import/export mangled names were used for the same Witness-table when its type is "enum" interface.
## Implementation
### Solution for "1. A Hoistable instance is immutable."
`IRWitnessTable::setConcreteType()` is removed, because when an `IRInst` is Hoistable, it is treated as immutable. Any `IRInst::setXXX()` methods don't work anymore.
There were two places calling `setConcreteType()` and their logic had to change little bit.
`DeclLoweringVisitor::visitInheritanceDecl()` in `source/slang/slang-lower-to-ir.cpp` was calling `setConcreteType()`. It had a little strange logic around `lowerType()`. The `IRWitnessTable` was added with `context->setGlobalValue()` first and its `concreteType` was changed later. This commit works around in a way that it sets the parent of `IRWitnessTable` temporarily and reset it with the correct `IRWitnessTable`. Without this logic, it went into an infinite recursion.
`AutoDiffPass::fillDifferentialTypeImplementation()` in `source/slang/slang-ir-autodiff.cpp` was calling `setConcreteType()`. It was changing the concreteType of `innerResult.diffWitness`. This commit creates a new `IRWitnessTable` and copies its `IRWitnessTableEntry`.
### Solution for "2. When tries to create a duplicated child, you will get a previously created instance of IRWitnessTable, instead of a new one"
After a call to `IRBuilder::createWitnessTable()`, this commit checks if the returned `IRWitnessTable` is a brand new or not. If it is not a new one, we have to avoid adding the decorations and children.
This commit decides when to add decorations and children based on whether `IRWitnessTable` has any of decorations or children already. It doesn't seem like a proper way to check. But when I tried, it was difficult to find a bottleneck point where the decorations and children are added to `IRWitnessTable` first time. Note that we are not trying to find when `IRWitnessTable` is created for the first time; we need to find if the decorations and children were added once.
It might be fine to have duplicated `IRWitnessTableEntry` in most of the cases, but I noticed that it fails an assertion check when `shouldDeepCloneWitnessTable()` returns false in `cloneWitnessTableImpl()`.
### Solution for "3. We don't actually want to hoist IRWitnessTable."
The reason why this commit makes `IRWitnessTable` is to prevent the duplicated instances of `IRInst`. But we don't really want to "Hoist" them.
When an `IRWitnessTable` gets Hoisted out, it causes unexpected problems and the specialization process fails due to the missing `IRWitnessTable` in the input.
This commit prevent from hoisting `IRWitnessTable` in `_replaceInstUsesWith()`. The way this is implemented feel little hack but we discussed on Slack and decided to go with this. One of the proper approaches could be to add a new flag in `IROpFlags` and have a new one like `kIROpFlag_Deduplicate`, which is different from just `kIROpFlag_Hoistable`.
### Solution for "4. There can be only one instance of Hoistable and it cannot appear as childs multiple times."
When `IRWitnessTable` is Hoistable, there can be only a unique set of instances. And we cannot have an instance as a duplicated childs. It is because `IRInst` has only one set of `IRInst* next` and `IRInst* prev`.
Before this commit, an instance of `IRGeneral` could have duplicated instances of `IRWitnessTable`. As an example, `IInteger` interface inherits two other interfaces, `IArithmetic` and `ILogical`. And they both inherits from `IComparable`.
```
interface IInteger : IArithmetic, ILogical {}
interface IArithmetic : IComparable {}
interface ILogical : IComparable
```
When we specialize it in `specializeGenericImpl()`, an `IRBlock` gets the following list of children:
- IRWitnessTable for IComparable,
- IRWitnessTable for IArithmetic,
- IRWitnessTable for IComparable,
- IRWitnessTable for ILogical,
For the cloning during the specialize, "IRWitnessTable for `IComparable`" must be cloned before the cloning of "IRWitnessTable for `IArithmetic`". Because "IRWitnessTable for `IArithmetic`" refers "IRWitnessTable for `IComparable`" as its `IRWitnessTableEntry`. The order they appear in the `IRBlock` as children decides which instances will be cloned first. And "IRWitnessTable for `IComparable`" must appear before "IRWitnessTable for `IArithmetic`".
Note that "IRWitnessTable for `IComparable`" appears twice, The first one was added for "IRWitnessTable for `IArithmetic`". And the second one is added for "IRWitnessTable for `ILogical`".
With this commit "IRWitnessTable for `IComparable`" can appear as a child only once in `IRBlock`. So it causes an error if it gets the following list:
- IRWitnessTable for IArithmetic,
- IRWitnessTable for IComparable,
- IRWitnessTable for ILogical,
In order to resolve the problem, "IRWitnessTable for `IComparable`" must appear before both "IRWitnessTable for `IArithmetic`" and "IRWitnessTable for `ILogical`" as following:
- IRWitnessTable for IComparable,
- IRWitnessTable for IArithmetic,
- IRWitnessTable for ILogical,
To address the problem, the instances of `IRWitnessTable` is always added to the end of the children list. If it is already added to the list, we don't move. This works out because the AST tree is built based on the dependencies.
### Solution for "5. Different import/export mangled names were used for the same Witness-table when its type is "enum" interface."
This issue was found while testing with Falcor tests where it uses Conformance-type feature of Slang.
We are using different import and export mangled names for a same Witness-table when the witness-table is for "Enum" interface.
The way we simplify the implementation of "Enum" causes a problem when it comes to generate export/import for the witness-table. And the exact repro step is still unclear.
There were two suggested solutions for the problem and this PR adopted the first option for now. Maybe we want to improve it with the second option later.
option 1, when we produce mangled names for those witness-table, we can use a mangled name with the underlying "int" type instead of the name of the enum type. In this way, all witness-tables for enum types whose underlying type is same will get the same mangled name. It will allow us to deduplicate the witness-table during the linking.
option 2, we can preserve type info for enum type when generating IR. We can still erase all other uses of the type info of enum types for now. But when we generate the witness-table, instead of filling the conforming type operand to IntType, we fill it as EnumType(IntType) where EnumType is a new global IROp code to represent all enum types (like InterfaceType/StructType). This way the operands for the two witness-tables will be different.
"option 1" is more quick and dirty and "option 2" is more proper way to address it.
I should go with "option 1" and improve it with "option 2" approach later.
|
|
* Include generics' operands in call graph construction
* add test
|
|
This reverts commit b3deec2001ea34e20e9a6af8ddf5cf3866cafac0.
|
|
close #6694
We should return nullptr when findAndValidateEntryPoint fails to valid
the entrypoint.
|
|
* Eliminate empty struct on metal target
Close 6573.
We previously disabled the type legalization for ParameterBlock on
Metal, but Metal doesn't allow empty struct in the argument buffer
which is mapped from ParameterBlock, so we will need legalizeEmptyTypes
on Metal target.
* update test
* update function name
|
|
* Fix SPV_KHR_maximal_reconvergence extension name spelling
Vulkan validation layers emit warnings on lowercase khr.
* Move OpExtension check
|
|
* Fix mul operator followed by global scope
This should fix expr like `2.0f * ::a::b::c`.
But it will no longer parse something like
```
extension<T> Ptr<T> { static void foo(); }
int*::foo() // won't work, but this is a less common case
```
Fixes #6684
* Update simpe-namespace.slang to test global scope
|
|
Close #6541.
Previously in type legalization pass, we skip the VoidType field when call make_struct, however in some optimization pass we keep counting the VoidType field. We have to make this behavior consistently over all our codebase.
So in this change, we spot the make_struct call and leave VoidType field as it.
|
|
* Don't load cached builtin module in slang-bootstrap.
* Fixes.
* format code
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
|
|
|
|
* spirv: add support for ops added by multiple extensions
Some spirv ops are added by multiple extensions and capabilities. This
commit adds support to avoid emitting unnecessary extensions and
capabilities if one of the options is already required by some other op.
* spirv: allow OpRaytracingAccelerationStructure to use multiple extensions
This Op is provided by both SPV_KHR_ray_tracing and SPV_KHR_ray_query
and the respective capabilities. Use one if already available and
otherwise fall back to SPV_KHR_ray_tracing.
* tests/vkray: add negative checks for RayTracingKHR and RayQueryKHR
- Add new rayquery-compute.slang to test that only RayQueryKHR is needed
in compute shaders.
- Add checks for RayTracingKHR and RayQueryKHR capabilities and
extensions in raygen.slang
---------
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
* initial wip
* more WIP
* preserve old lower behavior
* remove unnecessary includes
* add test
* add no target case in test
* fix broken test
---------
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
The previous implementation had two issues in the modifier processing loop:
1. isConst was incorrectly initialized to true, making the const check redundant
2. Premature loop break could skip processing important modifiers. e.g.
isExtern
Changes:
- Initialize isConst to false by default
- Remove early break condition to process all modifiers
Fixes: #6606
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
* Add -dump-module command to slangc
The new -dump-module command to slangc will load and disassemble a slang module, similar to what would be seen by the -dump-ir command, except that -dump-ir tells slangc to print IR as it performs some compilation command. That is, -dump-ir requires
some larger compilation task.
-dump-module on the otherhand requires no additional goal and will simply load a module and print its IR to stdout independently from other compilation steps.
Its intended purpose is to inspect .slang-module files on disk.
It can also be used on .slang files which will be parsed and lowered
if slang does not find an associated ".slang-module" version of the
module on disk.
The compilation API is extended with a new IModule::disassemble()
method which retrieves the string representation of the dumped IR.
Closes #6599
* format code
* Use FileStream not FILE
* format code
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
* image format json reflection
* format code
* use direct include
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
extension (#6641)
* Output SPV_KHR_compute_shader_derivatives instead of the NV extension
* add alias for nv extension
|
|
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
* Fix reinterpret and bitcast.
* Fix warning.
* Fix.
* Fix.
---------
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
* Implement floating-point pack/unpack intrinsics
* remove unused functions and update caps in glsl meta file
* rename pack capability
|
|
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
|
|
* Fix crash when swizzling non-differentiable types
* Update slang-ir-autodiff-fwd.cpp
|
|
- Add the diagnostic messages, and code to emit them
- Add some tests
This helps to address issue #6183.
|
|
* Fix lowering of associated types in generic interfaces.
* Update diff-assoctype-generic-interface.slang
* Fix-up lowering of differentiable witnesses for implicit ops
* Update slang-ir-autodiff-transcriber-base.cpp
* Fix issue with differentiating type-packs
|
|
* Implement structured buffer GetDimensions for WGSL
* byte address buffer fix
* enable wgsl test and minor fixups
* maybe fix get equivalent structured buffer
* remove unnecessary include
* clean up some code in meta file
|
|
dependency (#6594)
* Simplify text writing for core module headers
* Use an explicit dependency for generated headers instead of source file dependency
From the CMake Docs
> Do not list the output in more than one independent target that may build in parallel or the instances of the rule may conflict. Instead, use the add_custom_target() command to drive the command and make the other targets depend on that one. See the Example: Generating Files for Multiple Targets below.
---------
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
No functional change as overallocating was ok, but this was wrong
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
* initial wip
* more wip
* add test
* add unexpected for invalid target
* fixups and improve error message
* fixups and improve error message
* remove incorrect comment
---------
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
In the legacy compile request based API, the referenced modules are added to the request's
linkage libraries as part of compiler option parsing.
In the non-legacy compilation API, the argument parsing creates a temprary compile request
and so those libraries only survive as options.
This change will look for such options when creating an ISession object, and again add the
referenced modules to the libraries of the new linkage that's contained in the ISession
object.
This is done in two steps:
1. Factor out a helper to create a referenced module artifact in the same way as it's done
during legacy option parsing.
2. Use the helper function to create artifacts to add to the linkage libararies, when the
session is created.
This helps to address issue #4760, because it enables passing in downstream modules via
options, as is required for the following tests:
tests/library/library-test.slang.2 (dx12)
tests/library/export-test.slang.2 (dx12)
|
|
Co-authored-by: Jay Kwak <82421531+jkwak-work@users.noreply.github.com>
|
|
Abort precompileForTarget if it's already done.
Fixes #6516
|
|
* IR: Add SPIR-V disassembly for embedded downstream IR dumps
When dumping IR that contains embedded downstream SPIR-V code (via
EmbeddedDownstreamIR instructions), display the disassembled SPIR-V
instead of just showing "<binary blob>".
This CL also does:
- Adds a new interface for disassembly and get result.
- Modify export-library-generics.slang test test to check for the
disassembled SPIR-V
Fixes #6513
* Add module-dual-target-verify test
Fixes #6517
Adds a new test to verify that dxil and spirv targets are stored
separately in the precompiled blob.
* Fix review comments from cheneym2
* format code
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
Precompiled SPIR-V bits can independently resolve to different
versions of SPIR-V.
To avoid a linker error about mismatched versions, use a linker
feature to automatically use the highest version listed in
the modules.
Fixes #6548
|
|
* Add module reference options to options list when parsing them
Previously, the module was just added to the compile request directly.
This is a problem because the non-deprecated command line option parsing interface
IGlobalSession::parseCommandLineArguments just creates a temporary compile request.
Therefore these options are currently not detected by the API.
This helps to address issue #4760 since it will need to access these modules in order to
supply all of the required code.
* format code
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
profile (#6538)
If the profile/capability in options set, but set to unknown, then don't count the
setting as requesting a particular profile/capability.
This helps in addressing issue #4760 because the new session-based compilation API always
adds a profile setting via Linkage::addTarget, even if the profile is unknown.
That caused lots of tests to fail due to unexpected output:
"warning 41012: entry point 'computeMain' uses additional capabilities that are not part of the specified profile..."
The old compilation request -based API did not add profile/capabiltiy to the options, and
so did not generate this warning.
|
|
|
|
* Fix a bug in default ctor synthesizing
- This is fix for the implementation bug, when a struct has explicit ctor
we should not synthesize the default ctor anymore.
- When invoke the synthesized ctor converted from initializer list, we should
check if the struct is a c-style type if it struct has no synthesized ctor. In this
case we should report error because it's invalid to use initializer list here.
- The only exception is the unsized array, we still have to fall back to use the
legacy initializer list logic to initialize the unsized array until we formalize a
proper solution.
- update test.
|
|
* Update build to allow setting external paths
Update the build to allow setting user-specific paths for the external modules.
This allows building Slang without also fetching the external modules, assuming
they are already present elsewhere locally.
|
|
* Fix lowering of `extern` types with defaults.
* Fix test.
* Fix test.
|
|
* Update SPIRV-Tools and fix new validation errors.
* Implement pointers for glsl target.
* Reworked packStorage/unpackStorage code gen to operate on pointers rather than values.
|
|
This helps to address issue #4760.
The particular issue motivating this fix is that
IGlobalSession::parseCommandLineArguments uses a temporary compile request to parse
options.
The compile request only adds the OptionKind::ForceDXLayout (-fvk-use-dx-layout) to the
"current target" which may be the default target, which
IGlobalSession::parseCommandLineArguments didn't look for, meaning that options like
ForceDXLayout were just ignored.
This leads to test failures in tests that rely on this option, e.g.
tests/spirv/cbuffer-dx-layout-1.slang.
|
|
The new option "SkipDownstreamLinking" will defer final downstream IR
linking to the user application. This option only has an effect if
there are modules that were precompiled to the target IR using
precompileForTarget().
Until now, the default behavior for SPIR-V was to use deferred linking, and
the default behavior for DXIL was to use immediate/internal linking in Slang.
This change only affects the SPIR-V behavior such that both deferred and
non-deferred linking is supported based on the new option.
To support the non-deferred option, Slang will internally call into
SPIRV-Tools-link to reconstitute a complete SPIR-V shader program when
necessary (due to modules having been precompiled to target IR).
Otherwise, if SkipDownstreamLinking is enabled, the shader returned by
e.g. getTargetCode() or getEntryPointCode() may have import linkage to
the SPIR-V embedded in the constituent modules.
Closes #4994
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
* Add cuda codegen bug repro
This just compiles tests/compute/simlpe.slang for PTX with the new compilation API, in
order to reproduce a code generation bug.
* Detect entrypoint more robustly when applying ConstRef hack during lowring
For shaders like tests/compute/simple.slang, which have a 'numthreads' attribute but no
'shader' attribute, the old compile request API would add an EntryPointAttribute to the
AST node of the entry point. However, the new API doesn't, and so a certain ConstRef hack
doesn't get applied when using the new API, leading to subsequent code generation issues.
This patch also checks for a 'numthreads' attribute when deciding whether to apply the
ConstRef hack.
This closes issue #6507 and helps to resolve issue #4760.
* Add expected failure list for GitHub runners
Our GitHub runners don't have the CUDA toolkits installed, so they can't run all tests.
|
|
This helps to address issue #4760.
|
|
* Consolidate multiple inout/outs into struct
Fixes #5121
VUID-StandaloneSpirv-IncomingRayPayloadKHR-04700 requires that there be
only one IncomingRayPayloadKHR per entry point. This change does two
things:
1. If an entry point has the one inout or out, or has only ins, then
stay with current implementation.
2. If there are multiple outs/inouts, then create a new structure to
consolidate these fields and emit this structure.
These two code paths are split into two separate functions for clarity.
This patch also adds a new test: multipleinout.slang to test this.
* Address review comments
* Refactor code as per review comments
* format code
* fix failing tests
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
* Allow partial specialization of existential arguments.
* Fix.
* Add test case for improved diagnostics.
* Fix compile error.
* Fix tests.
* Fix.
* Fix test.
* Fix compile issue.
* Fix typo.
* Address comment.
|
