| Age | Commit message (Collapse) | Author |
|---|
|
* Changed all getEntryPointCode calls to use RendererBase::getEntryPointCodeFromShaderCache
* Hashing hooked up, tests pass but need to add more to fully test functionality
* checkpoint
* Checkpoint: File system creation seems functional, saving is broken
* checkpoint: Fixed filename generation from MD5 hash, shader blob might be going missing ahead of pipeline state creation
* Fixed a lot of bugs related to hash code generation, shader cache is likely working but needs further testing
* Added workaround for module loading by re-creating the test device, shader cache test functional
* Vulkan shader caching bug fixed, checkpoint commit before more refinement
* pre-ToT merge checkpoint
* checkpoint commit, improving cache keys
* Significantly expanded items included in the dependency hash for Module; Added dependency hash functions to SpecializedComponentType and RenamedEntryPointComponentType
* Temporarily disable shader cache test
* Mid cleanup changes, solution successfully builds
* Added several helper update functions to slang-md5 to help simplify usage; Added a function under ISession to compute a hash for all linkage-related items; Function renames and cleaned up some comments
* Ran premake.bat; Renamed getASTBasedHashCode to computeASTBasedHash
* Added slang unit tests for Checksum and MD5; Extended gfx shader cache test to test with multiple shader files and one shader file with multiple entry points
* Solution builds and shader cache tests pass, but at least a couple other tests now failing
* ran premake.bat
* More cleanup changes
* Added shaderCachePath field to IDevice desc in gfx.slang, gfx-smoke.slang should be functional
* ran premake
* cleanup changes; Adding test printf to getEntryPointCodeFromShaderCache to see if output can be seen in CI
* Removed debugging printfs; Added handling for getEntryPointCode() failing
* Cleanup changes; Jonathan's fixes to SerialWriter to zero initialize otherwise uninitialized memory; Change to SwizzleExpr creation to zero initialize elementCount
* Changed enable_if_t to enable_if
* Fixed enable_if
* Added test for import vs include and changes to included and imported files; Fixed build errors in CUDA; Renamed shader cache statistics fields
* cleanup changes
* Readd removed file
* Restructured computeDependencyBasedHash calls, added computeDependencyBasedHashImpl to all classes dervied from ComponentType
* Applied same restructuring to the AST hash functions
* Cleanup changes; Moved HashBuilder out to slang-digest.h and added some helper functions to streamline the process of adding items to a hash
* Cleanup; Fixed incorrect expected results for shader import and include test
|
|
* Language feature: pointer sized int types.
* Fix.
* small change to test.
* Fix stdlib.
* Fix.
* Fix.
* Add typedef for `size_t` in stdlib.
* Fix test.
* Add `intptr_t::size` constant.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
* Allow interface requirements to reference to the interface type itself.
* add comment explaining the change.
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>
|
|
* Multi parameter `__subscript`
* Fix.
* Fix bugs.
* Fix.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
(#2388)
|
|
|
|
* Compiler time evaluation of all int and bool operators.
* Fix linux compile error.
* Fix.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
* Warning on bool to float conversion.
* Fix test cases.
* Improve.
* LanguageServer: don't show constant value for non constant variables.
* Fix tests.
* Fix warnings in tests.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
|
|
* Add `none` literal that is convertible to `Optional`.
* Fix cpu code gen.
* Include vk and cpu test for is-as operator test.
* Inline comparison operators.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
* `is` and `as` operator and `Optional<T>`.
* Fix.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
* Merge slang-ir-diff-jvp.cpp
* Added support and tests for other float vector types
* Added swizzle test and code to handle it (tests failing currently)
* Fixed one test, the other is still pending
* Fixed instruction cloning logic to avoid modifying original function
* Fixed an issue with custom 'pow_jvp' and added support for vector contructor
* Minor update to comments
* Fixed support for division
* Fixed an issue with uninitialized diagnostic sink
* Moved derivative processing to after mandatory inlining.
Skip instructions that don't have side-effects and aren't used by anything.
* WIP: Handling unconditional control flow and multi-block functions
* Support for unconditional multi-block functions
* Added a dead code elimination step to the derivative pass
* Changed name of 'hasNoSideEffects()'
* Refactored variable names
* Added initial IR defs for new type system
* Added necessary logic for semantic checking
* Overhauled type system to use builtin pair types and conform to the IDifferentiable interface
* Automatically replace IRDifferentiablePairType to a custom IRStructType
* Added generics handling by expanding the conformance context functionality and allowing for type parameters
* Minor fix: early return in processPairTypes()
* Minor fixes to differentiable resolution on generic types
* Added new instructions for differential pairs. Basic tests work now.
Looking into generic types.
* Adjusted most tests to the new type system. OutType and InOutType are still not properly working.
* Updated __jvp to produce both primal and differential output
* Moved autodiff related declarations to diff.meta.slang
* Refactored variable names
* Added initial IR defs for new type system
* Added necessary logic for semantic checking
* Overhauled type system to use builtin pair types and conform to the IDifferentiable interface
* Automatically replace IRDifferentiablePairType to a custom IRStructType
* Added generics handling by expanding the conformance context functionality and allowing for type parameters
* Minor fix: early return in processPairTypes()
* Minor fixes to differentiable resolution on generic types
* Added new instructions for differential pairs. Basic tests work now.
Looking into generic types.
* Adjusted most tests to the new type system. OutType and InOutType are still not properly working.
* Updated __jvp to produce both primal and differential output
* Moved autodiff related declarations to diff.meta.slang
* Removed external changes
* Cleanup the transcription logic: each case returns a pair of insts for the primal and differential computation.
|
|
* Implicit pointer dereference when using member operator.
* Add expected test result
* Fix lookup.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
|
|
* Merge slang-ir-diff-jvp.cpp
* Added support and tests for other float vector types
* Added swizzle test and code to handle it (tests failing currently)
* Fixed one test, the other is still pending
* Fixed instruction cloning logic to avoid modifying original function
* Fixed an issue with custom 'pow_jvp' and added support for vector contructor
* Minor update to comments
* Fixed support for division
* Fixed an issue with uninitialized diagnostic sink
* Moved derivative processing to after mandatory inlining.
Skip instructions that don't have side-effects and aren't used by anything.
* WIP: Handling unconditional control flow and multi-block functions
* Support for unconditional multi-block functions
* Added a dead code elimination step to the derivative pass
* Changed name of 'hasNoSideEffects()'
|
|
* Added out/inout tests
* Added support for out and inout parameters. Still untested
* Fixed and tested support for out and inout types
* Removed some comments
|
|
* Support `class` types.
* Ignore class-keyword test
* Fix codereview comments and warnings.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
arithmetic on the float3 type (#2313)
* Added support for differentiating calls to basic functions, as well as arithmetic on the float3 type
* Added test expected result
|
|
* Language server: Inlay hints.
* Signature help for base exprs that is not a declref.
* Fix checking of jvp operator.
* Fix.
* Add clang-format based auto formatting.
* Fix clang error.
* Fix clang-format discovery logic.
* Fine tune auto formatting and completion experience.
* Update macos workflow.
* Fixes to configurations.
* Fix parser recovery to trigger completion for index exprs.
* Typo fix.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
arithmetic. Also added type-checking during the semantic stage. (#2303)
* Added JVPTranscriber to handle differentiation of load, store, var, param and return instructions, as well as conversion of data and function types
* Changed class names to be more in line with convention. Added correct type checking for __jvp() and verified that simple calls with only loads and stores are processed correctly
* Added logic to differentiate basic arithmetic and literals inside IRConstruct and fixed the way parameters are differentiated
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
framework for passes to process them. (#2297)
* Added a decorator to mark functions for forward-mode differentiation
* Fill out support for calls to non-decl values
The existing compiler logic has a few places (semantic checking plus AST-to-IR lowering) where it assumes that function calls (`InvokeExpr`) are only ever made to expressions that resolve to a specific `Decl` (`DeclRefExpr`). This assumption allows semantic checking and lowering code to inspect things like the parameter list of an actual declaration, rather than just the type signature of the callee, and that infrastructure is used to support various features (e.g., default argument values on parameters).
The AST and IR representations themselves have no matching requirement, and the places where the more general case of call expressions would need to be supported were relatively clear in the code. This change attempts to add suitable logic into each of those places.
Note that this change does *not* surface any valid way to form input code that would cause these new code paths to be executed, so it is entirely possible that there are bugs in the logic as written here. The primary goal of this change is simply to get a sketch of the correct code checked in so that we have something to build on once we have language features that will require this support.
* fixup: warnings-as-errors
* Added parser logic for '__jvp(<fn-name>)' operator
* Fixed issue with missing overload candidate item and added basic parsing test for the __jvp syntax
* Added a blank JVP Auto-diff pass and a pass that replaces 'JVPDerivativeOf' calls with the differentiated function
* Added a couple comments
* Added parameter handling for the JVP pass
Co-authored-by: Theresa Foley <tfoley@nvidia.com>
|
|
|
|
* Major language server features.
* Include slangd in binary release.
* Fix compiler issues.
* Fix compiler error.
* Completion resolve.
* Various improvements.
* Update diagnostic test expected output.
* Bug fix for source locations.
* Adjust diagnostic update frequency.
* Update github actions to store artifacts.
* Fix infinite parser loop.
* Fix parser recovery.
* Fix parser recovery.
* Update test.
* Fix test.
* Disable IR gen for language server.
* Allow commit characters in auto completion.
* Fix lookup for invoke exprs.
* More parser robustness fixes.
* update solution file
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
* Clean up `IRReturnVoid`.
* Update gitignore.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
* New language feature: basic error handling.
* Fix.
* Fix `tryCall` encoding according to code review.
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
The problematic case is when an `interface` has a `[mutating]` method:
interface ICounter
{
[mutating] void increment();
}
and code tries to invoke that method on a value of existential type:
ICounter c = ...;
c.increment();
We know that the existential value `c` is conceptually a tuple of:
* A concrete type `X`
* A witness that `X : ICounter`
* A value `v` of type `X`
We simply want to invoke `increment()` on the `v` part, using the `X : ICounter` witness table.
The catch that the compiler faces is that the variable `c` is mutable, so we need to be careful that we "snapshot" its value (the tuple `X, X:ICounter, v`) at a single point.
The snapshotting behavior is important when invoking a method that involves `This` or associated types in its signature, so we cannot get rid of it.
The snapshotting we do relies on the idea of a `LetExpr` AST node, which cannot be written in the input syntax.
A `LetExpr` introduces a variable binding (with an initial-value expression) and then evaluates a body expression in the context of that binding.
For a call site like `c.increment()` the front-end makes an intermediate copy of `c` and then "opens" that immutable value to get at the elements of the tuple `X`, `X : ICounter`, `v`.
The resulting AST after checking looks something like:
ICounter c = ...;
(let tmp = c in extractExistentialValue(tmp)).increment();
In that form it is more clear why the attempt to call `increment()` fails:
1. The binding `tmp` sure looks immutable
2. There is no logic in the compiler to make `extractExistentialValue(x)` be an l-value if `x` is
3. There is seemingly no logic to write back from `tmp` to `c` when the operation completes
Let us walk through those problems in order.
Item (1) turns out to be a bit of a non-issue.
Despite the way that I've written out `let` expressions above, the logic in `moveTemp()` in the compiler actually introduces a *mutable* binding.
Item (2) can be fixed for the purposes of semantic checking by modifying `openExistential()`.
Simplistically, we make the overall expression be an l-value if the operand is.
Item (3) is handled at the level of AST->IR lowering. Each kind of expression that can form an l-value needs to have a way to represent the "location" of that l-value in the `LoweredValInfo` type.
This change adds a case to handle the `extractExistentialVal` operation, by tracking both the extract value (of concrete type) and the underlying l-value (of existential type).
Where all of this comes crashing against reality a bit is that the scoping I've drawn for the `let` expressions above kind of doesn't work once we look at types.
The basic problem is that the *type* of the `(let tmp = c in ...)` expression is the concrete type `X` that was extracted from the existential.
That type can conceptually be written as `ExtractExistentialType(tmp)` which, notably, references `tmp`.
That means that we end up with AST expression nodes that reference the variable `tmp` *outside* of its scope.
Furthermore, those references to `tmp` can end up being lowered to IR *before* we have lowered the `let ...` expression itself.
Fixing the scoping issue turns out to be a major undertaking.
The first (and more obvious) issue is needing to address the scoping problem.
The solution I implemented includes a bit of refactoring to make all the `SemanticsVisitor` types better able to pass around the contextual scope-dependent state that might be needed during semantic checking, but really only adds a single piece of state.
The semantic-checking state used for checking expressions is bottlenecked so that there will (or at least *should*) always be an explicit representation of a "scope" that surrounds a complete expression (as opposed to a sub-expression).
When a `LetExpr` needs to be introduced, it is added to a pending list on the active scope, rather than being added locally.
Once the complete expression is checked, the resulting expression is wrapped up in the pending `LetExpr`s so that their scope is as broad as possible.
Technically this solution doesn't cover all cases. For example:
interface ICell { associatedtype Content; Content getContent(); }
...
ICell cell = ...;
let content = cell.getContent();
In this case the type of `content` refers to the binding introduced by a `LetExpr` in the initial-value expression.
I am leaving such issues as a piece of future work, in the hopes that we can get at least a partial fix for the problem in place.
A future fix probably nees to extend the scoping even wider (e.g., by unwrapping the `LetExpr`s from the initial-value expression and turning them into distinct temporaries).
The second piece of the fix is that we need a way for the modified value of the extracted existential to be "written back" to the original location.
Well...
We are actually being a little slippery here, based on some logic in the compiler codebase that I guess Just Works.
When AST->IR lowering encounters a `LetExpr` that binds an l-value to a name, it actually ends up binding that name more or less as a *reference* to that l-value.
At this point the `let`-ness of `LetExpr` is very much in doubt: the binding can be mutable, and it can even be an *alias* of some location?!?
In any case, the result is that the AST->IR codegen logic implicitly handles the "write-back" because the `let`-bound temporary is actually an alias for the original location.
A more complete future fix might need to introduce a distinct case in `LoweredValInfo` to handle the case of copy of a mutable temporary.
|
|
* Support `[DllImport]`
* Fix.
* Fix.
* Fix array type emit in cpp.
* Fix.
* Fix.
* Fix
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
An earlier refactoring pass over the compiler codebase split the
type that had been called `CompileRequest` into three distinct
pieces:
* `FrontEndCompileRequest` which was supposed to own state and
options related to running the compiler front end and producing
IR + reflection (e.g., what translation units and source
files/strings are included).
* `BackEndCompileRequest` which was supposed to own state and options
related to running the compiler back end to translate the IR
for a `ComponentType` (program) into output code. (Note that the
`BackEndCompileRequest` was conceived of as orthogonal to the
`TargetRequest`s, which store per-target and target-specific
options.)
* `EndToEndCompileRequest` which was an umbrella object that owns
separate front-end and back-end requests, plus any state that is
only relevant when doing a true end-to-end compile (such as the
kinds of compiles initiated with `slangc`). As originally conceived,
the only state that this type was supposed to own was stuff related
to "pass-through" compilation, as well as state related to writing
of generated code to output files.
That refactoring work was very useful at the time, because it allowed
us to "scrub" the back end compilation steps to remove all
dependencies on front-end and AST state (this was important for our
goals of enabling linking and codegen from serialized Slang IR).
At this point, however, it is clear that the hierarchy that was built
up serves very little purpose:
* The `BackEndCompileRequest` type is only used in two places:
* As part of an `EndToEndCompileRequest`, where the settings on
the `BackEndCompileRequest` can be configured, but only through
the `EndToEndCompileRequest`
* As part of on-demand code generation through the `IComponentType`
APIs. In this case, the settings stored on the
`BackEndCompileRequest` are not accessible to the application
at all, and will always use their default values, so that
instantiating a "request" object doesn't really make any sense.
* The `FrontEndCompileRequest` type has a similar situation:
* Front-end compilation as part of an `EndToEndCompileRequest`
supports user configuration of `FrontEndCompileRequest` settings,
but only through the `EndToEndCompileRequest`
* Front-end compilation triggered by an `import` or a `loadModule()`
call does not support user configuration of settings at all. It
will always derive all relevant settings from thsoe on the
session ("linkage").
In addition, subsequent changes have been made to the compiler that
show a bit of a "code smell" and/or forward-looking worries for this
decomposition:
* In some cases we've had to add the same setting to multiple types
in the breakdown (front-end, back-end, end-to-end, linkage, target,
etc.) which makes it harder for us to validate that all the possible
mixtures of state work correctly.
* Related to the above, in some cases we have manual logic that copies
state from one of the objects in the breakdown to another, in order
to ensure that the user's intention is actually followed.
* As a forward-looking concern, it seems that developers have sometimes
added new configuration options and state to places that don't really
make sense according to the rationale of the original decomposition
(e.g., we probably don't want to have a lot of state that is only
available via end-to-end requests, given that the API structure is
meant to push users *away* from end-to-end compiles).
As a result of all of the above, I've been planning a large refactor
with the following big-picture goals:
* Eliminate `BackEndCompileRequest`
* Move all relevant state/options from the back-end request to
the end-to-end request, since that is the only place they could
be set anyway.
* Introduce a transient "context" type to be used for the duration
of code generation that serves the main functions that back-end
requests really served in the codebase
* Make `EndToEndCompileRequest` be a subclass of
`FrontEndCompileRequest`
* Consider addding a transient "context" type for front-end
compiles that can be used in `import`-like cases rather than
needing a full front-end request object. If this works, then
eliminate `FrontEndCompileRequest` and be back to world with
just a single `CompileRequest` type
* Move *all* compiler configuration options to a distinct type (named
something like `CompilerConfig` or `CompilerOptions` or whatever)
which stores setting as key-value pairs, and has a notion of
"inheritance" such that one configuration can extend or build on top
of another. Make all the relevant types use this catch-all structure
instead of redundantly storing flags in many places.
This change deals with the first of those bullets: removeal of
`BackEndCompileRequest`. The addition of the `CodeGenContext` type is
perhaps an unncessary additional step, but making that change helps
clean up a bunch of the code related to per-target code generation,
so I think it is the right choice.
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
Read/write resource types (what D3D/HLSL often refer to as UAVs) can be broadly categorized based on whether they require an underlying format (e.g., a `DXGI_FORMAT`) for reads, or not. D3D refers to the ones that require a format as "typed" UAVs (even though a `RWStructuredBuffer<MyData>` is clearly "typed" at the HLSL level). Vulkan refers to these cases as "storage images" and "storage texel buffers."
Under the D3D model, an application does not have to specify the exact format for a formatted/"typed" UAV in order for loads to work, but it *does* need to specify if an HLSL resource with a declared `float` or vector-of-`float` element type will be backed by data with a `*_UNORM` or `*_SNORM` format. This is where the `unorm` and `snorm` type modifiers come in.
Superficially, it might seem that adding this feature to the Slang compiler is "just" a matter of adding the two modifiers, which is easily done with a pair of one-line `syntax` declarations in `core.meta.slang` plus the corresponding AST node types.
Unfortunately the superficial view misses the detail that, to date, Slang has not had any support for *type modifiers* at all, and has only supported *declaration modifiers*. The distinction has so far not mattered, even with modifiers like `const` because, e.g., the difference between a "`const` array of `float`" and an "array of `const float`" doesn't really matter.
So, adding these two modifiers required introducing a lot of infrastructure along the way. Let's walk through what needed to happen:
* As described above, the actual `syntax` was added easily in the Slang stdlib
* I added a new subclass of `Modifier` for `TypeModifier`s in the AST, and added the AST nodes for `unorm` and `snorm` as subclasses of that.
* In order to syntactically support modifiers applied to types (e.g., `unorm float`), I needed to add a `ModifiedTypeExpr` subclass of `Expr` that represents a base type expression with one or more modifiers applied
* The parser needed some subtle new logic. There are two main cases where type modifiers will come up:
1. In contexts where we might be parsing a declaration (e.g., `const unorm float a`), we need to support a list of modifiers that might freely mix type modifiers and "declaration modifiers" which are not intended to apply to types. In this case we need to split the lis tof modifiers into the type-related ones and the declaration-related ones, and attach each subset to the appropriate place. This is very important for features like C-style pointers, where in `static const float* a;`, the `static` modifier applies to the entire declaration of `a`, but the `const` modifier *only* applies to the `float` type specifier, and *not* to the outer pointer type (the actual type of `a`).
2. In contexts where we are not parsing a declaration (e.g., a generic type argument), we need to support a list of modifiers and appy them *all* to the type specifier being parsed, even if some of them might not be appropriate.
* While working in the parser I implemented a certain amount of unrelated cleanup for code that was using raw `Modifier*`s to represent lists of modifiers, instead of the purpose-built `Modifiers` type.
* The `_parseGenericArg` case needed specific work, because it is an important case in the grammar where we need to parse *either* a type expression or a value exprssion, but cannot easily predict which we will see. The fix implemented for now is to always try to parse modifiers and, if we see any, to assume we are in the type case. Because of the rules for how modifiers in a C-like language inhere to the type specifier (and not necessarily the entire type), we need to refactor some of the type expression parsing routines to support parsing a "suffix" of a type expression.
* Note: I decided to be conservative and only make these changes in `_parseGenericArg` because that is place that is *needed* in order for user code with `unorm`/`snorm` to work, but in practice a user could still confuse our parser by using type modifiers as part of a cast (e.g., `x = (unorm float)y;`). While there is currently no reason why a user should want to do this, it *does* suggest that we need to be prepared to see type modifiers in other ambiguous "expression or type?" contexts. We have so far preferred to avoid looking up built-in syntax declarations like modifiers in expression contexts, because we want to allow users to create variable names that might conflict with some of the more surprising modifier keywords in HLSL (e.g., both `triangle` and `sample` are modifier keyword). A nuanced strategy may be required when we get around to closing this gap (which will be needed around when we want full pointer support, since a cast like `(const SomeType*)somePtr` is pretty common).
* In semantic checking, we now need a `visitModifiedTypeExpr`, which visits the base expression to produce a `Type` and then checks each of the `Modifier`s attached to it. During this process we need to translate the AST-level `Modifier`s into something that can exist properly in the universe of `Type`s. We introduce a `ModifiedType` subclass of `Type`, distinct from the `ModifiedTypeExpr` subclass of `Expr`. Furthermore, we introduce a `ModifierVal` subclass of `Val`, distinct from `Modifier`/`TypeModifier`.
* One unfortunate thing here is that it means we have both, e.g., `UNormModifier` to represent the parsed syntax, and `UNormModifierVal` to represent the `Type`/`Val`-level representation of the same concept. It is quite likely that we are near the point where we can/should consider having two distinct AST representations: one for freshly-parsed ASTs and one for semantically-checked ASTs. The `Type`/`Val` hierarchy clearly belongs to the latter.
* No actual semantic checking is currently being applied to the `unorm` and `snorm` modifiers, although we should in principle check that they are only being applied to `float` and vector-of-`float` types.
* In an attempt to simplify some of the creation logic and build a tiny bit of reusable infrastructure, I went ahead and added the skeleton of a dedupe-caching system in `ASTBuilder` so that we can easily ensure only a single `UNormModifierVal` and a single `SNormModifierVal` ever get created inside the scope of a single builder.
* TODO: Thinking about this, I'm now worried the deduplication does not mean I can make the simplifications I currently do in semantic checking by assuming that any two `UNormModifierVal`s will be pointer-identical. This is because we do not currently (IIRC) have the required "bottleneck" in the compiler where all ASTs get serialized after initial checking, and then deserialized when `import`ed into a downstream module, so that every AST node during a checking step comes from a single `ASTBuilder`. Hmm...
* If we can rely on deduplication to do its thing, then the `Val` and `Type` implementations of modifiers can be relatively simple.
* TODO: One issue here is that the equality comparison for `ModifiedType` currently checks for the same base type and the same modifiers in the same order. This works for now when we only have a small number of type modifiers and any given type will hae at most one, but in the longer run it relies on us to implement some kind of canonicalization scheme, which would both ensure that between `Modified(T, {A, B})` and `Modified(T, {B, A})` only one is allowed (that is, a canonical ordering on modifiers), and that we do not allow `Modified(Modified(T, {A}), {B})`.
* TODO: One other issues is that the `ModifiedType` case does not currently interact correctly with the `as()`-based casting for types (whereas that operation *does* interact in a semantically-correct fashion with `typedef`s). Fixing this issue in a robust way really depends on us re-architecting the `Type` system so that *any* `Type` can have modifiers attached, with modifiers affecting type identity/deduplication.
* The key place where `ModifiedType` creates a complication in semantic checking is type conversion/coercion. A user is likely to declare a `RWTexture2D<unorm float>`, fetch from it (producing a value of type `unorm float`) and then assign the result to a `float` variable, prompting for a conversion from `unorm float` to `float` (because they are distinct `Type`s).
* We handle this case in the core `_coerce()` operation by checking if either `toType` or `fromType` is a `ModifiedType`. If *either* one is a modified type, we apply logic to check for modifiers that are present on one and not the other. Basically we check which modifiers need to be "dropped" and which need to be "added" during conversion, and validate that these modifiers *can* be dropped/added without creating a semantic error. The only type modifiers we support right now *can* be dropped/added like this, so we are fine.
* TODO: When we add more complete pointer support, we could need logic here to validate when casts between, e.g., `const int*` and `int*` should/shouldn't be allowed.
* Note: Even opening the door to type modifiers at all creates the same kind of challenges for user-defined generic types (and functions!) since `MyType<int>` and `MyType<const int>` are distinct instantiations in a future where we support `const` as a type modifier. We *may* need to plan to restrict where modified types can be used, so that certain built-in generic types support modified types as arguments, but user-defined types don't (or at least might need to opt-in to get support).
* The result of a `_coerce()` that drops/adds modifiers is a `ModifierCastExpr`, which is a kind of no-op AST node that merely expresses that the conversion is allowed and valid.
* In IR lowering we currently do the simple thing and translate a `ModifiedType` to a distinct IR node called `AttributedType`.
* The change in terminology from "modifier" to "attribute" is to follow the way that these kinds of modifiers best map to the `IRAttr` case in the IR (rather than the `IRDecoration` case). We probably ought to do a careful terminology scrub here, because having this terminology mismatch between IR and AST could be a source of confusion.
* TODO: In principle, using `IRAttributedType` creates the same basic problems as using `ModifiedType`: code that is usin `as()` or similar operations to check for a specific subclass of `IRType` may not see the case they were looking for due to use of `IRAttributedType`.
* Initially I had hoped to avoid the problem by having the `IRAttr`s be attached directly as operands to an otherwise-ordinary `IRType`. E.g., a lowered `unorm float4` would be an `IRVectorType` with an "extra" operand that is an `IRUNormAttr`, something like: `Vector<Float, 4, UNorm>`. This sounds great (and looks great!), but runs into the problem that it is incompatible with the way we currently represent things like generic type parameters. A generic type parameter `T` is represented as an `IRParam`, and it does *not* make sense to have an additional `IRParam` to represent `const T` or `unorm T`, etc.
* The Right Way to solve this stuff at both the AST and IR levels is to avoid passing around bare `Type*` or `IRType*` in general, and instead use a value type that implements the needed policy more directly: something like a `TypeHolder` or `IRTypeHolder` (placeholder name). The `*Holder` type would abstract over the various "wrapper" nodes required to store all the additional data like attributes but, importantly, would *not* allow that extra information to be dropped or lost during operations like casting (e.g., note how the current `Type` implementation of `as()` loses information on `typedef` names, making our error messages slightly worse). This is actually quite similar to how we currently use the `DeclRef<T>` system to allow working with what is *usually* a `T*` under the hood, but in a way that ensures we don't lose track of any generic substitution information.
* During C-like code emit we have a process that turns an `IRType` into a chain of declarators as needed to emit a C-like declaration with pointers, arrays, etc. The `IRAttributedType` case needs to get folded into this logic. Basically, when we see an `IRAttributedType` we immediately emit any modifiers that are required to be in a prefix position, then recursively emit the underlying type with an extra layer of declarator that tracks the modifiers, so that we can emit any modifiers that should be placed in a postfix position *after* the type. As a specific example, our C/C++ back-end would want to use the postifx option to handle `const`, because then it can properly emit stuff like `int const * const *` and not the incorrect `const const int**`.
* The HLSL emit logic overrides the prefix case for handling type attributes, and uses it to emit `unorm` and `snorm` where they occur.
* One unfortunate detail is that (apparently) some downstream HLSL compilers do not allow the `unorm`/`snorm` modifiers to apply to `vector<float, *>` types, even though that should be semantically valid. Instead, they only support `float`, `float2`, `float3`, and `float4` explicitly. To work around this issue, we go ahead and change our HLSL emit logic so that when we encountered 1-to-4 component vectors of `float`, `int`, or `uint` we emit the type name using the typical HLSL shorthand. This is actually a signficicant change in our HLSL output, but it both seemed like a good fix to have anyway, and was also the only obvious way to address the downstream parser shortcomings without a massive kludge.
* As a result of this change the `half-texture.slang` test broke, since it was using raw HLSL as the expected output. I changed the test to do a DXIL comparison instead, which is our preferred way of testing cross-compilation behavior (since it is more robust in the face of small changes to our source output).
|
|
* #include an absolute path didn't work - because paths were taken to always be relative.
* Fix bool handling in constant folding for generic parameters.
|
|
Fixes #1990
The underlying problem here is in the `ExtractExistentialType` AST node class.
An "existential" in current Slang is typically a value of interface type. When such a value is used in an operation, the type-checker "opens" the extistential so that subsequent type-checking steps can work with the (statically unknown) specific type of the value stored inside. The `ExtractExistentialType` AST node represents the type of an existential that has been "opened" in this way.
When the front-end performs lookup "into" a value with one of these types, it nees to use a reference to the original interface declaration with a "this-type substitution" that refers to the "opened" type (a this-type substitution tells the compiler the concrete type it should use in place of `This` in signatures within the interface; it allows compiler to "see" the right associated type definitions to use in a context).
Prior to this change, the implementation would store the specialized reference to the original interface declaration in the `ExtractExistentialType` node as part of its state. The catch there is that the specialized interface reference indirectly refers to the `ExtractExistentialType` AST node itself, creating a circularity. As soon as the front-end performs any operation that tries to recurse over that structure, it would go into an infinite loop.
The fix here sounds kind of like a hack, but seems to be pretty nice in practice. Instead of always storing the specialized interface reference, we instead store the few values that are needed to construct it, and then create and cache the actual reference on-demand. The on-demand created fields are not considered part of the state of the AST node for any kind of recursion or serialization, so they avoid the original problem.
A single test case was added that represents the original bug, and confirms the fix.
|
|
This change originally started with the simple goal of allowing generic functions with default argument values on their parameters to work:
```
void someFunction<T>(T value, int optional = 0);
```
The core problem there was that the compiler code was (correctly) anticipate the case where the default argument value for a parameter depends on a generic parameter, such as:
```
interface IDefaultable { static This getDefault(); }
void anotherFunction<T : IDefaultable>(T first, T second = T.getDefault());
```
Supporting this latter case requires some kind of ability to apply subsitutions to an `Expr`, but our compiler logic simply errored out in that case. The first major fix that went into this change was to add a new `SubstExpr<T>` type that behaves a lot like `DeclRef<T>` in that it stores a `T*` plus a set of substititions that need to be applied to it.
In addition, it was found that even if `anotherFunction<ConcreteType>(...)` might work, when generic argument inference was used for just `anotherFunction(...)` would fail because it includes a strict match on the number of arguments/parameters in the call expression.
The next problem that arose was that the test I'd created used an interace with an `__init` requirement, and it appeared that our code generation didn't work for that case:
```
interface IStuff { __init(int val); }
void f<T : IStuff>(T x = T(0));
```
In this case, the `T(0)` initialization would get compiled to `(ConcreteType) 0` in the output rather than calling the function generated for the `__init` inside `ConcreteType`. The basic problem there was a bit of crufty old logic we have in place to work around the large number of `__init` declarations in the stdlib that don't have proper `__intrinsic_op` modifiers on them. We really need to fix the underlying problem there, but I worked around it by having the IR lowering pass only do its workaround magic on stdlib declarations.
The next problem down this line was that my test had two different `__init` declarations in the concrete type and the logic for checking interface conformance was picking the wrong one to satisfying an interface requirement despite it being obviously wrong (not even the right number of parameter).
This last problem led me down the rabbit-hole of trying to actually get our semantic checking for interface requirements right. There were a few pieces to that work:
* Actually checking that the parameter and result types for two callables match is the simple part. If that was all that would be required we would have implement this logic a long time ago.
* Next we have to deal with functions that make use of the `This` type, associated types, etc. We have to know that when the interface uses `This`, we want to treat that as equivalent to `ConcreteType`, and similarly for associated types. Getting that working is mostly a matter of setting up a this-type subsitution for the interface member being checked.
* Finally, when comparing generic declarations like `IBase::doThing<T>` and `Derived::doThing<U>` we need to deal with the way that `T` and `U` represent the "same" logical type parameter, but are distinct `Decl`s. This is handled by specializing the base declaration to the parameters of the derived one (e.g., forming `IBase::doThing<U>` using the `U` from `Derived::doThing`).
The result seems to be passing our tests, but there are still a few gotchas lurking, I'm sure.
|
|
* #include an absolute path didn't work - because paths were taken to always be relative.
* Split out AST 'printing'.
* Replace listener with List<Section>
* Section -> Part.
* Kind -> Type Flags -> Kind for ASTPrinter::Part
* Improve comments around ASTPrinter.
* toString -> toText on Val derived types. toText appends to a StringBuilder.
* Added toSlice free function.
Added operator<< for Val derived types.
Use << where appropriate in doing toText.
* More work at mark down output.
* Fill in sourceloc for enum case.
Add more sophisticated location determination for EnumCase.
Refactored documentation output into DocMarkdownWriter.
* Improvements for sig output.
|
|
The basic feature here is the ability to use the `&` operator to produce the conjunction/intersection of two interfaces. That is, you can have interfaces:
interface IFirst { int getFirst(); }
interface ISecond { int getSecoond(); }
and if you need a generic function where the type parameter `T` must conform to *both* of these interfaces, you express that by constraining the parameter to the intersection of the interfaces:
void someFunction<T : IFirst & ISecond>(T value) { ... }
Without this feature, the main alternative an application would have is to define an intermediate interface, like:
interface IBoth : IFirst, ISecond {}
Forcing users to deal with an intermediate interface creates more work for type authors (they need to remember to inherit from the right combined interface(s)), or for `extension` authors (when you add `ISecond` to a type that used to just support `IFirst`, you had better also add `IBoth`). In the worst case, a family of N related "leaf" interfaces would give rise to an exponential number of intermediate interfaces to represnt the possible combinations.
A conjunction like `IFirst & ISecond` is officially its own type, and can be used to declare a type alias:
typealias IBoth = IFirst & ISecond;
This change only includes the first pass of work on this feature, so there are several caveats to be aware of:
* Using a conjunction as part of an inheritance clause is not yet supported (e.g., `struct X : IFirst & ISecond`). This is true even if the conjunction was introduced by an intermediate `typealias`
* The `&` syntax introduced here is only parsed in places where only a type (not an expression) is possible. This means you cannot do things like cast to a conjunction with `(IFirst & ISecond)(someValue)`.
* This work *should* apply to conjunctions of more than two interfaces (like `IA & IB & IC`) but that has not yet been tested
* In the long run it may be sensible to allow conjunctions that use concrete types, but we really ought to have the semantic checking logic rule that out for now.
* During testing, I encountered compiler crashes when trying to use this feature together with `property` declarations. Further investigation and debugging is called for.
* The handling of conjunction types is currently incomplete, in that there are many equivalences the compiler does not yet understand. For example, it is clear that `IA & IB` is equivalent to `IB & IA`, but the compiler currently does not understand this and will treat them as different types. A deeper implementation approach is called for.
* Conjunctions are currently only supported for generic type parameter constraints, when performing full specialization. Use of conjunctions for existential-type value parameters or with dynamic dispatch is not yet supported.
|
|
* Fix constant folding in attributes
* remove unnecessary change
* remove unnecessary change
* remove unnecessary change
* Fixed circular checking issue.
* cleanup
* more cleanup
* minimize diff
* minimize diff
* minimize diff
|
|
* Clean up the way that lookup "through" a base type is encoded
In order to undestand this change, it is important to undestand how lookup through base interfaces works prior to this change. In order to understand *that* it helps to be reminded of how inheritance relationships get encoded in the AST.
Suppose the user writes:
struct Base { int val; }
struct Derived : Base { ... }
...
Derived d = ...;
int v = d.val;
The question is how an expression like `d.val` gets semantically checked, and how it is encoded into the IR after semantic checking. You might assume it gets checked and encoded so that we end up with:
int v = ((Base) d).val;
and that seems like it should Just Work... so of course that isn't what Slang has been doing. Instead, we relied on the fact that the inheritance relationship `Derived : Base` is represented as an `InheritanceDecl` member of the `Derived` type, and we ended up checking the code into something like:
int v = d.<anonymous>.val;
where `<anonymous>` stands in for the name of the `InheritanceDecl` that represents inheritance from `Base`. This design choice makes a limited amount of sense when you consider how inheritance would typically be lowered to a C-like output language:
// struct Derived : Base { ... }
// =>
struct Derived { Base base; ... }
The problem with that encoding is that it really doesn't make sense for almost any other scenario. In particular, if you have a generic type parameter `T` that was constrianed with `T : ISomething`, then the constraint isn't even technically a *member* of the type parameter `T`, so expressing thing as a member reference in the AST is completely incorrect. Unfortunately, by the time it was clear that we needed something better, a bunch of implementation work was done based on the existing representation.
This change tries to clean things up so that lookup of a super-type member through a value of a sub-type does the obvious thing: cast the value to the super-type and then look up the member (as in `((Base) d).val`).
The core of the change is that in lookup, instead of creating `Constraint` breadcrumbs whenever we are looking up in a super-type (with a reference to the `TypeConstraintDecl` being used) we instead use `SuperType` breadcrumbs (with a reference to a `SubtypeWitness`). Then when we create the expression from a `LookupResultItem`, we translate any `SuperType` breadcrumbs into `CastToSuperTypeExpr`s (an expression type that already existed).
This change also adds support for lookup through the `This` type in the context of an interface, and in order for that to work we need a new kind of subtype witness to represent the knowledge that a `This` type is a subtype of the enclosing interface. Making that work forces us to change the representation of `TransitiveSubtypeWitness` so that it takes a pair of subtype witnesses (and not one subtype witness plus one `TypeConstraintDecl`). For the most part this is a small change, but it raises the possibility that some pieces of the code aren't going to be robust against all possible shapes of subtype witnesses.
The IR lowering logic has relied on the weird `d.<anonymous>` representation in order to ensure that when looking up interface members we weren't always casting to the interface type (which would create a `makeExistential` instruction), and then calling using that. Basically, the IR lowering would ignore the `d.<anonymous>` part and just emit `d`, but we can't do that for `((Base) d)` or `((IThing) d)` because whehter or not we should actually perform the cast depends on context.
For now we solve that problem by adding specific logic to ignore up-casts to interface types when they appear in member expressions or method calls. A more robust solution might be needed down the line, but this seems to work in practice.
All of this work is cleanup that I found was needed in order to make `extension`s of `interface` types workable.
* fixup: disable an incorrect test
|
|
* Another fix for overriding property decls
The central problem we keep running into with `property` decls in `interface`s comes down to two choices:
1. When a member lookup `obj.someName` or a simple lookup for `someName` produces an overloaded result, we make no attempt to resolve the overloading right away, and instead postpone disambiguation until the point where that expression gets *used*, in case the context where it gets used can help in disambiguation (a notable case being when there is a call expression `obj.someName(...)` or `someName(...)`).
2. When looking up members in a the scope of a type (either for `obj.someName` or `someName` in the context of a method), we include all results from base types in the set of overloads returned, even in cases where the type has a direct member that "overrides" the inherited one.
The combination of these factors means that when a `struct` type implements a `property` to satisfy a requirement of an inherited `interface`, then references to `obj.someProp` end up being ambiguous between the property in the concrete `struct` type and the property it inherits through the `interface`.
There is no quick fix possible for issue (2). It might seem that we could just skip over members inherited through `interface`s when doing lookup in a type, but that solution wouldn't apply to inheritance from another `struct` type, or any future scenario where we support default implementations of methods in interfaces. The simple idea of saying that a derived-type member named `M` hides all inherited members named `M` is possible, but would lead to a bad user interface when a type wants to support both a core "bottleneck" method and a bunch of convenience overloads with the same name.
That leaves us with issue (1), and trying to find a reasonable fix for it. The common case is that any expression `e` eventually gets used in a context where it will be be subject to disambiguation:
* If we form a call expression `e(...)`, then the overload resolution logic will (obviously) work to disambiguate which `e` was meant.
* If `e` is used as an argument to another call (`f(... e ...)` or `... + e`), then `e` will be coerced to the expected parameter type for its argument position, and that coercion will disambiguate it (this is the bit that was fixed in #1501)
* If `e` is used in another context where a type is expected/known, it will also be coerced: `if(e)`, `int v = e`, etc.
The problem case that is left behind is any scenario where `e` is not subject to one of the above resolution cases, which mostly amounts to cases where an expression is never coerced to a single fixed type. There are a few important cases where this occurs today:
* When the expression is used as the left-hand side of an assignement (`e = ...`).
* When an expression is used to initialize a variable with an implicit type (`let v = e`).
* When inferring generic arguments from the value arguments at a call site (`f(e)` where `f` is defined as `f<T>(T v)`)
The key connecting thread in each of these cases is that the front-end needs to determine the type of `e` to make progress.
Our semantic checking logic already has functions that try to draw a distinction between the two cases:
* The `CheckTerm()` operation is supposed to be used when we expect that we will eventually coerce or otherwise diambiguate the term, and also in cases where we don't yet know if a term should name a type or a value
* The `CheckExpr()` operation is supposed to be used when we do not expect that we will apply coercion/disambiguation to a term, and need to have assurances that it has been coerced into a non-overloaded expression with a reasonable type
The simple part of the fix made here is to make `CheckExpr()` actually do part of what it is suppsoed to (attempt to disambiguate overloaded terms), and then audit all the call sites to `CheckExpr()` to make sure they are actually ones that intend to opt into that logic.
The messier part of the fix is dealing with generic argument inference, because we need to extract the type of the disambiguated expression for the purposes of inference, but we don't want to disturb the actual argument list at a call site (because type coercion of the arguments is supposed to handle the disambiguation). This part is done with a bit of special-casing in the overload-resolution context, by adding a method that gets the type or an argument after disambiguation (when possible).
* fixup
Co-authored-by: Yong He <yonghe@outlook.com>
|
|
dispatch) (#1508)
* Allow calling a generic function with an existential value (dynamic dispatch).
* Fixes per review comments.
* Clean up implementation by having `openExistential` return `ExtractExistentialType` instead of a DeclRef to the interface with a `ThisTypeSubstitution`.
* More cleanups
Co-authored-by: Tim Foley <tfoleyNV@users.noreply.github.com>
Co-authored-by: Yong He <yhe@nvidia.com>
|
|
Our current lookup process always finds *all* members of a type, which can include both an inherited member (e.g., from an `interface`) and one that logically overrides/implements it. If something downstream doesn't filter this result down and favor the derived member, then an ambiguity error will result.
To date, this has mostly been a non-issue because we haven't emphasized inheritance, and the main case we did support (`struct` types implemented `interface` methods) gets disambiguated as part of overload resolution for function calls.
Recent changes to support `property` declarations to `interface`s add the possibility for ambiguity between a "base" and "derived" declaration that can't rely on overload resolution for disambiguation.
The approach in this PR is to add disambiguation logic to the other main place where the results of lookup get used. If a lookup result is being assigned to a variable, passed to a function, or otherwise used in a case where a value of a specific type is needed, it will be "coerced" to the desired type. This change makes it so that the first step in the coercion logic is to try to disambiguate the expression that is being coerced.
In order to ensure that an overloaded expression can be detected and resolved even when just checking if coercion is possible, I needed to update the `canCoerce*()` functions to also take the expression that is being tested for coercibility, and not just its type. There is only one case (that I saw) where coercion checks were being made without an expression value available, and that case didn't actually need/want to handle overloading.
In order to test the fixes here, I added logic to the `property`-in-`interface` test to make sure that the critical cases work as expected (references to a derived member using "dot syntax" and "implicit `this`" syntax).
Alternatives Considered
-----------------------
The first attempt at this fix took a simpler approach: I added the disambiguation logic as a post-process on member lookup. That is, given `obj.foo` I would take the `LookupResult` for `foo` and immediately try to filter it to include only the most-derived members. This approach has the major benefit of catching even more use cases of values (and thus helping to ensure that we don't spend forever chasing down more of these ambiguity errors), but it also has two critical problems:
1. If we only trigger disambiguation when looking up `obj.foo`, then we can't do anything to help when `foo` is looked up as an ordinary identifier, but is actually equivalent to `this.foo`. A full fix would require doing this disambiguation on *every* name lookup, which leads to the second issue:
2. It is important that for a method call like `obj.m(...)` we do *not* disambiguate when looking up `obj.m`, and instead let the overload resolution for the call resolve things. That choice is what makes it possible to call an inherited `m` declaration even when there is a derived `m` with a different signature.
Issue (1) is covered by the test case that was added here, but we should probably have a test case for (2) to make sure we don't break that use case.
Caveats
-------
An important case that we don't solve in this PR is when the result of a lookup is captured in a variable without an explicit type:
let f = obj.foo;
That case also needs disambiguation, and should be addressed in a later change.
A secondary issue is that our approach to prioritizing declarations during lookup is still quite naive. We really need a way for lookup to attach information about nesting of scopes to results (to be clear that results from inner scopes should be preferred over those from outer scopes), as well as have a robust mechanism for comparing the priority of members based on the inheritance graph of a type. This change doesn't do anything to make the situation better or worse.
|
|
There are two main features in this change. First, we allow for `interface`s to declare `property` requirements, which can be satisfied by matching `property` declarations in a type that conforms to the interface:
interface IRectangle
{
property float width { get; }
property float height { get; }
}
struct Square : IRectangle
{
float size;
property float width { get { return size; } }
property float height { get { return size; } }
}
Second, we allow a type to satisfy a `property` requirement with an ordinary field of the same name:
struct Rectangle : IRectangle
{
float width;
float height;
// no explicit `property` declarations needed
}
The implementation of these features is mostly in `slang-check-decl.cpp` in the logic for checking conformance of a type to an interface.
The first feature simply requires adding logic to checking whether a candidate satisfying `property` declaration matches a required `property` declaration. To do so, it must have the same type, and an accessor to satisfy each of the required accessors.
The second feature requires adding logic to synthesize an AST `property` declaration for a type, based on a required `property` declaration and its accessors. This means that, more or less, any type where `this.name` yields a storage location that does what is needed can satisfy a property requirement (there is no specific rule that says the storage needs to be a field, although that is the most likely case).
The way that witnesses are stored for property declarations probably merits some description. During IR lowering, an abstract storage declaration like a subscript or `property` more or less desugars away, so that the actual interface requirements correspond to the accessors within it (the `get`, `set`, etc.). This means that a witness table should have entries/keys corresponding to the accessors and not the property itself. The process of finding/recording witnesses for `property` requirements thus installs entries for the individual accessors (with care taken to only install accessor witnesses once we are sure we have witnesses for all the requirements). Currently, the code also installs an entry for the property itself, although that is not strictly required, and might not be something we continue to do long-term.
(Aside: it was somewhat surprising that an end-to-end test of `property` declarations in `interface`s Just Worked without any changes to IR lowering.)
As we continue to write more code that synthesizes and checks AST expressions/statements, it becomes necessary to refactor the semantic checking logic so that it splits the recursive part (e.g., checking the operands of an assignment) from the validation part (e.g., checking that the assignment itself is valid). It is probably too big of a change to justify at this point, but it might be valuable in the future to have distinct hierarchies that represent unchecked and checked ASTs, with semantic checking mostly being a transformation from one to the other. The benefit of such a change is we could factor out a distinct "builder" API for constructing validated/checked AST nodes, with both semantic checking and AST synthesis being clients of that API.
|
|
* Initial work on property declarations
Introduction
============
The main feature added here is support for `property` declarations, which provide a nicer experience for working with getter/setter pairs.
If existing code had something like this:
```hlsl
struct Sphere
{
float4 centerAndRadius; // xyz: center, w: radius
float3 getCenter() { return centerAndRadius.xyz; }
void setCenter(float3 newValue) { centerAndRadius.xyz = newValue; }
// similarly for radius...
}
void someFunc(in out Sphere s)
{
float3 c = s.getCenter();
s.setCenter(c + offset);
}
```
It can be expressed instead using a `property` declaration for `center`:
```hlsl
struct Sphere
{
float4 centerAndRadius; // xyz: center, w: radius
property center : float3
{
get { return centerAndRadius.xyz; }
set(newValue) { centerAndRadius.xyz = newValue; }
}
// similarly for radius...
}
void someFunc(in out Sphere s)
{
float3 c = s.center;
s.center = c + offset;
}
```
The benefits at the declaration site aren't that signficiant (e.g., in the example above we actually have slightly more lines of code), but the improvement in code clarity for users is significant.
Having `property` declarations should also make it easier to migrate from a simple field to a property with more complex logic without having to first abstract the use-site code using a getter and setter.
An important future benefit of `property` syntax will be if we allow `interface`s to include `property` requirements, and then also allow those requirements to be satisfied by ordinary fields in concrete types.
Subscripts
----------
The Slang compiler already has limited (stdlib-use-only) support for `__subscript` declarations, which are conceptually similar to `operator[]` from the C++ world, but are expressed in a way that is more in line with `subscript` declarations in Swift. A `SubscriptDecl` in the AST contains zero or more `AccessorDecl`s, which correspond to the `get` and `set` clauses inside the original declaration (there is also a case for a `__ref` accessor, to handle the case where access needs to return a single address/reference that can be atomically mutated).
A major goal of the implementation here is to re-use as much of the infrastructure as possible for `__subscript` declarations when implementing `property` declarations.
Nonmutating Setters
-------------------
One additional thing added in this change is the ability to mark a `set` accessor on either a subscript or a property as `[nonmutating]`, and indeed all of the existing `set` accessors declared in the stdlib have been marked this way.
The need for this modifier is a bit subtle. If we think about a typical subscript or property:
```hlsl
struct MyThing
{
int f;
property p : int
{
get { return f; }
set(newValue) { f = newValue; }
}
}
```
it is clear we want the `set` accessor to translate to output HLSL as something like:
```
void MyThing_p_set(inout MyThing this, int newValue)
{
this.f = newValue;
}
```
Note how the implicit `this` parameter is `inout` even though we didn't mark anything as `[mutating]`. This is the obvious thing a user would expect us to generate given a property declaration.
Now consider a case like the following:
```hlsl
struct MyThing
{
RWStructuredBuffer<int> storage;
property p : int
{
get { return storage[0]; }
set(newValue) { storage[0] = newValue; }
}
}
```
This new declaration doesn't require (or want) an `inout` `this` parameter at all:
```
void MyThing_p_set(MyThing this, int newValue)
{
this.storage[0] = newValue;
}
```
In fact, given the limitations in the current Slang compiler around functions that return resource types (or use them for `inout` parameters), we can only support a `set` operation like this if we can ensure that the `this` parameter is considered to be `in` instead of `inout`. This is exactly the behavior we allow users to opt into with a `[nonmutating] set` declaration.
All of the subscript operations in the stdlib today have `set` accessors that don't actually change the value of `this` that they act on (e.g., storing into a `RWStructuredBuffer` using its `operator[]` doesn't change the value of the `RWStructuredBuffer` variable -- just its contents).
We'd gotten away without this detail so far just because `set` accessors were only being declared in the stdlib and they were all implicitly `[nonmutating]` anyway, so it never surfaced as an issue that the code we generated assumed a setter wouldn't change `this`.
Implementation
==============
Parser and AST
--------------
Adding a new AST node for `PropertyDecl` and the relevant parsing logic was mostly straightforward. The biggest change was allowing a `set` declaration to introduce an explicit name for the parameter that represents the new value to be set.
This change also adds a `[nonmutating]` attribute as a dual to `[mutating]`, for reasons I will get to later.
Semantic Checking
-----------------
The `getTypeForDeclRef` logic was updated to allow references to `property` declarations.
Some of the semantic checking work for subscripts was pulled out into re-usable subroutines to allow it to be shared by `__subscript` and `property` declarations.
The checking of accessor declarations, which sets their result type based on the type of the outer `__subscript` was changed to also handle an outer `property`.
Some special-case logic was added for checking of `set` declarations to make sure that their parameter is given the expected type.
Some logic around deciding whether or not `this` is mutable had to be updated to correctly note that `this` should be mutable by default in a `set` accessor, with an explicit `[nonmutating]` modifier required to opt out of this default. (This is the inverse of how a typical method or `get` accessor works).
IR Lowering
-----------
The good news is that after IR lowering, access to properties turns into ordinary function calls (equivalent to what hand-written getters and setters would produce), so that subsequent compiler steps (including all the target-specific emit logic) doesn't have to care about the new feature.
The bad news is that adding `property` declarations has revealed a few holes in how IR lowering was handling `__subscript` declarations and their accessors, so that it didn't trivially work for the new case as-is.
The IR lowering pass already has the `LoweredValInfo` type that abstractly represents a value that resulted from lowering some AST code to the IR. One of the cases of `LoweredValInfo` was `BoundSubscript` that represented an expression of the form `baseVal[someIndex]` where the AST-level expression referenced a `__subscript` declaration. The key feature of `BoundSubscript` is that it avoided deciding whether to invoke the getter, the setter, or both "too early" and instead tried to only invoke the expected/required operations on-demand.
This change generalizes `BoundSubscript` to handle `property` references as well, so it changes to `BoundStorage`. Making the type handle user-defined property declarations required fixing a bunch of issues:
* When building up argument lists in the IR, we need to know whether an argument corresponds to an `in` or an `out`/`inout` parameter, to decide whether to pass the value directly or a pointer to the value. Some of the logic in the lowering pass had been playing fast and loose with this, so this change tries to make sure that whenever we care computing a list of `IRInst*` that represent the arguments to a call we have the information about the corresponding parameter.
* Similarly, when emitting a call to an accessor in the IR, the information about the expected type of the callee was missing/unavailable, and the code was incorrectly building up the expected type of the callee based on the types of the arguments at the call site. The logic has been changed so that we can extract the expected signature of an accessor (how it will be translated to the IR) using the same logic that is used to produce the actual `IRFunc` for the accessor (so hopefully both will always agree).
* Dealing with `in` vs. `inout` differences around parameters means also dealing with the "fixup" code that is used to assign from the temporary used to pass an `inout` argument back into the actual l-value expression that was used. That logic has all been hoisted out of the expression visitor(s) and into the global scope.
Future Work
===========
The entire approach to handling l-values in the IR lowering pass is broken, and it is in need a of a complete rewrite based on new first-principles design goals. While something like `LoweredValInfo` is decent for abstracting over the easy cases of r-values, addresses, and a few complicated l-value cases like swizzling, it just doesn't scale to highly abstract l-values like we get from `__subcript` and `property` declarations, nor other corner cases of l-values that we need to handle (e.g., passing an `int` to an `inout float` parameter is allowed in HLSL, and performs conversions in both directions!).
It Should be Easy (TM) to extend the logic that tries to synthesize an interface conformance witness method when there isn't an exact match to also support synthesizing a property declaration (plus its accessors) to witness a required property when the type has a field of the same name/type.
* fixup: pedantic template parsing error (thanks, clang!)
* fixup: cleanups and review feedback
* Removed some `#ifdef`'d out code from merge change
* Added proper diagnostics for accessor parameter constraints, which led to some fixes/refactorings
* Added a test case for the accessor-related diagnostics
|
|
* Diagnose circularly-defined constants
Work on #1374
This change diagnoses cases like the following:
```hlsl
static const int kCircular = kCircular;
static const int kInfinite = kInfinite + 1;
static const int kHere = kThere;
static const int kThere = kHere;
```
By diagnosing these as errors in the front-end we protect against infinite recursion leading to stack overflow crashes.
The basic approach is to have front-end constant folding track variables that are in use when folding a sub-expression, and then diagnosing an error if the same variable is encountered again while it is in use. In order to make sure the error occurs whether or not the constant is referenced, we invoke constant folding on all `static const` integer variables.
Limitations:
* This only works for integers, since that is all front-end constant folding applies to. A future change can/should catch circularity in constants at the IR level (and handle more types).
* This only works for constants. Circular references in the definition of a global variable are harder to diagnose, but at least shouldn't result in compiler crashes.
* This doesn't work across modules, or through generic specialization: anything that requires global knowledge won't be checked
* fixup: missing files
* fixup: review feedback
|
|
|
|
Fixes #1377
|
|
* 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.
|
|
* First steps toward inheritance for struct types
This change adds the ability for a `struct` type to declare a base type that is another `struct`:
```hlsl
struct Base
{
int baseMember;
}
struct Derived : Base
{
int derivedMember;
}
```
The semantics of the feature are that code like the above desugars into code like:
```hlsl
struct Base
{
int baseMember;
}
struct Derived
{
Base _base;
int derivedMember;
}
```
At points where a member from the base type is being projected out, or the value is being implicitly cast to the base type, the compiler transforms the code to reference the implicitly-generated `_base` member. That means code like this:
```hlsl
void f(Base b);
...
Derived d = ...;
int x = d.baseMember;
f(d);
```
gets transformed into a form like this:
```hlsl
void f(Base b);
...
Derived d = ...;
int x = d._base.baseMember;
f(d._base);
```
Note that as a result of this choice, the behavior when passing a `Derived` value to a function that expects a `Base` (including to inherited member functions) is that of "object shearing" from the C++ world: the called function can only "see" the `Base` part of the argument, and any operations performed on it will behave as if the value was indeed a `Base`. There is no polymorphism going on because Slang doesn't currently have `virtual` methods.
In an attempt to work toward inheritance being a robust feature, this change adds a bunch of more detailed logic for checking the bases of various declarations:
* An `interface` declaration is only allowed to inherit from other `interface`s
* An `extension` declaration can only introduce inheritance from `interface`s
* A `struct` declaration can only inherit from at most one other `struct`, and that `struct` must be the first entry in the list of bases
This change also adds a mechanism to control whether a `struct` or `interface` in one module can inherit from a `struct` or `interface` declared in another module:
* If the base declaration is marked `[open]`, then the inheritance is allowed
* If the base declaration is marked `[sealed]`, then the inheritance is allowed
* If it is not marked otherwise, a `struct` is implicitly `[sealed]`
* If it is not marked otherwise, an `interface` is implicitly `[open]`
These seem like reasonable defaults. In order to safeguard the standard library a bit, the interfaces for builtin types have been marked `[sealed]` to make sure that a user cannot declare a `struct` and then mark it as a `BuiltinFloatingPointType`. This step should bring us a bit closer to being able to document and expose these interfaces for built-in types so that users can write code that is generic over them.
There are some big caveats with this work, such that it really only represents a stepping-stone toward a usable inheritance feature. The most important caveats are:
* If a `Derived` type tries to conform to an interface, such that one or more interface requirements are satisfied with members inherited from the `Base` type, that is likely to cause a crash or incorrect code generation.
* If a `Derived` type tries to inherit from a `Base` type that conforms to one or more interfaces, the witness table generated for the conformance of `Derived` to that interface is likely to lead to a crash or incorrect code generation.
It is clear that solving both of those issues will be necessary before we can really promote `struct` inheritance as a feature for users to try out.
* fixup: trying to appease clang error
* fixups: review feedback
|
|
* Working matrix swizzle.
Supports one and zero indexing and multiple elements.
Performs semantic checking of the swizzle. Matrix swizzles are
transformed into a vector of indexing operations during lowering to the
IR.
This change does not handle matrix swizzle as lvalues.
* Renaming
* Added missing semicolon
* Initialize variable for gcc
* Added the expect file for diagnostics
* Matrix swizzle updated per PR feedback
* Stylistic fix
* Formatting fixes
* Fix compiling with AST change.
Change indentation.
Co-authored-by: jsmall-nvidia <jsmall@nvidia.com>
|
|
* 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.
|
