| Age | Commit message (Collapse) | Author |
|---|
|
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
Work on #6892
The function `tryInferLoopMaxIterations` in `slang-check-stmt.cpp` was
doing:
auto litExpr = m_astBuilder->create<LiteralExpr>();
but the declaration of `LiteralExpr` in `slang-ast-expr.h` had been
marked as abstract, during the switch to using the `slang-fiddle` tool
to generate code:
FIDDLE(abstract)
class LiteralExpr : public Expr
{ ... }
In this case, the intention of the AST design is that `LiteralExpr`
should be kept abstract, and only the concrete subclasses should
ever be instantiated.
Because of some historical design choices, the `ASTNodeType`
enumeration includes both the concrete and abstract AST classes, so
the code ended up constructing a `LiteralExpr` that had the tag
`ASTNodeType::LiteralExpr`. Then attempts to use the `ASTNodeDispatcher`
code on such a node caused crashes, because the dispatcher code only
included `case` statements for the non-abstract `ASTNodeType`s.
The quick fix here was to change the `tryInferLoopMaxIterations`
function to instead do:
auto litExpr = m_astBuilder->create<IntegerLiteralExpr>();
A test case was added to help catch any future regressions on this
specific issue.
A more long-term fix should involve introducing code that statically
and/or dynamically prohibits the creation of instances of AST node
classes that have been marked abstract.
|
|
* Fix #6544: Properly format nested type names in extensions
Modify DeclRefBase::toText to properly handle types defined
in extensions by qualifying them with their parent type name.
This ensures getFullName() returns the full name like
'FullPrecisionOptimizer<half>.State' instead of just '.State'.
Also handle other nested types in structs/classes similarly.
* Update extension reflection handling - with generics args and namespaces
- stopping namespace inclusion for extension members
- Update to use getTargetType() to handle the generic arguments
- update test cases
* Simplify code to remove using parentDecl
|
|
* A new approach to AST serialization
This change completely overhauls the way that AST nodes are being serialized, and the offline source-code generation steps that enable that serialization.
In practice, this ends up being a complete overhaul of the way that *modules* are being serialized (not just the AST part), although things like the serialization format for the Slang IR and for source locations are not affected.
The rest of this commit message is broken down in to sections, in an attempt to help guide anybody looking at the code in how to make sense of all the changes.
The Old C++ Extractor
---------------------
AST serialization used to be driven by information scraped using the `slang-cpp-extractor` tool, which did an ad hoc parse of the C++ declarations of the AST node types and then generated a set of "X macros" that could be for macro-based code generation within the rest of the compiler.
While the existing approach was functional, it wasn't easy to understand or maintain, and it has been getting in the way of forward progress on other features we'd like to work on in the language and compiler.
This change removes the `slang-cpp-extractor` tool entirely.
Marking Up the AST Declarations
-------------------------------
The most notable change that contributors to the compiler may notice is the large number of invocations of a macro `FIDDLE()` on the declarations of the AST node types.
The basic idea is that only declarations (namespaces, types, fields) that are preceded by `FIDDLE()` are visible to the code generator tool.
So if somebody is working with the AST and wondering why a new node type isn't working, or why a field they added isn't being serialized correctly, it is probably because they need to add `FIDDLE()` in front of it.
Generating the Boilerplate Code
-------------------------------
The file `slang-ast-boilerplate.cpp` provides a good example of how the information extracted from the marked-up AST declarations gets used.
In that file, the `FIDDLE TEMPLATE` construct is used to generate type information for each of the AST node types.
Similar logic is used in `slang-ast-forward-declarations.h` to generate the declaration of the `ASTNodeType` enumeration, and forward-declare all the AST node classes.
For many parts of the code, simply including that file replaces the need for the old `slang-generated-*.h` files.
Replacing Visitors and Related Logic
------------------------------------
The old visitor types for the AST used the macros that were generated by `slang-cpp-extractor`, so something new was needed to replace them.
The same goes for the `SLANG_AST_NODE_VIRTUAL_CALL` macros.
The core of the solution implemented here is in `slang-ast-dispatch.h`.
Given a "dispatchable" AST node type (say, `Expr`), a call like:
```
ASTNodeDispatcher<Expr,R>(expr, [&](auto e) { return doSomething(e); })
```
is an expression of type `R`, which does the equivalent of something like:
```
switch(expr->getTag())
{
case ASTNodeType::VarExpr: return doSomething(static_cast<VarExpr*>(expr));
// ...
}
```
The `SLANG_AST_NODE_VIRTUAL_CALL` macro is now implemented in terms of `ASTNodeDispatcher`.
The implementation of the visitor types is more involved.
The code in this change retains some of the macro names from the original version, just to try and make the parallels more clear.
The visitor types are all implemented on top of the `ASTNodeDispatcher` approach, and use `FIDDLE TEMPLATE` to generate all the boilerplate `visit*()` method declarations.
Refactoring of `Linkage` Module Loading
---------------------------------------
Needing to revisit all the places where modules get deserialized made it clear that there is a lot of complexity and apparent duplication in the core routines on the `Linkage` that get used for loading modules.
This change tries to clean up some of that logic, but it is worth noting that there are two legacy features that get in the way of making things as clean as they should be:
* The `LoadedModuleDictionary` type that gets passed around a lot exists entirely to handle the corner case where somebody uses the Slang API to perform a compilation with multiple `TranslationUnitRequest`s in the same `FrontEndCompileRequest`, and one of the translation units `import`s the module defined by another of the translation units.
* There are a lot of special-case behaviors and routines entirely there to support the `ModuleLibrary` feature, although that feature should be considered deprecated (or at least subject to getting entirely re-designed down the line).
The basic idea of the cleanup is that all of the (non-deprecated) ways load a module from a serialized binary, or compile one from source should now bottleneck through `loadModuleImpl`, which then bifurcates into `loadSourceModuleImpl` for the compilation case and `loadBinaryModuleImpl` for the deserialization case.
High-Level Serialization Approach
---------------------------------
The old serialization logic used the [RIFF](https://en.wikipedia.org/wiki/Resource_Interchange_File_Format) format to encode the high-level structure of things, and this change retains that usage (and actually doubles down on the RIFF usage).
The old serialization system relied on the idea that for any given type `Foo` that wants to support serialization, there should be something like a `SerialFooData` type in C++, that can represent the state of a `Foo`, and then the actual serialization applied to that `SerialFooData`. This means that in most cases there are four pieces of code written:
* During serialization:
* Copying the data of a `Foo` in memory over to a `SerialFooData` in memory
* Writing the state of a `SerialFooData` into the serialized data stream
* During deserialization:
* Reading the state of a `SerialFooData` from a serialized data stream
* Copying the data of the `SerialFooData` in memory over to a `Foo`
The new logic gets rid of the intermediate `SerialFooData`.
In the serialization direction, we take a `Foo` and write it to the `RIFFContainer` directly, or using some other utilities layered on top of it.
In the deserialization direction, we have additional flexibility. Given a `RIFFContainer::Chunk*` that represents a serialized `Foo`, we often navigate through the in-memory representation of the RIFF data to get to the parts of the serialized value that we actually want/need, without needing to deserialize the entire `Foo`.
To support this kind of operation, this change introduces a few helper types like `ContainerChunkRef` an `ModuleChunkRef`, that are little more than typed wrappers around a `RIFFContainer::Chunk*`.
The Module "Container" Part
---------------------------
A serialized `Module` is encoded as a RIFF chunk, using logic in `slang-serialize-container.cpp` - both before and after this change.
This change reorganizes a lot of the code in that file, to account for the way that eliminating the intermediate `SerialContainerData` type streamlines the overall task of writing out the parts of the module.
In the deserialization logic... there isn't really much to do in `slang-serialize-container.cpp`. Most of the logic in `slang.cpp` and `slang-module-library.cpp` that pertains to deserializing modules uses the `ModuleChunkRef`-based approach, and simply extracts the pieces of the serialized module that it needs.
The Actual Serialization of the AST
-----------------------------------
The actual AST serialization logic is in `slang-serialize-ast.cpp`.
The basic approach in both the writing and reading directions is:
* Use the `FIDDLE TEMPLATE` system to generate a set of functions, one for each AST node type, that recursively invoke the read/write logic on each field of that node (after recursively invoking the case for its direct superclass)
* Use the `ASTNodeDispatcher` system to dispatch out to those functions whene reading or writing anything derived from `NodeBase`
* For now, handle all types *not* derived from `NodeBase` by hand.
There's a lot of room for improvement around that last item: it should be just as easy to generate the serialization and deserialization logic for other types that don't inherit from `NodeBase`, but the current change tries to err on the side of making the logic as explicit and simplistic as possible, rather than trying to get too clever too soon.
The actual serialization *format* used for the AST is almost comically simplistic: the code uses hierarchical RIFF chunks to emulate a JSON-like structure. This is a very wasteful representation (e.g., a `bool` or a null pointer each take up *8 bytes*), but the goal for now is to start with the simplest thing that could possibly work, and only add more cleverness once we are sure it won't get in the way of important future improvements (like lazy/on-demand deserialization or IR and AST, to improve compiler startup times).
The files `slang-serialize.{h,cpp}` have been co-opted to define a new pair of types `Encoder` and `Decoder` that are used for a more-or-less stream-oriented way or reading or writing RIFF chunks for the JSON-like structure.
Almost everything related to the actual AST serialization could do with a cleanup pass, and some time spent on picking good/better names for everything.
Smaller Stuff
-------------
* Cleaned up a lot of code that was using bare `ASTNodeType` or the extractor's `ReflectClassInfo` type to consistently use `SyntaxClass`.
* Fixed an apparent bug in how the destination-driven code genarator was handling `TryExpr`s
* Fixed an apparent bug in how the GLSL legalization pass was handling translation of certain `SV_*` semantics.
* format code
* fixup: template errors caught by non-VS compilers
* format code
* fixup: more template errors
* fixup: more stuff VS didn't catch
* fixup: it's amazing VS doesn't catch these...
* fixup: yet more template stuff VS ignores
* fixup: more VS template nonsense
* fixup: unreachable return macro usage
* fixup: more unreacable returns
* fixup: unused parameter
* fixup: strict aliasing
* fixup: allow missing entry point list chunk
* fixup: wasm build script
* fixup: AST changes since this PR was created
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
* Initial implementation for SPIRV, GLSL and Metal
* test add bool test
* Fix and improve subgroup rotate tests
* Add proper GLSL extensions and proper Metal type checking
* Clean up tests and add diagnostics test for subgroup type for Metal
* Update wave-intrinsics docs
|
|
|
|
* Add struct member offset qualifier for SPIRV
* Implement for GLSL target and add tests
* clean up
* fix formatting
* fix typo
* renamed GLSLStructOffset to VkStructOffset and added emit-spirv-via-glsl test case
|
|
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
|
|
Change modifies the countbits intrinsic to use generics in order to
support 64bit countbits on select platforms where this is supported.
On platforms where this is not natively supported, we emulate by
converting the 64-bit type into a uint2 (metal and spir-v).
This should align with the implementation of other uint64_t
intrinsics such as abs, min, max and clamp.
Added new countbits64 test to verify changes.
Updated documentation for 64bit-type-support.html
|
|
* Added Dictionary::erase(iterator) and fixed crashing when filtering a dictionary in slang-ir-autodiff-loop-analysis.cpp
* Added Dictionary::removeIf(Predicate)
* Removed Dictionary::erase(it)
---------
Co-authored-by: Julius Ikkala <julius.ikkala@gmail.com>
|
|
Close #6589.
In PR #6487, we support partial specialization. However there is a corner case we didn't handle correctly.
For the IR like this:
%val: specialize(...) = some inst;
%arg1: specialize(...) = makeExistential(%val, ...);
%arg2: %SomeConcreteType: load(...);
call func(%arg1, %arg2);
when we specialize the call func instruct, we will also specialize the function parameters.
On our existing logic, when we find an argument is a makeExistential, we will always extract the existential value, and use its type as the new parameter.
But in this case, %arg1 is not fully specialized yet, so it's type will still be a specialize. In this case, we will change the function's first parameter from an existential type to a specialize. This will result in that we lose the chance to specialize the first argument in the next iteration, because the first parameter of this function is not an existential type any more.
The reason behind this is that we should always keep specializing the arguments and parameters at the same time.
So this PR just does a check before specializing the parameters that if the argument cannot be fully specialized, we won't specialize the parameter this early. Instead, we will wait for the next iteration until the argument can be specialized.
|
|
* Add Yet Another Source Code Generator
This change introduces an offline source code generation tool,
provisionally called `fiddle`. More information about the design of
the tool can be found in `tools/slang-fiddle/README.md`.
Yes... this is yet another code generator in a project that already
has too many. Yes, this could easily be a very obvious instnace of
[XKCD 927](https://xkcd.com/927/).
This change is part of a larger effort to change how the AST
types are being serialized, and the way code generation for them
is implemented.
Right now, the source code for the new tool is being checked in and
the relevant build step is enabled, just to make sure everything is
working as intended, but please note that this change does *not*
introduce any code in the repository that actually makes use of
the new generator. All of the AST-related reflection information that
feeds the current serialization system is still being generated using
`slang-cpp-extractor`.
The design of the new tool is primarily motivated by the new approach
to serialization that I'm implementing, and once that new approach
lands we should be able to deprecate the `slang-cpp-extractor`.
In addition, the new tool should in principle be able to handle
many of the kinds of code generation tasks that are currently being
implemented with other tools like `slang-generate` (used for the core
and glsl libraries). This tool should also be well suited to the task
of generating more of the code related to the IR instructions.
* format code
* Build fixes caught by CI
* Fix another warning coming from CI
* Another CI-caught fix
* Change bare hrows over to more proper abort execptions
* format code
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
* Eliminate back-reference in ChildStmt
This change is part of a larger effort to improve the code for AST
serialization in the Slang compiler.
Tree structures are understandably easier to serialize than DAGs,
and DAGs are easier than fully generaal graphs.
The Slang AST nodes form a tree structure... except when they don't.
Among the exceptions to nice tree-structured ASTs are:
1. References to `Decl`s are encoded as pointers to the AST `Decl`
nodes themselves. This can result in cycles in the graph, and
requires care in serialization.
2. Nodes that inherit from `Val` represent, well, *values* instead
of actual pieces of syntax, and as such they are deduplicated so
that identical values will (hopefully) be identical pointers.
This results in a DAG structure for `Val`s, but at least it's not
a general graph (except for cycles that go through a `Decl`).
3. There are some minor cases of DAG-structured sharing that the
parser can introduce to deal with cases when a traditional-style
declaration includes multiple declarators. E.g., given:
```
static int a, b;
```
The resulting `DeclGroup` will include distinct `Decl`s for `a`
and `b`, which will share the `static` modifier through a
`SharedModifiers` node, and the `int` type specifier through a
`SharedTypeExpr` node.
This duplication can be ignored, for the purposes of serialization,
since duplicating those parts of the AST has no major down-sides.
4. There is the case of `ChildStmt`, used for things like `break`
and `continue`, which stores a direct `Stmt*` to the enclosing
parent statement being targetted. Storing the target is useful so
that IR lowering doesn't need to repeat the work that the semantic
checking logic did to associate each child statement with its parent.
The parent link inside of `ChildStmt` creates a cycle in the AST
`Stmt` hierarchy, since the outer statement contains the inner,
and the inner statement stores a pointer to the outer.
This change eliminates the last of these sources of complication for
AST serialization, by changing the `ChildStmt` type to stored an
integer ID for the enclosing statement that it matches to, and having
each `BreakableStmt` (used to represent the outer `switch`, or loop,
or whatever) generate its own unique ID as part of semantic checking.
Note: if necessary, it is reasonable for the outer statement to have
its unique ID generated as part of parsing, rather than semantic
checking.
* format code
* Change unique ID to be a proper Decl
The fix here is to make the "unique ID" representation be a full
`Decl`-derived AST node, so that it is both allowed to break the
tree-structuring rules cleanly, and it is also trivially guaranteed
to be unique across all loaded ASTs.
* format code
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
* Fix compiler warning with clang 18.1.8 on windows
|
|
|
|
* Remove support for ad hoc Slang IR compression
This change is part of a larger effort to clean up the approach to
serialization in the Slang compiler. The overall goal is to simplify
and streamline all of the serialization-related logic, so that we are
left with code that is less "clever," and easier to understand for
contributors to the codebase.
Removing support for compression of serialized Slang IR has
benefits that include:
* Reduction in code complexity: consider things like the subtle way
that the `FOURCC`s for compressed chunks were being computed from
the uncompressed versions, and the mental overhead that goes into
understanding that, for anybody who would dare to touch this code.
* Reduction in testing burden: there have been, de facto, two
very different code paths for serialization of the Slang IR, and
it is not clear that the existing test corpus for Slang has
sufficient coverage for both options. By having only a single code
path, every test that performs any amount of IR serialization helps
with test coverage of that one path.
* Opportunity to explore alternatives. This is perhaps a reiteration
of the first point, but once the code is stripped down to the
simplest thing that could possibly work (I am not claiming it has
reached that point yet), it becomes easier for contributors to
understand, and it becomes more tractable for somebody to come along
with an improved approach that performs better (in either
compression ratio or performance) while still being maintainable.
In my own local setup, I found that removing support for Slang IR
compression led to the `slang-core-module-generated.h` file increasing
in size from 46.1MB to 47.4MB. This increase in the `.h` file size
for the core library binary only resulted in a release build of
`slang.dll` increasing from 20.0MB to 20.2MB. Removing the ad hoc
compression support has almost no impact on the size of actual binary
Slang modules *so long* as the additional LZ4 compression step is
being applied to them.
* format code
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
|
|
* Use the latest Ubuntu version not specific old version
|
|
* initial wip for spirv
* working tiled example
* clean up store and load
* minor fixes
* fix loadAny name
* add initial tests, including broken/unimplemented intrinsics
* fix subscript
* run tests at 16x16, remove not supported arithmetic tests
* minor fixups on implementation
* rename CoopMatMatrixUse
* Update tests to pass validation layers locally
* Add mat-mul-add test and minor fixes
* Add more tests
* Remove dead code
* Add coopMatLoad function and tests, enforce constexpr for matrix layout
* Use getVectorOrCoopMatrixElementType in place of getVectorElementType
|
|
Documenting CoopVec related functions.
This commit also fixes a few warning printed from the doc generation tool.
Some of comments are removed or converted from /// to //, because the overloading functions can have /// style comment only once.
|
|
* Consume `;` after parsing typedef decl.
* Fix.
* Fix regressions.
|
|
* Fix matrix division by scalar for Metal and WGSL targets
* Add tests
* Minor fix
* Fix compilation error
* Convert to multiplication for WGSL
* Minor cleanup
---------
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
Co-authored-by: Simon Kallweit <simon.kallweit@gmail.com>
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
* Fix User Attribute string reflection
Fixes #6794
* Fix strings not being properly escaped
---------
Co-authored-by: Darren Wihandi <65404740+fairywreath@users.noreply.github.com>
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
* Add gl_PointCoord support in GLSL compat mode
* Add SV_PointCoord
* Test on metal as well
* Update SPIRV system value semantics table in docs
* Update metal docs for SV_PointCoord
|
|
* Fix pointer field access for GLSL
* Add test
* Fix SPIRV test
* add spirv via glsl test
|
|
This fixes issue #6654
Only hoist instructions that are optimized by prepareFuncForForwardDiff.
Add flag hoistLoopInvariantInsts to IRSimplificationOptions and set this
to true only if called from prepareFuncForForwardDiff, then only hoist
if the flag is set. Additionally, do not hoist loops if they only have a
single trivial iteration.
|
|
interface-typed output parameter (#6788)
* More specific diagnostic for invalid concrete-to-interface arg coercion
* Add test for the new error message
* Fix typo in expected test result
|
|
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
* Get real value for typeAdapter
When the type is mismatch and typeAdapter is used, get the real value
from typeAdapter so that we don't get nullptr for irValue.
This fixes the assert if uint is used for SV_VertexID, which is an int
in the system binding semantic.
Fixes: #6525
* Add test case; add nullptr check
|
|
* void field rework
* move void cleanup pass earlier
|
|
Closes https://github.com/shader-slang/slang/issues/5995
|
|
Fixes #6624
This commit changes the behavior of getArgumentValueString() to return
the string's value, instead of returning the string's token,
as that token also contains the surrounding quotation marks.
This commit also modifies the relevant unit test accordingly,
to not check for the surrounding quotations.
|
|
* Add support for Ray Payload Access Qualifiers (PAQs) (#3448)
- Added [raypayload] attribute for struct declarations
- Implemented field validation requiring read/write access qualifiers
- Added diagnostic error for missing qualifiers
- Enabled PAQs in DXC compiler and HLSL emission
- Added new test demonstrating PAQ syntax
- Implemented proper handling of ray payload attributes in IR generation
* format code
* Cleanup: Remove unused vars
* Add check to enablePAQ only for profile >= lib_6_7
* Review Fix - Add PAQ support for DX Raytracing
add enablePAQ flag to DownstreamCompileOpitons, improve PAQ handling
update raypayload-attribute-paq.slang to ensure hlsl and dxil is
validated
* Add diagnostic test for missing paq for lib_6_7
Compile using `-disable-payload-qualifiers` aka lib_6_6 profile
raypayload-attribute-no-struct.slang and
raypayload-attribute.slang
---------
Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Ellie Hermaszewska <ellieh@nvidia.com>
|
|
|
|
(#6651)
* Fix incorrect assert on mixed global uniform and varyings
* add test
* remove unnecessary include
* fix incorrect logic
* fix comment grammar
* address review comments and improve test
* minimize diff
* fix more issues for cuda build
* remove unnecessary line for diff
|
|
|
|
(#6696)
* Initial loop analysis pass
* More changes for a single-pass implication propagation
* Update slang-ir-autodiff-loop-analysis.cpp
* Cleanup + new system for loop analysis
* Fixup bugs in loop analysis
* Remove some relation types to simplify the analysis. Add test
* Remove unused
* Address comments
* Fix issue with continue loops
* Update reverse-loop-exit-value-inference-1.slang
* Update reverse-continue-loop.slang
|
|
output) function outputs (#6737)
Closes https://github.com/shader-slang/slang/issues/6632
|
|
* Implement sparse texture Load intrinsics for SPIRV
* changed test name from TEST_load to TEST_sparse
---------
Co-authored-by: Darren Wihandi <65404740+fairywreath@users.noreply.github.com>
|
|
* Fixed generic interface specialization crashes:
- Add an export decoration to specialized generic interfaces.
* Fixed generic interface specialization crashes:
- Add an export decoration to specialized generic interfaces.
- Use getTypeNameHint(...) instead of a manual mangler.
* In cloneInstDecorationsAndChildren: specialize all linkage decorations, not just the exports.
- If a linkage decoration is already present, it is not specialized and replaced by the specialized one.
- If a specialization uses the TypeNameHint, sanitize it to be used as an identifier.
- Use the identifier name sanitizer from slang-mangle.
* Added tests/generics/generic-interface-linkage.slang
- See #6601 and #6688
|
|
* Reapply "Eliminate empty struct on metal target (#6603)" (#6711)
This reverts commit bc9dc6557fc0cc3a4c0c2ff27e636940e361cf5d.
* Remove argument in make_struct call corresponding to void field
This is a follow-up of #6543, where we leave the VoidType field as it in
make_struct call during legalization pass.
So during cleaning_void IR pass, when we remove "VoidType" from struct,
we will have to also clean up the argument corresponding to the
"VoidType" field.
|
|
* Enable "-HV 2021" option for DXC
|
|
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
|
