| Age | Commit message (Collapse) | Author |
|---|
|
* Make DeclRefBase a Val, and DeclRef<T> a helper class.
* Fixes.
* Workaround gcc parser issue.
* Revert NodeOperand change.
* Fix.
* Fix clang incomplete class complains.
* Fix code review.
* Small cleanups and improvements.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
* Bottleneck DeclRef creation through ASTBuilder.
* Fix clang error.
* Fix.
* Fix.
* More fix.
* Rebase on top of tree.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
|
|
* #include an absolute path didn't work - because paths were taken to always be relative.
* WIP lowerCamel Dictionary.
* WIP more lowerCamel fixes for Dictionary.
* Add/Remove/Clear
* GetValue/Contains
* Fix tabs in dictionary.
Count -> getCount
* Fix fields with caps.
* Key -> key
Value -> value
Use m_ for members where appropriate.
Use lowerCamel in linked list.
* Some small fixes/improvements to Dictionary.
* Kick CI.
|
|
stdlib. (#2615)
* Allow array parameters in forward diff.
* Use type canonicalization instead of coersion.
* Reimplement array type.
* Fix.
* Update test case.
---------
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
|
|
|
|
* wip: dedup AST type nodes and cache lookup.
* Fix.
* Remove profiling.
* Fixes.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
(#2388)
|
|
* Fix errors when building with the latest Xcode
* Bring back unused variable to better match comments
Co-authored-by: jsmall-nvidia <jsmall@nvidia.com>
|
|
* Add debug symbols for release build.
* Hack to try and capture failing compilation.
* Typo fix for capture hack.
* Specify return type on lambdas.
* Added const.
* Try breakpoint.
* Up count
* Let's capture everything so we can valgrind.
* Disable always writing repros.
* Make Scope non RefCounted.
* Fix issue with not serializing Scope.
* More comments around changes to Scope.
Remove Scope* from serialization.
* Remove code used for testing original issue.
|
|
* "Shader Toy" example and related fixes
This change introduces a new `shader-toy` example program that is primarily designed to show how Slang's features for type-based encapsulation and modularity can be applied to modularity for effects along the lines of those from `shadertoy.com`.
The Example
-----------
The example is being checked in with an example "toy" effect that I hastily put together, so that it would not be encumbered with any IP concerns. I wrote the effect using the shadertoy.com editor, so I can be sure it is valid GLSL. During bringup of the application I used a pre-existing and larger effect for testing, so some of the support code that was added is not being used at present.
The big-picture idea here is to have an exmaple that shows how to modularize things using Slang interfaces and generics, and then to use the Slang compiler API to manage the compilation, composition, specialization, and linking steps. For better or worse this leads to the sequence of API calls involved being much longer than what was in something like the `hello-world` example.
Future Work (Example)
---------------------
There is a lot of room for improvement and expansion here, so this should be viewed as a checkpoint of work in progress rather than something I'm claiming as a finalized demonstration of all we'd like to achieve. Areas for future work include:
* We need to copy the integration of "Dear, IMGUI" that was already done for the `model-viewer` example so that this example can have a UI.
* Now that the compilation flow is broken into all these additional steps, it should be possible to have the application load multiple effects as distinct modules, and then provide a UI for switching between them. The chosen effect module would be used to specialize the top-level shader(s) before kernel generation.
* The checked-in logic includes a compute shader that can execute an effect, but that hasn't been tested nor has it been wired up to any kind of UI. We should have a way to switch between multiple execution methods, with a goal of eventually including CPU execution.
* The "GLSL compatibility" code needs a lot of improvements before it is likely to be usable for a nontrivial number of shaders. Some of that work is waiting on Slang compiler fixes, though.
* We should consider allowing the individual "toy" effects to define their own uniform parameters and expose those via a UI and reflection. The catch in this case is not that this would be difficult to do, but that it would be a semantic change to how shader toy effects currently work.
The Compiler Fixes
------------------
Doing this work exposed a few bugs in Slang, and this change includes fixes for the ones that were quick to address.
We already had logic in `slang-check-shader.cpp` that was validating the entry points in a compile request - either by checking the explicitly-listed entry points, or by scanning for `[shader("...")]` attributes. The problem is that the routine that did that checking was not being invoked on all compiles. The logic that handled entry points was only being run for manual compiles using `SlangCompileRequest`, while anything using `import` or `loadModule` would ignore entry points. I refactored the relevant code into a subroutine that will be invoked in all compilation scenarios.
There were already `TODO` comments in `SpecializedComponentType` which made the point about how a specialized entry point like `myShader<YourType>` would need to properly show that it has dependencies on both the module that defines `myShader` *and* the module that defines `YourType`, while only the former was being handled at present. I went ahead and implemented the logic to scan the generic arguments for a specialized compoment type in order to determine what module(s) the arguments depend on (both type arguments and witness tables). With that change, using `IComponentType::link` on a specialized component will properly pull in the module(s) that the generic arguments come from.
In `slang-ir-legalize-types.cpp` we could run into assertion failures in debug builds because of code trying to legalize layout `IRAttr`s for fields or parameters with types that need legalization. In practice it is safe to skip these layout attributes, because legalization of the fields/parameters they pertain to would result in creation of entirely new layout attributes, and the old ones would then be unreferenced.
Future Work (Fixes)
-------------------
There are other compiler bugs that this work exposed, but which this change does not address. These will need to be resolved as part of subsequent changes:
* Slang allows for default-initialization of variables of a generic type. That is, given `<T : ISomething>` a user is allowed to declare `T x = {};` and the Slang front-end does not complain. Instead, this leads to an internal compiler error during IR lowering.
* The Slang `__init()` feature probably needs to be upgraded to a properly supported feature, and we probably need a way to make implementing default-initialization an easy thing (e.g., any `struct` type that has initial-value expressions for all its fields should automatically and implicitly satsify an `init();` requirement declared in an interface)
* Iniside an `__init()` definition, code has mutable access to members of the enclosing type, but for some reason the front-end is incorrectly treating `this` as immutable in those contexts. As a result you can write to `someField` but not `this.someField`.
* User-defined operator overloads flat out don't work (which isn't surprising given that no clients have decided to use them yet, and we have no test coverage for them). This is actually due to the shadowing rules being used for lookup right now, so a fix for this issue is going to have far-reaching consequences around what overloads are visible where (and anything that impacts overload resolution is a big can of worms, including around performance).
* fixup: test case had missing main function
|
|
* #include an absolute path didn't work - because paths were taken to always be relative.
* WIP for COM CompileRequest.
* Add more methods to IGlobalSession.
* Fix createCompileRequest.
Made slangc tool use COM style methods.
* m_ prefix variables in EndToEndCompileRequest
|
|
* Allow existential types in `StructuredBuffer` element type.
* Handle StructuredBuffer.Load/.Consume methods
* Clean up unnecessary changes
* Code cleanup
* Update test comment
|
|
During semantic checking, the compiler used to link together `ExtensionDecl`s into a singly-linked list dangling off of the `AggTypeDecl` that they applied to. This approach made lookup relatively easy, because given a `DeclRef` to an `AggTypeDecl` one could easily find and walk the list of candidate extensions.
Unfortunately, the simple approach has two major strikes against it:
* First, as we recently ran into, it creates a lifetime/ownership problem, in cases where the `ExtensionDecl` is outlived by the `AggTypeDecl` it applies to. This creates the one and only place in the compiler today where an "old" AST node might point to a "new" AST node, and it resulted in use-after-free problems in client code.
* Second, the scoping of `extension`s ends up being completely wrong. All of the `extension` methods on a type end up being visible in all cases, instead of just in the context of modules where the `extension` itself is visible. The comparable feature in C# (static extension methods) is careful to not make scoping mistakes like this. The Swift langauge has loose scoping for `extension` more akin to what we have in Slang today, but the maintainers seem to consider it a misfeature.
This change attempts to clean up both issues by changing the way that extension declarations are stored. There are two main pieces:
1. The primary "source of truth" for extension lookup has been moved to the `ModuleDecl`, where a module is responsible for storing a cache of the extensions declared within that module (keyed by the declaration of the type being extended). This cache is updated at the same point where the old code would mutate the AST node being depended on.
2. A secondary aggregated cache is added to the `SharedSemanticsContext` used during semantic checking. This cache includes entries from across multiple modules, and is intended to be invalidated and rebuilt on demand if new modules are added during checking.
Access to the candidate extensions has now been put behind subroutines that require a semantics-checking context to be passed in (there was always one available in contexts that care about extensions).
In addition, the operation for looking up members including those from extensions was refactored heavily to involve internal rather than external iteration and, more importantly, was changed so that it actually tests whether the `ExtensionDecl`s it loops over apply to the type in question, rather than blindly letting extensions members be looked up in ways that don't make sense.
There are three test cases added here to confirm aspects of the fix:
* First, I added a test that reproduces the crash that was being seen, so that we have a regression test for the fix.
* Second, I added a basic semantic-checking test to confirm that an `extension` from an `import`ed module is still visible/usable, to confirm that I didn't break existing valid uses of extensions.
* Third, I added a diagnostic test that ensures that we correctly ignore extensions that should not be visible in a given context as a result of `import` declarations.
Co-authored-by: jsmall-nvidia <jsmall@nvidia.com>
|
|
* * Fix output in slang repro command line
* Profile uses lowerCamel method names (had mix of upper and lower)
* Rename slang-serialize-state/SerializeStateUtil to slang-repro and ReproUtil.
|
|
|
|
|
|
* Add a ASTBuilder to a Module
Only construct on valid ASTBuilder (was being called on nullptr on occassion)
* Add nodes to ASTBuilder.
* Compiles with RefPtr removed from AST node types.
* Initialize all AST node pointer variables in headers to nullptr;
* Initialize AST node variables as nullptr.
Make ASTBuilder keep a ref on node types.
Make SyntaxParseCallback returns a NodeBase
* Don't release canonicalType on dtor (managed by ASTBuilder).
* Give ASTBuilders a name and id, to help in debugging.
For now destroy the session TypeCache, to stop it holding things released when the compile request destroys ASTBuilders.
* Moved the TypeCheckingCache over to Linkage from Session.
* NodeBase no longer derived from RefObject.
* Only add/dtor nodes that need destruction.
First pass compile on linux.
|
|
|
|
* Small improvements to documentation and code around DiagnosticSink
* Made methods/functions in slang-syntax.h be lowerCamel
Removed some commented out source (was placed elsewhere in code)
* Making AST related methods and function lowerCamel.
Made IsLeftValue -> isLeftValue.
|
|
|
|
* Compiles.
* Small tidy up around session/ASTBuilder.
* Tests are now passing.
* Fix Visual Studio project.
* Fix using new X to use builder when protectedness of Ctor is not enough.
Substitute->substitute
* Add some missing ast nodes created outside of ASTBuilder.
* Compile time check that ASTBuilder is making an AST type.
* Moced findClasInfo and findSyntaxClass (essentially the same thing) to SharedASTBuilder from Session.
|
|
* Fields from upper to lower case in slang-ast-decl.h
* Lower camel field names in slang-ast-stmt.h
* Fix fields in slang-ast-expr.h
* slang-ast-type.h make fields lowerCamel.
* slang-ast-base.h members functions lowerCamel.
* Method names in slang-ast-type.h to lowerCamel.
* GetCanonicalType -> getCanonicalType
* Substitute -> substitute
* Equals -> equals
ToString -> toString
* ParentDecl -> parentDecl
Members -> members
|
|
* Token size on 64 bits is 24 bytes (from 40). On 32 bits is 16 bytes from 24.
* Added hasContent method to Token.
Some other small improvements around Token.
|
|
* * Added MemberFilterStyle - controls action of FilteredMemberList and FilteredMemberRefList
* Splt out template implementations
* Use more standard method names dofr FilteredMemberRefList
* Added reflect-static.slang test
* Added isNotEmpty/isEmpty to filtered lists
* Added ability to index into filtered list (so not require building of array)
* Default MemberFilterStyle to All.
* Remove explicit MemberFilterStyle::All
|
|
This change adds logic for parsing `namespace` declarations, referencing them, and looking up their members.
* The parser changes are a bit subtle, because that is where we deal with the issue of "re-opening" a namespace. We kludge things a bit by re-using an existing `NamespaceDecl` in the same parent if one is available, and thereby ensure that all the members in the same namespace can see on another.
* In order to allow namespaces to be referenced by name they need to have a type so that a `DeclRefExpr` to them can be formed. For this purpose we introduce `NamespaceType` which is the (singleton) type of a reference to a given namespace.
* The new `NamespaceType` case is detected in the `MemberExpr` checking logic and routed to the same logic that `StaticMemberExpr` uses, and the static lookup logic was extended with support for looking up in a namespace (a thin wrapper around one of the existing worker routines in `slang-lookup.cpp`.
* I made `NamespaceDecl` have a shared base class with `ModuleDecl` in the hopes that this would allow us to allow references to modules by name in the future. That hasn't been tested as part of this change.
* I cleaned up a bunch of logic around `ModuleDecl` holding a `Scope` pointer that was being used for some of the more ad hoc lookup routines in the public API. Those have been switched over to something that is a bit more sensible given the language rules and that doesn't rely on keeping state sititng around on the `ModuleDecl`.
* I added a test case to make sure the new funcitonality works, which includes re-opening a namespace, and it also tests both `.` and `::` operations for lookup in a namespace.
* The main missing feature here is the ability to do something like C++ `using`. It would probably be cleanest if we used `import` for this, since we already have that syntax (and having both `import` and `using` seems like a recipe for confusion). Most of the infrastructure is present to support `import`ing one namespace into another (in a way that wouldn't automatically pollute the namespace for clients), but some careful thought needs to be put into how import of namespaces vs. modules should work.
|
|
* Some Slang API additions
These are additions to the public Slang API that came up while I was trying to write an example to demonstrate GPU printing. The additions aren't strictly necessary for the example, but I found these to be missing services when writing the code the way I wanted to.
The main public changes are:
* There is a new distinct `IEntryPoint` interface which inherit from `IComponentType` (much like `IModule` before). For now this doesn't expose any new functions on top of `IComponentType`, but I expect it to do so eventually.
* It is now possible to get the `IModule` for a specific translation unit in a compile request with `spCompileRequest_getModule`. Even for a compile request that had only one translation unit this is *not* the same object as gets returned by `spCompileRequest_getProgram`. The latter should probably be called `*_getLinkedProgram` because it returns a composite component type that links the module with everything it `import`s.
* An `IModule` can look up entry points declared in the module. Eventually a module should support looking up most of the declarations in the module (e.g., types) by name, but entry points are an obvious case.
* A new `link()` operation is added to `IComponentType`. It is possible to have component types that have unsatisfied dependencies, such that trying to generate kernel code from them will fail. The `link()` operation tries to produce a new composite component type that combines a component with its dependencies, to enable code generation. The implementation of end-to-end compilation was using a function like this internally, but it hadn't been exposed to the API.
Notes on the implementation:
* The list of entry points declared in a given translation unit has moved from `TranslationUnitRequest` to the `Module` inside of it.
* `EntryPoint` now has to do a song and dance much like `Module` to both inherit from `ComponentType` and support the `IEntryPoint` interface.
* The `Session* m_session` member in `Linkage` (in terms of public API, this is the `slang::ISession` holding a pointer to the `slang::IGlobalSession`) has been changed to a `RefPtr`. Without this change an application can't just hold onto a `ComPtr<slang::ISession>`; they also need to retain the `IGlobalSession` or things will crash. The new behavior seems more correct, but I worry that it might introduce a leak.
* The `asInternal` operation for `IComponentType` had to be updated to not just perform a cast. A type like `Module` has two `IComponentType` sub-objects, and only one of these is at the same address as the `ComponentType` base.
* Similarly, the `Module::getInterface` logic was changed to fall back to `Super::getInterface` for all the cases other than `IID_IModule`, so that it would be guaranteed to return the `IComponentType` at the same address as `ComponentType` in response to a `queryInterface`.
* Fixes for memory retain cycles
As part of the earlier change, I made the `Linkage` type hold a `RefPtr` to the `Session`.
The motivation there is it lets a user hang onto just a `slang::ISession` without having to also retain the `slang::IGlobalSession` for no immediately apparent reason.
There are two problems that this surfaced, one pre-existing and one new.
The new problem was that `Session` already held a `RefPtr<Linkage> m_builtinLinkage` for the linkage that holds the stdlib code.
I solved that problem by splitting the parent pointer in `Linkage` into two pointers: a raw pointer that is used to actually locate the parent session, and a ref-counted one that can be used to *optionally* retain the parent session.
The builtin linkage is then set up to explicitly not retain its parent, thus breaking the cycle.
The second problem was a pre-existing one, where every `ComponentType` was holding a retained pointer to its parent `Linkage`, but in turn the `Linkage` was holding retained pointers to many `ComponentTypes` (and subclasses thereof).
For this case I used the more expedient fix of making the parent pointer into a raw pointer, and figuring that it is a reasonable rule to expect user to retain the `Linkage` (aka `slang::ISession`) that owns a component type if they want to be able to use the component type.
I might need/want to investigate a better fix for the latter issue, but for now this seems to clean up the issues I was seeing in the tests. Fingers crossed.
|
|
* Remove legacy feature for merging global shader parameters
There is a fair amount of special-case code in the Slang compiler today to deal with the scenario where a programmer declares the "same" shader parameter across two different translation units:
```hlsl
// a.hlsl
Texture2D a;
cbuffer C { float4 c; }
```
```hlsl
// b.hlsl
cbuffer C { float4 c; }
Texture2D b;
```
An important note here is that the declaration of `C` may be in a header file that both `a.hlsl` and `b.hlsl` `#include`, because from the standpoint of the parser and later stages of the compiler, there is no difference between `C` being in an included file vs. it being copy-pasted across both `a.hlsl` and `b.hlsl`.
When a user invokes `slangc a.hlsl b.hlsl` (or the equivalent via the API), then they may decide that it is "obvious" that the shader parameter `C` is the "same" in both `a.hlsl` and `b.hlsl`.
Knowing that the parameter is the "same" may lead them to make certain assumptions:
* They may assume that generated code for entry points in `a.hlsl` and `b.hlsl` will both agree on the exact `register`/`binding` occupied by `C`.
* They may assume that reflection information for their program will only reflect `C` once, and it will reflect it in a way that is applicable to entry points in both `a.hlsl` and `b.hlsl`
* They may assume that the compiler can and should handle this use case even when `C` contains fields with `struct` types that are declared in both `a.hlsl` and `b.hlsl` that have the "same" definition.
* They may assume that in cases where `C` is declared inconsistently between `a.hlsl` and `b.hlsl` the compiler can and will diagnose an error.
Making these assumptions work in practice required a lot of special-case code:
* When composing/linking programs was `ComponentType`s we had to include a special case `LegacyProgram` type that could provide these "do what I mean" semantics, since they are *not* what one would want in the general case for a `CompositeComponentType`.
* During enumeration of global shader parameter in a `LegacyProgram`, we had to detect parameters from distinct modules (translation units) with the same name, and then enforce that they must have the "same" type (via an ad hoc recursive structural type match). No other semantic checking logic needs or uses that kind of structural check.
* During parameter binding generation, we need to handle the case where a single global shader parameter might have multiple declarations, and make sure to collect explicit bindings from all of them (checking for inconsistency) and also to apply generated bindings to all of them.
* The `mapVarToLayout` member in `StructTypeLayout` is a concession to the fact that we might have multiple `VarDecl`s for each field of the struct that represents the global scope, we might need to look up a field and its layout using any of those declarations (much of the need for this field had gone away now that IR passes are largely using IR-based layout).
All of these different special cases added more complex code in many places in the compiler, all to support a scenario that isn't especially common.
Most users won't be affected by the original issue, because they will do one of several things that rule it out:
* Anybody using `slangc` like a stand-in for `fxc` or `dxc` and compiling one translation unit at a time will not suffer from any problems. If/when such users want consistent bindings across translation units, they already use either explicit binding or rely on consistent ordering and implicit binding.
* Anybody who puts all the entry points that get combined into a pass/pipeline in a single file will not have problems. They will automatically get consistent bindings because of Slang's guarantees, and there can't be duplicated declarations when there is only one translation unit.
* Anybody using `import` to factor out common declarations while compiling multiple translation units at once will not be affected. Parameters declared in an `import`ed module are the "same" in a much deeper way that it is trivial for Slang to support.
Only users of the Falcor framework are likely to be affected by this, and they have two easy migration paths: either put related entry points into the same file, or factor common parameters into an `import`ed module.
(It is also worth noting that for command-line `slangc`, it is possible to have a single module with multiple `.slang` files in it, which can all see global declarations like parameters across all the files. Anybody who buys into doing things the Slang Way should have no problem avoiding duplicated declarations)
With the rationale out of the way, the actual change mostly just amounts to deleting lots of code that is no longer needed. An astute reviewer might notice several `assert`-fail conditions where complex Slang features were never actually made to work correctly with this legacy behavior.
A small number of test cases broke with the code changes, but these were tests that specifically exercised the behavior being removed. In the case of the tests around binding/reflection generating, I rewrote the tests to use one of the idomatic workarounds (putting the shared parameters into an `import`ed module), but doing so required me to add support for `#include` when doing pass-through compilation with `fxc`. That logic added a bit more cruft than I had originally hoped to this commit, but having `#include` support when doing pass-through compilation is probably a net win.
* fixup: 64-bit warning
|
|
* Initial work for "global generic value parameters"
The main new feature here is support for the `__generic_value_param` keyword, which introduces a *global generic value parameter*.
For example:
__generic_value_param kOffset : uint = 0;
This declaration introduces a global generic value parameter `kOffset` of type `uint` that has a nominal default value of zero.
The broad strokes of how this feature was added are as follows:
* A new `GlobalGenericValueParamDecl` AST node type is introduces in `slang-decl-defs.h`
* A new `parseGlobalGenericValueParamDecl` subroutine is added to `slang-parser.cpp`, and is added to the list of declaration cases as the callback for the `__generic_value_param` name.
* Cases for `GlobalGenericValueParamDecl` are added to the declaration checking passes in `slang-check-decl.cpp`, mirroring what is done for other variable declaration cases.
* A case for `GlobalGenericValueParamDecl` is aded to the `Module::_collectShaderParams` function, so that it is recognized as a kind of specialization parameter. This introduces a specialization parameter of flavor `SpecializationParam::Flavor::GenericValue` (which was already defined before this change, although it was unused).
* A case for `SpecializationParam::Flavor::GenericValue` is added in `Module::_validateSpecializationArgsImpl` to check that a specialization argument represents a compile-time-constant value (not a type).
* A case for `GlobalGenericValueParmDecl` is introduced in `slang-lower-to-ir.cpp` that introduces a global generic parameter in the IR
* The `IRBuilder` is extended to support creating `IRGlobalGenericParam`s for the distinct cases of type, witness-table, and value parameters. The same IR instruction type/opcode is used for all cases, and only the type of the IR instruction differs.
* The existing mechanisms for lowering specialization arguments to the IR, and doing specialization on the IR itself Just Work with global generic value parameters since they already support value parameters on explicit generic declarations.
That's the santized version of things, but there were also a bunch of cleanups and tweaks required along the way:
* The `SpecializationParam` type was extended to also track a `SourceLoc` to help in diagnostic messages, which meant some churn in the code that collects specialization parameters.
* The `_extractSpecializationArgs` function is tweaked to support any kind of "term" as a specialization argument (either a type or a value).
* To allow *parsing* specialization arguments that can't possibly be types (e.g., integer literals) we replace the existing `parseTypeString` routine with `parseTermString` and then in `parseTermFromSourceFile` call through to a general case of expression parsing (which can also parse types) rather than only parsing types directly.
* Right before doing back-end code generation, we check if the program we are going to emit has remaining (unspecialized) parameters, in which case we emit a diagnostic message for the parameters that haven't been specialized rather than go on to emit code that will fail to compile downstream.
* Within the `render-test` tool we collapse down the arrays that held both "generic" and "existential" specialization arguments, so that we just have *global* and *entry-point* specialization argument lists. This mirrors how Slang has worked internally for a while, but the difference hasn't been important to the test tool because no tests currently mix generic and existential specialization. The logic for parsing `TEST_INPUT` lines has been streamlined down to just the global and entry-point cases, but the pre-existing keywords are still allowed so that I don't have to tweak any test cases.
There are several significant caveats for this feature, which mean that it isn't really ready for users to hammer on just yet:
* There is no support for `Val`s of anything but integers, so there is no way to meaningfully have a generic value param with a type other than `int` or `uint`.
* We allow for a default-value expression on global generic parameters, but do not actually make use of that value for anything (e.g., to allow a programmer to omit specialization arguments), nor check that it meets the constraints of being compile-time constant.
* Global generic value parameters are *not* currently being treated the same as explicit generic parameters in terms of how they can be used for things like array sizes or other things that require constants. This will probably be relaxed at some point, but allowing a global generic to be used to size an array creates questions around layout.
* The IR optimization passes in Slang currently won't eliminate entire blocks of code based on constant values, so using a global generic value parameter to enable/disable features will *not* currently lead to us outputting drastically different HLSL or GLSL. That said, we expect most downstream compilers to be able to handle an `if(0)` well.
* Fix regression for tagged union types
The change that made specialization arguments be parsed as "terms" first, and then coerced to types meant that any special-case logic that is specific to the parsing of types would be bypassed and thus not apply.
Most of that special-case logic isn't wanted for specialization arguments, since it pertains to cases were we want to, e.g, declare a `struct` type while also declaring a variable of that type.
The one special case that *is* useful is the `__TaggedUnion(...)` syntax, which is the only way to introduce a tagged union type right now.
In order to get that case working again, all I had to do was register the existing logic for parsing `__TaggedUnion` as an expression keyword with the right callback, and the existing logic in expression parsing kicks in (that logic was already handling expression keywords like `this` and `true`).
I left in the existing logic for handling `__TaggedUnion` directly where types get parsed, rather than try to unify things.
A better long-term fix is to make the base case for type parsing route into `parseAtomicExpr` so that the two paths share the core logic.
That change should probably come as its own refactoring/cleanup, because it creates the potential for some subtle breakage.
* fixup: typo
|
|
* Split apart `SemanticsVisitor`
The existing `SemanticsVisitor` type was the visitor for expressions, statements, and declarations, and its monolithic nature made it hard to introduce distinct visitors for different phases of checking (despite the fact that we had, de facto, multiple phases of declaration checking).
This change splits up `SemanticsVisitor` as follows:
* There is nosw a `SharedSemanticsContext` type which holds the shared state that all semantics visiting logic needs. This includes state that gets mutated during the course of semantic checking.
* The `SemanticsVisitor` type is now a base class that holds a pointer to a `SharedSemanticsContext`. Most of the non-visitor functions are still defined here, just to keep the code as simple as possible. The `SemanticsVisitor` type is no longer a "visitor" in any meaningful way, but retaining the old name minimizes the diffs to client code.
* There are distinct `Semantics{Expr|Stmt|Decl}Visitor` types that have the actual `visit*` methods for an appropriate subset of the AST hierarchy. These all inherit from `SemanticsVisitor` primarily so that they can have easy access to all the helper methods it defines (which used to be accessible because these were all the same object).
Any client code that was constructing a `SemanticsVisitor` now needs to construct a `SharedSemanticsContext` and then use that to initialize a `SemanticsVisitor`. Similarly, any code that was using `dispatch()` to invoke the visitor on an AST node needs to construct the appropriate sub-class and then invoke `dispatch()` on it instead.
This is a pure refactoring change, so no effort has been made to move state or logic onto the visitor sub-types even when it is logical. Similarly, no attempt has been made to hoist any code out of the common headers to avoid duplication between `.h` and `.cpp` files. Those cleanups will follow.
The one cleanup I allowed myself while doing this was getting rid of the `typeResult` member in `SemanticsVisitor` that appears to be a do-nothing field that got written to in a few places (for unclear reasons) but never read.
* Remove some statefulness around statement checking
Some of the state from the old `SemanticsVisitor` was used in a mutable way during semantic checking:
* The `function` field would be set and the restored when checking the body of a function so that things like `return` statements could find the outer function.
* The `outerStmts` list was used like a stack to track lexically surrounding statements to resolve things like `break` and `continue` targets.
Both of these meant that semantic checking code was doing fine-grained mutations on the shared semantic checking state even though the statefullness wasn't needed.
This change moves the relevant state down to `SemanticsStmtVisitor`, which is a type we create on-the-fly to check each statement, so that we now only need to establish the state once at creation time.
The list of outer statements is handled as a linked list threaded up through the stack (a recurring idiom through the codebase).
There was one place where the `function` field was being used that wasn't strictly inside statement checking: it appears that we were using it to detect whether a variable declaration represents a local, so I added an `_isLocalVar` function to serve the same basic purpose.
With this change, the only stateful part of `SharedSemanticsContext` is the information to track imported modules, which seems like a necessary thing (since deduplication requires statefullness).
* Refactor declaration checking to avoid recursion
The flexiblity of the Slang language makes enforcing ordering on semantic checking difficult. In particular, generics (including some of the built-in standard library types) can take value arguments, so that type expressions can include value expressions. This means that being able to determine the type of a function parameter may require checking expressions, which may in turn require resolving calls to an overloaded function, which in turn requires knowing the types of the parameters of candidate callees.
Up to this point there have been two dueling approaches to handling the ordering problem in the semantic checking logic:
1. There was the `EnsureDecl` operation, supported by the `DeclCheckState` type. Every declaration would track "how checked" it is, and `EnsureDecl(d, s)` would try to perform whatever checks are needed to bring declaration `d` up to state `s`.
2. There was top-down orchestration logic in `visitModuleDecl()` that tried to perform checking of declarations in a set of fixed phases that ensure things like all function declarations being checked before any function bodies.
Each of these options had problems:
1. The `EnsureDecl()` approach wasn't implemented completely or consistently. It only understood two basic levels of checking: the "header" of a declaration was checked, and then the "body," and it relied on a single `visit*()` routine to try and handle both cases. Things ended up being checked twice, or in a circular fashion.
2. Rather than fix the problems with `EnsureDecl()` we layered on the top-down orchestration logic, but doing so ignores the fact that no fixed set of phases can work for our language. The orchestration logic was also done in a relatively ad hoc fashion that relied on using a single visitor to implement all phases of checking, but it added a second metric of "checked-ness" that worked alongside `DeclCheckState`.
This change strives to unify the two worlds and make them consistent. One of the key changes is that instead of doing everything through a single visitor type, we now have distinct visitors for distinct phases of semantic checking, and those phases are one-to-one aligned with the values of the `DeclCheckState` type.
More detailed notes:
* Existing sites that used to call `checkDecl` to directly invoke semantic checking recursively now use `ensureDecl` instead. This makes sure that `ensureDecl` is the one bottleneck that everything passes through, so that it can guarantee that each phase of checking gets applied to each declaration at most once.
* The existing `visitModuleDecl` was revamped into a `checkModule` routine that does the global orchestration, but now it is just a driver routine that makes sure `ensureDecl` gets called on everything in an order that represents an idealized "default schedule" for checking, while not ruling out cases where `ensureDecl()` will change the ordering to handle cases where the global order is insufficient.
* Because `checkModule` handles much of the recursion over the declaration hierarchy, many cases where a declaration `visit*()` would recurse on its members have been eliminated. The only case where a declaration should recursively `ensureDecl()` its members is when its validity for a certain phase depends on those members being checked (e.g., determining the type of a function declaration depends on its parameters having been checked).
* All cases where a `visit*()` routine was manually checking the state/phase of checking have been eliminated. It is now the responsibility of `ensureDecl` to make sure that checking logic doesn't get invoked twice or in an inappropriate order.
* Most cases where a `visit*()` routine was manually *setting* the `DeclCheckState` of a declaration have been eliminated. The common case is now handled by `ensureDecl()` directly, and `visit*()` methods only need to override that logic when special cases arise. E.g., when a variable is declared without a type `(e.g., `let foo = ...;`) then we need to check its initial-value expression to determine its type, so that we must check it further than was initially expected/required.
* This change goes to some lengths to try and keep semantic checking logic at the same location in the `slang-check-decl.cpp` file, so each of the per-phase visitor types is forward declared at the top of the file, and then the actual `visit*()` routines are interleaved throughout the rest of the file. A future change could do pure code movement (no semantic changes) to arrive at a more logical organization, but for now I tried to stick with what would minimize the diffs (although the resulting diffs can still be messy at times).
* One important change to the semantic checking logic was that the test for use of a local variable ahead of its declaration (or as part of its own initial-value expression) was moved around, since its old location in the middle of the `ensureDecl` logic made the overall flow and intention of that function less clear. There is still a need to fix this check to be more robust in the future.
* Add some design documentation on semantic checking
The main thing this tries to lay out is the strategy for declaration checking and the rules/constraints on programmers that follow from it.
* fixup: typos found during review
|
|
* Add basic support for entry points in `.slang-lib` files.
The basic idea here is that when writing out a `.slang-lib` file based on a compile request, we include new sections in the generated RIFF that represent the entry points that were requested. The entry-point information is serialized in an entirely ad hoc fashion (a future change might clean it up to use the `OffsetContainer` machinery), and contains the name, profile, and mangled symbol name of an entry point.
When deserializing this information, we create a list of "extra" entry points that gets attached to the front-end compile requests. These "extra" entry points get turned into `EntryPoint` objects at the same place in the code that entry points specified on the command line or via API would be checked, but the extra entry points bypass the semantic checking and just create "dummy" `EntryPoint` objects.
Aside: the ability for a compile request to end up with entry points that weren't originally specified via API or command-line is not new. We already had support for compiling a translation unit with entry points entirely specified via `[shader(...)]` attributes, and this new support tries to function similarly.
Because the "dummy" entry points don't retain AST-level information, several parts of the code have been modified to defensively check for `EntryPoint` objects without a matching AST declaration, and skip over them.
The main place where this creates a problem is paramete binding, where ignoring the dummy entry point is appropriate since we currently assume linked-in library code has been laid out manually.
One small cleanup here is that the `-r` command-line flag and the `spAddLibraryReference` API functio now bottleneck through a common routine to do their work, so that they both gain the new behavior without needing copy-paste programming.
In order to keep the existing test case for library linking with entry points working, I had to add a flag to the `render-test` tool so that it can skip specifying entry point names as part of the compile request it creates. In that case it must instead assume that the entry points will be added to the compile request via other means. This logic is a bit magical, and hints that we should be looking for other ways to expose the library linking functionality over time.
* fixup: remove alignment assertion
|
|
The semantic checking logic was all inside `slang-check.cpp` and as a result this was a monster file that was extremely hard to follow. This change splits `slang-check.cpp` into several smaller files, although some of the resulting files are still quite large.
This change attempts to be a copy-paste job as much as possible and does *not* perform any cleanup on naming, structure, duplication, etc. in the code it deal with. No function bodies or signatures have been touched.
|
