summaryrefslogtreecommitdiff
path: root/source
AgeCommit message (Collapse)Author
2019-01-29Add support for user defined attributes.Yong He
2019-01-28Merge branch 'master' into yong-fix2Tim Foley
2019-01-28Support function parameters of existential (interface) type (#802)Tim Foley
* Support function parameters of existential (interface) type The basic idea here is that you can define a function that takes an interface-type parameter: ```hlsl interface IThing { void doSOmething(); } void coolFunction(IThing thing) { ... thing.doSomething() ... } ``` and call it with a concrete value that implements the given interface: ```hlsl struct Stuff : IThing { void doSomething() { /* secret sauce */ } } ... Stuff stuff; coolFunction(stuff); ``` The compiler implementation will specialize `coolFunction` based on the concrete type that was actually passed in, resulting in output code along the lines of: ```hlsl struct Stuff { ... } void Stuff_doSomething(Stuff this) { /* secret sauce */ } void coolFunction_Stuff(Stuff thing) { ... Stuff_doSomething(thing); } ``` In terms of implementation the new specialization approach has been integrated into the existing pass for generic specialization (which has been refactored significantly along the way), because generic specialization can open up opportunities for existential/interface simplification and vice versa, so there is no fixed interleaving of the two passes that can clean up everything. The new logic therefore subsumes the old code for simplifying existential types (which only worked on local variables) in `ir-existential.{h,cpp}`. The local simplification rules from that implementation have become part of the core specialization pass instead, so that they can open up further transformation opportunities enabled by existential-type simplifications. This code in place right now only handles the basic case of a function parameter that directly uses an interface type, and not one that wraps up an interface type in an array, structure, etc. Additional simplifications need to be introduced to deal with those cases as well. * fixup: typos
2019-01-28Fix type legalization to correctly handle empty struct fields.Yong He
2019-01-28Merge branch 'master' into texture-fixYong He
2019-01-28Feature/bit cast glsl (#808)jsmall-nvidia
* First attempt at asint, asuint, asfloat intrinsics. * Test with countbits * Placing glsl definitions first makes them get picked up. * Some more improvements around asint. * Add support for vector versions of asint/asunit * Fix some typos in asuint/asint intrinsics for glsl. Simplified and increased coverage of as/u/int tests. * Added bit-cast-double test. Added notional support for asdouble bit casts to glsl - but couldn't test because glslang doesn't seem to support the extension. * Try to get double bit casts working - doesn't work cos of block issue. * Only output parents on intrinic replacement if return type is not void.
2019-01-25Add GLSL translation rules for `SampleCmp`, `asint` and `asfloat`.Yong He
2019-01-25fix up empty-struct-parametersYong He
2019-01-25Move glsl entry point legalization to later stage of compilation.Yong He
This allows generic types to be used in entry point parameters.
2019-01-25Fix GLSL translation of several Texture* operations (#800)Tim Foley
A user found that the `Texture2D<float2>.Load(...)` operation was not being compiled to GLSL properly, such that it returned a `vec4` instead of the expected `vec2`. The GLSL texture-related functions always return (and take) 4-component vectors, and we already have infrastructure in `emit.cpp` for recognizing a `$z` operator in the GLSL intrinsic definition to stand in for an appropriate swizzle based on teh number of components in the texture result type. This change just adds that `$z` operator to the GLSL code for several more texture operations (including `Load()`) that are defined on a `Texture*<T>` and that return `T`. This change doesn't try to add additional GLSL translations for texture-related operations (e.g., additional variations like `SampleCmp` that we have defined in the stdlib but not given GLSL translations for). That work still needs to be done.
2019-01-25If dxil fails to load, do not report it's missing as an error to the sink. ↵jsmall-nvidia
It's missing will be detected later in processing and output as a warning. (#799)
2019-01-25Fixup handling of empty structs in function return types and parameters. (#796)Yong He
* Fixup handling of empty structs in function return types and parameters. * Bug fix in legalizeFunc() * More comprehensive empty struct test * Fix `legalizeFieldExtract` for empty struct field. * Add empty struct handling for construct inst
2019-01-24Support "modern" declaration syntax as an option (#792)Tim Foley
* Support "modern" declaration syntax as an option Fixed #202 This change adds four new declaration keywords: The `let` and `var` keywords introduce immutable and mutable variables, respectively. They can only be used to declare a single variable at a time (unlike C declaration syntax), and they support inference of the variable's type from its initial-value expression. Examples: ``` let a : int = 1; // immutable with explicit type and initial-value expression let b = a + 1; // immutable, with type inferred var c : float; // mutable, with explicit type var d = b + c; // mutable, with type inferred ``` These declaration forms can be used wherever ordinary global, local, or member variable declarations appeared before. Right now they do not change rules about what is or is not considered a shader parameter. The `static` modifier should work on these forms as expected, but a `static let` variable is *not* the same as a `static const`, so an explicit `const` is still needed if you want that behavior. A `typealias` declaration introduces a named type alias, similar to `typedef`, but with more reasonable syntax. It inherits from the same AST class that `typedef` uses, so all of the code after parsing should be able to treat them as equivalent. To give a simple example: ``` // typedef int MyArray[3]; typealais MyArray = int[3]; ``` A `func` declaration introduces a function. Like `typealias` it re-uses the existing AST class, so there is no need for major changes after parsing. A `func` declaration uses a syntax similar to `let` variables for its parameters, and takes the (optional) result type in a trailing position. For example: ``` func myAdd(a: int, b: int) -> int { return a + b; } ``` If a `func` declaration leaves of the return type clause, the return type is assumed to be `void`. The main difference (beyond the trailing return type) is that the parameters of a `func`-declared function are immutable (unless they are `out`/`inout`). This change doesn't add support for declaring operator overloads with `func`, but that should be added later, and I'd like to make that the only way to declare such operations: ``` func +(left: MyType, right: MyType) -> MyType { ... } ``` The use of `:` for declaring parameter types here means that a function declared with modern syntax currently cannot include HLSL-style semantics on its parameters (or its result). We might consider introducing an `[attribute]`-based syntax for adding semantics to parameters if we think this is important, but for now it is fine to insist that users declare their entry points using traditional syntax. This change strives to avoid unecessary changes after parsing, but if the new syntax catches on with users there are some small ways we can take advantage of it for performance. In particular, since `let` declarations and parameters of modern-style functions are immutable, we do not need to generate read/write local temporaries for them during lowering to the IR (technically we can make the same optimization for `const` locals). In the process of implementing these new forms I also added a few subroutines to help share code better between existing cases in the parser. In particular, parsing of generic parameter lists on declarations that can be generic is now simplified and more unified. * Fixup: remove leftover debugging code * fixup: typos
2019-01-24Fix a regression in geometry shader cross-compilation (#794)Tim Foley
The underlying problem here was that legalization of entry point parameters for GLSL eliminates all the parameters to `main()`, but we still left a dangling reference to one of those parameters if it was a geometry shader output stream. The un-parented parameter would lead to an infinite loop in a later IR step, because it would never be reached by the transformation, and thus could never change its status to the one for "visited" instructions. The fix here is to simply replace any refernces to the GS input stream parameter with an `undefined` instruction in the IR, and then rely on the fact that the downstream GLSL emit logic wouldn't actually reference that value anyway (hence why the danlging reference wasn't originally an issue). I included a basic cross-compilation test case for geometry shaders to try to avoid subsequent regressions like this (Vulkan GS support is one of the most commonly recurring regressions we've had). The comment I put into the IR legalization logic makes it clear that the strategy used there isn't 100% rock-solid anyway (it only works in all the `EmitVertex()` calls come from the shader entry point function, and not subroutines. Adding a better (more robust) translation strategy for geometry shaders would be a nice bit of future work.
2019-01-23Fixing GLSL sign function.Yong He
fixes #602
2019-01-23Fix IR emit logic for methods in `struct` types (#791)Tim Foley
There was a bug in the logic for emitting initial IR, such that it was neglecting to emit "methods" (member functions) unless they were also referenced by a non-member (global) function, or were needed to satisfy an interface requirement. This would only matter for `import`ed modules, since for non-`import`ed code, anything relevant would be referenced by the entry point so that the problem would never surface. This change fixes the underlying problem by adding a step to the IR lowering pass called `ensureAllDeclsRec` that makes sure that not only global-scope declarations, but also anything nested under a `struct` type gets emitted to the initial IR module. There are also a few unrelated fixes in this PR, which are things I ran into while making the fix: * Deleted support for the (long gone) `IRDeclRef` type in our `slang.natvis` file * Added support for visualizing the value of IR string and integer literals when they appear in the debugger * Fixed IR dumping logic to not skip emitting `struct` and `interface` instructions. Switching those to inherit from `IRType` accidentally affected how they get printed in IR dumps by default. * Fixed up the IR linking logic so that it correctly takes `[export]` decorations into account, so that an exported definition will always be taken over any other (unless the latter is more specialized for the target). I initially implemented this in an attempt to fix the original issue, but found it wasn't a fix for the root cause. It is still a better approach than what was implemented previously, so I'm leaving it in place.
2019-01-22Clean up variable declaration class hierarchy (#787)Tim Foley
The AST class hierarchy for variable declarations had a few messy bits. First, there are more subclasses of `VarDeclBase` than seem strictly necessary; especially for stuff like `struct` member variable which use `StructField` even for `static` fields (which are effectively globals). Second, the AST node type for the "cases" within an `enum` was made a subclass of `VarDeclBase` for expediency, but this isn't really semantically accurate (and doesn't seem to be paying off much in deduplication of code). This change tries to address both of those problems. First, we replace the existing `Variable` and `StructField` cases with a single `VarDecl` case that covers globals, locals, and member variables. I haven't gone so far as to replace function parameters or generic value parameters, but that might be worth considering as a further clean-up. Second, we change `EnumCaseDecl` to inherit directly from `Decl` instead of `VarDeclBase` and add an explicit case for handling them where they were previously handled as if they were variable declarations (this was done by manually surveying all locations in the code that referenced `VarDeclBase`).
2019-01-22* Fix comment on VerbosePath flag (#790)jsmall-nvidia
* Only output a 'verbose path' if it's different from the already output nominal path
2019-01-21Feature/file unique identity (#789)jsmall-nvidia
* * Fix memory bug around expanding va_args - needed buffer to have space for terminating 0 * Fix problem with FileWriter defaults being globals, as memory they allocate, will only be freed after return from main - work around by making StdWriters RefObject derived, and kept in scope such the writers are destroyed before checks for leaks is found * Added SimplifyPathAndHash mode for CacheFileSystem - will simplify the path and see if simplified path is in cache before reading file (limiting amout of underlying file requests) * * Added calcReplaceChar * Renamed DefaultFileSystem to OSFileSystem * Made OSFileSystem convert windows \ to / on linux * Simplified logic for caching in CacheFileSystem. * Added pragma-once-c to add extra test, but also so there is an 'include' directory in preprocessor tests. * Small fixes in pragma once test. * Simplified cache handling path, so that paths/simplified paths area always added. * Improve naming of methods for different caches. * Removed references to 'canonicalPath' and made 'uniqueIdentity' * * Re-add support for canonicalPath to ISlangFileSystem -> not for uniqueIdentifier but as a way to display 'canonicalPath' * Added peliminary support for being able to display verbose paths in a diagnostic * Added 'clearCache' support * Added verbose path support to SourceManager (now needs a ISlangFileSystemExt to do this) * Added support for '-verbose-path' option to slangc and slang-test.
2019-01-21Path simplification/hash mode, plus bug fixes (#788)jsmall-nvidia
* * Fix memory bug around expanding va_args - needed buffer to have space for terminating 0 * Fix problem with FileWriter defaults being globals, as memory they allocate, will only be freed after return from main - work around by making StdWriters RefObject derived, and kept in scope such the writers are destroyed before checks for leaks is found * Added SimplifyPathAndHash mode for CacheFileSystem - will simplify the path and see if simplified path is in cache before reading file (limiting amout of underlying file requests) * * Added calcReplaceChar * Renamed DefaultFileSystem to OSFileSystem * Made OSFileSystem convert windows \ to / on linux * Simplified logic for caching in CacheFileSystem. * Added pragma-once-c to add extra test, but also so there is an 'include' directory in preprocessor tests. * Small fixes in pragma once test. * Simplified cache handling path, so that paths/simplified paths area always added. * Improve naming of methods for different caches.
2019-01-18Delete what appears to be stray code in options validation (#784)Tim Foley
The code for options validation had something like: ```c++ { if(rawTarget.redundantProfileSet) if(rawTarget.conflictingProfileSet) { ... } else if(rawTarget.redundantProfileSet) { ... } } ``` The first `if` there seems to be stray code left over from some edit to this logic, since its case is handled later on.
2019-01-17Feature/hash for source identity (#786)jsmall-nvidia
* * Added COMMAND_LINE_SIMPLE test type * Made how spawning works controllable by paramter/type SpawnType * Made break-outside-loop and global-uniform run command line slangc * calcRelativePath -> calcCombinedPath * Add 64 bit version of GetHash. * Add support for Hash based mode for CacheFileSystem.
2019-01-17Fix diagnostic output for command-line slangc (#783)Tim Foley
Somehow the work to support output redirection when running things in `slang-test` ended up interfering with the API-specified diagnostic callback in the `slangc` code. This change doesn't include fixes to the testing infrastructure to prevent a regression like this happening again.
2019-01-16Not finding dxil no longer an error. Outputs a warning. (#781)jsmall-nvidia
* * Allow dxc compilation to take place if dxil is not found. * Output a warning that output will not be signed. * Remove .dll from dxil in warning so more applicable cross platform.
2019-01-16Initial support for dynamic dispatch using "tagged union" types (#772)Tim Foley
* Initial support for dynamic dispatch using "tagged union" types Suppose a user declares some generic shader code, like the following: ```hlsl interface IFrobnicator { ... } type_param T : IFrobincator; ParameterBlock<T : IFrobnicator> gFrobnicator; ... gFrobincator.frobnicate(value); ``` and then they have some concrete implementations of the required interface: ```hlsl struct A : IFrobnicator { ... } struct B : IFrobnicator { ... } ``` The current Slang compiler allows them to generate distinct compiled kernels for the case of `T=A` and the case of `T=B`. This means that the decision of which implementation to use must be made at or before the time when a shader gets bound in the application. This change adds a new ability where the Slang compiler can generate code to handle the case where `T` might be *either* `A` or `B`, and which case it is will be determined dynamically at runtime. This means a single compiled kernel can handle both cases, and the decision about which code path to run can be made any time before the shader executes. This new option is supported by defining a *tagged union* type. Via the API, the user specifies that `T` should be specialized to `__TaggedUnion(A,B)` (the double underscore indicates that this is an experimental and unsupported feature at present). We refer to the types `A` and `B` here as the "case" types of the tagged union. Conceptually, the compiler synthesizes a type something like: ```hlsl struct TU { union { A a; B b; } payload; uint tag; } ``` The user can then allocate a constant buffer to hold their tagged union type, and when they pick a concrete type to use (say `B`), they fill in the first `sizeof(B)` bytes of their buffer with data describing a `B` instance, and then set the `tag` field to the appopriate 0-based index of the case type they chose (in this case the `B` case gets the tag value `1`). Actually implementing tagged unions takes a few main steps: * Type parsing was extended to special-case `__TaggedUnion` as a contextual keyword. This is really only intended to be used when parsing types from the API or command-line, and Bad Things are likely to happen if a user ever puts it directly in their code. Eventually construction of tagged unions should be an API feature and not part of the language syntax. * Semantic checking was extended to recognize that a tagged union like `__TaggedUnion(A,B)` shoud support an interface like `IFrobnicator` whenever all of the case types suport it, as long as the interface is "safe" for use with tagged unions (which means it doesn't use a few of the advancd langauge features like associated types). * The IR was extended with instructions to represent tagged union types and to extract their tag and the payload for the different cases as needed. * IR generation was extended to synthesize implementations of interface methods for any interface that a tagged union needs to support. Right now the implementation is simplistic and only handles simple method requirements, which it does by emitting a `switch` instruction to pick between the different cases. * A new IR pass was introduced to "desugar" any tagged union types used in the code. The downstream HLSL and GLSL compilers don't support `union`s, so we have to instead emit a tagged union as a "bag of bits" and implement loading the data for particular cases from it manually. * Final code emit mostly Just Works after the above steps, but we had to introduce an explicit IR instruction for bit-casting to handle the output of the desugaring pass. There are a bunch of gaps and caveats in this implementation, but that seems reasonable for something that is an experimental feature. The various `TODO` comments and assertion failures in unimplemented cases are intended, so that this work can be checked in even if it isn't feature-complete. * fixup: missing files * fixup: typos
2019-01-16Improve handling of {} initializer list expressions (#778)Tim Foley
Fixes #775 It was reported (in #775) that Slang doesn't handle initializer-list syntax when initializing matrix variables. When starting on a fix for that it became apparent that the time was right to fix two broad issues in the compiler's current handling of `{}`-enclosed initializer lists. The first issue was that the front-end checking of initializer lists wasn't handling the C-style behavior where an initializer list can either contain nested `{}`-enclosed lists for sub-arrays/-structures, or directly contain "leaf" values for initializing those aggregates. For example, the following two variable declarations ought to be equivalent: ```hlsl int4 a[] = { {1, 2, 3, 4}, {5, 6, 7, 8} }; int4 b[] = { 1, 2, 3, 4, 5, 6, 7, 8 }; ``` Getting this distinction right is important because we want to support initializing a matrix either from a list of vectors for its rows, or a list of scalars for its elements (in row-major order). The front-end semantic checking logic for initializer lists was revamped so that it conceptually tries to "read" an expression of a desired type from the initializer list, and decides at each step whether to consume a single expression by coercing it to the desired type, or to recursively read multiple sub-values to construct the type as an aggregate. The logic for deciding between direct vs aggregate initialization could potentially use some tweaking, but luckily it should always handle the case where users introduce explicit `{}`-enclosed sub-lists to make their intention clear, so that existing Slang code should continue to work as before. The second issue was that initializers without the expected number of elements weren't implemented in code generation, so they would lead to internal compiler errors. This change revamps the codegen logic for initializer lists so that it can synthesize default values for fields/elements that were left out during initialization. This includes an attempt to support default initialization of `struct` fields based on explicitly written initialization expressions.
2019-01-16Feature/external compiler reporting (#776)jsmall-nvidia
* Added support for converting SlangResult to string in PlatformUtil. * * Added reportExternalCompilerError * Made external compilers use this * Made DiagnosticSink accept UnownedStringSlice * Made emitXXX compiler functions return SlangError * Use smart pointers to handle life of Com interfaces * * Make SlangResult compatible with HRESULT for some common cases. * Make PlatformUtil::appendResult return SlangResult * Compile check SLANG_RESULT. * Add tests for checking diagnostics from external compilers. * * Make external compiler tests only run on windows for now. * Added 'windows' and 'unix' categories * Added categories based on what backends are available. Will make more tests run on linux and handle case where dxcompiler is not available on appveyor. * * Added spSessionCheckPassThroughSupport * Use to determine whats available for categories for tests * Add support for outputting source filename/s when using pass through.
2019-01-16Fix a bug in IR linking (#777)Tim Foley
The IR linking logic was recently rewritten to use the (optional) `IRLinkageDecoration`s instead of assuming `IRGlobalVals` always have a mangled name field, and in that process a bug seems to have crept in where in the case that an instruction that would usually quality as a "global value" does *not* have linkage, we were failing to register the instruction we create in the output module as a replacement for the original instruction. This problem affects `static` variables inside of functions, leading to them potentially getting emitted multiple times.
2019-01-16Add proper IR codegen support for local static const variables (#779)Tim Foley
Previously the IR codegen logic was treating function-scope `static const` variables just like `static` variables, which results in them generating less efficient output HLSL/GLSL. This change special-cases function-local `static const` variables with logic that mirrors how we handle global-scope `static const` variables. The approach in this change attempts to find a simpler solution to deal with `static const` variables inside of generic functions than what is currently done for `static` variables in generic functions, but I haven't tested whether that works in practice, so I didn't apply the same approach to the plain `static` case. That would make a good follow-on change. I've included a single test case to demonstrate that with this fix the Slang compiler generates output DXBC that uses an indexable "immediate" constant buffer, whereas without the fix it generates an array in local memory (slow).
2019-01-15Fix up declaration checking order for enums (#774)Tim Foley
The logic in `check.cpp` for declaration checking is very messy and needs to be re-written, but in the interim we need to be careful to avoid any cases where a declaration, or some piece of it, gets redundantly checked multiple times. The way the logic had been working, the different "cases" in an `enum` type were being checked twice, and that meant that any initialization expression for a case would be type-checked the first time (potentially leading to a new AST) and then the checked AST would be checked again. This created a problem if the first round of checking introduced any AST nodes that the checking logic would not expect to see (because the parser cannot possibly produce them). The fix here is to follow the style of the other declaration checking cases, where checking is separated into two distinct phases (the "header" phase makes the declaration usable by others, while the "body" phase checks its implementation details for internal consistency). This change includes a test case that produced an internal compiler error before, and compiles without error now.
2019-01-14Add an error for global uniform parameter declarations (#773)Tim Foley
A global uniform parameter in HLSL might canonically be defined like this: ```hlsl uniform float gSomeParameter; ``` The fxc and dxc compilers automatically collect all such parameters into a synthesized constant buffer, along the lines of: ```hlsl cbuffer $Globals { float gSomeParameter; } ``` Slang currently supports parsing and semantic checking of declarations like the above, and computes shader parameter layout/binding information that is appropriate for a constant buffer like `$Globals` above, but it does not include the support to emit HLSL or GLSL code that matches that layout, so that use of global uniforms in Slang is silently unsupported. Making this problem worse, the HLSL language is quite lax, and will parse the following as shader parameters as well: ```hlsl int gCounter = 0; const float kScaleFactor = 2.0f; ``` Each of those declarations introduces a global shader parameter, and then provides a default value for it via the initializer. These declarations do *not* introduce an ordinary global variable or constant as might be expected. (For anybody who wants to know, `static` is required to introduce a "real" global variable (although it will be a *thread-local* global in practice), while `static const` is required to introduce a global constant) I was not too worried about users trying to use global-scope uniforms and failing (since that has fallen out of common HLSL/GLSL practice), but the possibility that users might try to declare global variables/constants and get shader parameters by mistake creates more of a risk so that this hole is worth plugging. The right long-term fix is of course to support the intended semantics of global-scope uniforms, but that feature needs to be prioritized against other requests. A few of the Slang tests were unwittingly relying on this functionality, including some compute tests that seemingly got away with it based on the DXBC generated from the HLSL output by Slang just happening to match the layout they expected. These tests have all been tweaked to use explicit `cbuffer`s or `ParameterBlock`s instead.
2019-01-11Fix some subtle bugs in D3D constant buffer layout (#771)Tim Foley
* Fix some subtle bugs in D3D constant buffer layout The root of the issue here is that the D3D constant buffer layout rules require 16-byte alignment for arrays and structures, but they do *not* round up the size of an array/structure type to account for that alignment. That means that in cases like the following: ```hlsl cbuffer C0 { float3 a[2]; float c0; } struct A { float4 x; float3 y; }; cbuffer C1 { A a; float c1; } ``` The `c0` and `c1` fields get an offset of 28 and not 32 like you might expect if the preceding array/structure field `a` had been padded out to match its 16-byte alignment. The actual fix here is relatively simple, and mostly amount to shuffling around some code in `type-layout.cpp` to ensure that the D3D constant buffer layout don't inherit the logic that was rounding up array/structure sizes. Along the way I took the opportunity to clean up the inheritance hierarchy by making the GLSL-family layout rules not try to share anythign with the D3D family (not that there is very little to share), which in turn allowed for some simplification of the GLSL side of things. Fixing this behavior changed the output of a few reflection tests. In the case of `tests/reflection/arrays.hlsl` the output confirmed that we had been producing bad reflection information in these kinds of cases. The output for `tests/reflection/matrix-layout.slang` also showed some bugs in our reflection, but these were overall more minor: we mis-reported the size of certain matrices as 64 bytes instead of 60, and as a result also computed the size of the overall constant buffer as 4 bytes bigger than needed. In all of these cases I double-checked the expected output against dxc to make sure that the new offsets/sizes are what we should have been producing in the first place. I also updated the reflection test harness to start outputting layout information for the element type of a structured buffer, which changed the output of `tests/reflection/structured-buffer.slang`, but this didn't show any change in what we reported: it is just information that wasn't in the output to begin with. Finally, I added two new tests around these subtle cases of buffer layout behavior (especially subtle because it varies across target APIs). The `tests/compute/buffer-layout.slang` test simply sets up a type to ilustrate the troublesome scenarios and then embeds it in both a constant buffer and structured buffer that will be backed by memory with sequential `int` values. We then read out the value of a field as a way to probe its de facto *offset* at runtime. This test doesn't really stress the Slang compiler (except for our ability to pass through the same type declarations to downstream compilers), but it is useful to confirm our expectations about where things land in memory. The `tests/reflection/buffer-layout.slang` test then uses the reflection test infrastructure to confirm that the same type declarations used in the compute test produce the expected offsets in our reported reflection information. Before the fixes in this change this test showed us producing dangerously incorrect results in our D3D reflection information, which has now been fixed to match the empirically-determined offsets from the compute test. * fixups based on review feedback
2019-01-10Fixes problem when passing in nullptr for the channel - we need the ↵jsmall-nvidia
DiagnosticSink.writer to be nullptr so that we get the buffered result. Unfortunately here, it was set to the default for the channel, which in diagnostics case meant throwing away the diagnostic. (#770)
2019-01-10Improvements around review of debug serialization info (#769)jsmall-nvidia
* * Make SourceView and SourceFile no longer derive from RefObject * Both have life time now managed by SourceManager * Tidied up a little around the serialization test code - just create the IRModule once * Simplified code around deleting SourceView/File. * Looked into generateIRForTranslationUnit - seems reasonable to just call it once, because it has side effects.
2019-01-07Feature/serialization debug info (#767)jsmall-nvidia
* Remove AppContext. Use StdChannels to hold writers, and TestToolUtil to hold test tool specific functionality. * StdChannels -> StdWriters * getStdOut -> getOut, getStdError -> getError * Renamed main.cpp files of tools to try and stop visual studio getting confused between files - such that clicking on an error takes editor to the right location. * Work in progress on being able to serialize debug information. * * Added MemoryStream * First pass converting to IRSerialData * Able to read and write IRSerialData with debug data * Start at reconstruting IR serialized data. * First pass of generation debug SourceLocs from debug data. Works for test set for line nos. * Bug fixes. Moved testing of serialization into IRSerialUtil * Work around problem with irModule = generateIRForTranslationUnit(translationUnit); two times in a row produces different output(!). Fix by just creating once. * Remove problem with use of ternary op in slang.cpp on gcc/clang. * Added -verify-debug-serial-ir option that makes IR modules go through full serialization with debug information and verification. * Add a test that does serial debug verification that is run by default on linux.
2018-12-21Feature/remove app context (#765)jsmall-nvidia
* Remove AppContext. Use StdChannels to hold writers, and TestToolUtil to hold test tool specific functionality. * StdChannels -> StdWriters * getStdOut -> getOut, getStdError -> getError
2018-12-20Feature/lex memory reduction (#762)jsmall-nvidia
* Only do scrubbing if needed. When allocating content try to limit size (with scrubbing each token takes up 1k), now it's 16 bytes min size. * Don't allocate for every call to write on the CallbackWriter - use the m_appendBuffer. * Don't allocate memory for CallbackWriter use m_appendBuffer. * Use UnownedStringSlice for suffix output for parsing float/int literals. Fix typo in invalidFloatingPointLiteralSuffix * Using memory arena to hold tokens that are not in SourceManager. * Improve comment on lexing. * Make UnownedStringSlice allocation simpler on SourceManager. * Fix error on gcc around UnownedStringSlice - because VC converted string + UnownedStringSlice automatically into a String. * Fix generateName needing concat string for gcc. * When constructing a Token in parseAttributeName - because it's a Identifier, we have to set the Name. * Remove translation through String on getIntrinsicOp * Make func-cbuffer-param disablable with -exclude compatibility-issue * Move memory leak in render-test. * From review - can just use "?:" instead of performing a concat.
2018-12-19Refactor several IR passes (#761)Tim Foley
* Refactor several IR passes This change takes some IR passes that lived together in `ir.cpp` and moves them into their own files to improve clarity. In most cases these were passes introduced early in the life of the IR, so that it didn't seem like a big deal to have them all in one file, but now that `ir.cpp` has grown unwieldly this seems like an important cleanup to make. To give a quick rundown of the passes involved: * The IR "linking" step has been pulled out to `ir-link.{h,cpp}`. This code for this pass is pretty much identical to what was in `ir.cpp`, and no attempt has been made to clean up or refactor it in the current change. * The GLSL legalization step has been pulled out to `ir-glsl-legalize.{h,cpp}`. This used to be invoked directly from the linking step, but has been made a new top-level pass invoked from `emit.cpp`. Just like with the linking, the code in the new file is just a copy-paste of what was in `ir.cpp`, and no attempt at cleanup has been made. Also note that it might be a good idea to move this pass later in the overall sequence, but this PR doesn't attempt to do that as it could change results. * The generic specialization step has been pulled out to `ir-specialize.{h,cpp}`. The file name does not explicitly reference *generic* specialization because I anticipate this pass having to perform other kinds of specialization as well. The code in this case amounts to a heavy cleanup/refactoring pass and thus deserves careful scrutiny. The reason for the cleanup is that the generic specialization step used to be part of the "linking" step long ago, and continued to share infrastructure with it long after that stopped making sense. The newly cleaned up pass has much simpler logic that should be easy enough to follow from the comments. * In order to reduce code dulication, the IR "cloning" part of the `ir-specialize-resources.{h,cpp}` pass was pulled into its own files (`ir-clone.{h,cpp}`) that both the generic specialization step and the resource-based specialization step now share. The remaining changes then pertain to deleting a bunch of code out of `ir.cpp` and adding the new files to the build. The only test that needed updating was `vkray/raygen`, where some subtle ordering change in the refactored generic specialization logic has lead to the relative order of the specialized `TraceRay` and `saturate` functions beind reversed. * fixup: typo in assert * fixup: typos in comments
2018-12-18Fix for byte-address buffers on Vulkan (#760)Tim Foley
* Fix output comparison for compute tests There was some vestigial logic there that was first doing a string-based comparison of actual/expected output, and then falling back to a path that parsed the expected output as a float, then converted that to an integer, then printed that integer in hex, and did the comparison with the result of that conversion. I'm not even clear on what that code was trying to accomplish, but its main effect was allowing a test failure to slide by unnoticed becaues somehow an all-zeroes actual output file was matching an expected output file with no zeros. My understanding is that it went something like this: * The first line of expected output was `A` (as in hexidecimal for the decimal integer `10`), and the first line of actual output was `0`. * The `StringToFloat` function was failing on the input string `"A"` and returned `0.0` to indicate failure (rather than reporting any kind of error) * We then converted the `0.0` to integer `0` and converted it to a base-16 string `"0"` * The comparison to the actual output passed, and then a careless early exit in the comparison loop meant that a full test would pass as soon as one line of output passed according to this "second change" logic. This change removes the broken code in the test runner since nothing was relying on it, other than the one broken test case we wanted to fix anyway. * Fix the declarations of byte-address buffer methods for Vulkan The HLSL `ByteAddressBuffer` and `RWByteAddressBuffer` types have methods `Load` and `Store` that take *byte* offsets from the start of the buffer, but we translate them into GLSL that uses `uint[]` array, so that indexing into that array will be off by a factor of four. Somehow the code for mutable byte address buffers was written to add 4, 8, and 12 bytes to the base offset of a vector to get to its subsequent components, but I never thought about the implications this would have for the base address itself. This change includes the following fixes: * Any place in the translation of a byte-address `Load` or `Store` method that was using the address/offset value has been changed to use `$1 / 4` instead of `$1`. * The offsets of 4, 8, and 12 have been changed to 1, 2, and 3 since they are now being added to an *index* instead of a byte offset * The `GetDimensions` methods have introduced a factor of `* 4` to account for the fact that they need to return a byte size and not a count of elements. With this change the existing `byte-address-buffer` test now produces the desired output under Vulkan.
2018-12-17First step toward supporting use of interfaces as existential types (#716)Tim Foley
* First step toward supporting use of interfaces as existential types Traditional generics involve universal quantification. E.g., a declaration like: ``` void drive<T : IVehicle>(T vehicle); ``` indicates for *for all* types `T` that implement the `IVehicle` interface, the `drive()` function is available. In contrast, whend directly using an interface type like: ``` IVehicle v = ...; v.doSomething(); ``` we only know that there *exists* some concrete type (we could call it `E`) such that `v` refers to a value of type `E`, and `E` implements the `IVehicle` interface. In order to perform an operation like `v.doSomething()` we need to "open" the existential value so that we can look at the concrete type and how it implements the `IVehicle.doSomething` requirement. This change adds a very explicit representation of existentials to Slang's IR. An operation like `e = makeExistential(v, w)` creates a value of some existential type (interfaces being our only existential types for now), by wrapping a concrete value `v` (the type of `v` can be seen as an implicit operand) and a witness table `w` showing that the type of `v` implements the requirements of the chosen interface type. In turn, opening of an existential is handled with operations `extractExistential{Value|Type|WitnessTable}` which pull the corresponding piece of information out of a value of existential type (which somewhere in the code had to have been created with `makeExistential`). The change includes a trivial simplification pass that can detect cases where an `extractExistential*` operation is applied direclty to a `makeExistential` operation, so that there is only one possible result that could be extracted. This allows for simplification of existential types used in trivial ways for local variables (this is mostly so I can check in a functional test, rather than to actually support useful code involving interfaces right now). The logic in the semantic checking phase of the compiler is comparatively more complex. When we are about to perform member lookup given an expression like `obj.member` we will first check if `obj` has an existential type, and if it does we will construct a suitable local context in which we extract the value, type, and witness table from the existential (these all become explicit AST expression nodes), and then use the extracted value as the base of the lookup operation. The nature of existential values is that two different values with the same existential (interface) type could wrap concrete values with differnt types, so that we need to carefully refer only to the extracted type/value/witness-table of specific *values*. We handle this right now by conceptually moving the existential-type value into a local variable (by introducing a `LetExpr` that amounts to `let v = <init> in <body>`) and then require that the extract expressions must refer to the (immutable) variable declaration from which they are extracting a value. (Eventually we should expand this so that when using an immutable local variable of existential type we just use that variable as-is rather than introduce a new temporary) A simple test case is included that uses an interface type in an almost trivial way for a local variable; this test can be run and produces the expected results. A more complex test case that passes an existential into a function is included, but left disabled because a more aggressive simplification approach is required to generate working code from it. * Add missing file for expected test output * Fixups for merge from top-of-tree
2018-12-17Specialize away resource-type function parameters (#759)Tim Foley
* Specialize away resource-type function parameters Work on #397. Introduction ------------ Suppose a user writes a function that takes a resource type as a parameter: ```hlsl float4 getThing(RWStructuredBuffer<float4> buffer, int index) { return buffer[index]; } ``` This function creates challenges when generating code for GLSL-based targets, because a global shader parameter of type `RWStructuredBuffer`: ```hlsl RWStructuredBuffer<float4> gBuffer; ``` translates to a global GLSL `buffer` declaration: ```hlsl buffer _S0 { float4 _data[]; } gBuffer; ``` There is no equivalent to that `buffer` declaration that can be used in function parameter position, and it is illegal in GLSL to pass `gBuffer` into a function. (Aside: yes, we could in principle translate a function parameter like `RWStructuredBuffer<float4> buffer` to `float4 buffer[]`, but that will not in turn generalize to arrays of structured buffers; it is a dead-end strategy) The solution employed by many shader compilers is to "inline everything" to eliminate the need for parameters of resource types, and then rely on dataflow optimization to eliminate locals of resource types. This strategy can of course lead to an increase in code size, and it also means that call stacks are lost when doing step-through debugging. Another serious issue is that an "early `return`" from a function can turn into the equivalent of a multi-level `break` when inlined, and not all of our targets support multi-level `break`. The solution implemented in this change works around some, but not all, of the problems with full inlining. The approach here generates specialized versions of a function like `getThing`, adapted to the actual arguments provided at different call sites. Thus if we have code like: ```hlsl RWStructuredBuffer<float4> gA; RWStructuredBuffer<float4> gB[10]; ... getThing(gA, x); getThing(gA, y); getThing(gB[someVal], z); ``` we will generate two specializations of `getThing`: one specialized for the `buffer` parameter being `gA` and the other for `gB`: ```hlsl float4 getThing_gA(int index) { return gA[index]; } float4 getThing_gB(int _val, int index) { return gB[_val][index]; } ``` and the call sites will change to match: ```hlsl getThing_gA(x); getThing_gA(y); getThing_gB(someVal, z); ``` Note how in the case where the argument being passed in was obtained by indexing into an array of resources, the callee is specialized to the identity of the global shader parameter (`gB`), and now accepts a new parameter to indicate the array index into it. While this description motivates the change based on GLSL output, the same basic issue can arise for other targets. For example, while current HLSL has added the `ConstantBuffer<T>` type, it is not supported on older targets, and it turns out that even dxc does not allow functions to have `ConstantBuffer<T>` parameters. Longer-term, we will likely need to do even more aggressive specialization both in order to generate SPIR-V output directly, and also to deal with function that have return values or `out` parameters of resource types. Implementation -------------- The meat of the change is in `ir-specialize-resources.{h,cpp}`, where we have a pass that looks at all call sites (`IRCall` instructions) in the program, and attempts to replace them with calls to specialized functions, where the specializations are generated on-demand. The code in this pass is heavily commented, so hopefully it serves to explain itself all right. After specialization is complete, we may still have functions like the original `getThing` that will produce invalid code when emitted as GLSL, so we need a way to make sure they don't appear in the output. To date we've had some very ad hoc approaches for ignoring IR constructs that we don't want to affect emitted code, but this change goes ahead and adds a more real dead code elimination (DCE) pass in `ir-dce.{h,cpp}`. This pass follows a straightforward approach of tagging instructions that are "live" and then propagating liveness through the whole program, before making a single pass to delete anything that isn't live. When I first added the DCE pass it eliminated *everything* because there were no "roots" for liveness. I solved this for now by adding a new decoration, `IREntryPointDecoration`, to mark shader entry points in the IR which should always be live (as should anything they depend on). A secondary problem that arose was that for GLSL ray tracing shaders it is possible for the incoming/outgoing payload or attributes parameters to be unused, but eliminating them as dead would change the signature of a shader an potential break the rules for how ray tracing programs communicate. I added a very simple `IRDependsOnDecoration` that allows one IR instruction to keep another alive *as if* it used it, without actually using it. There's also a fixup in the IR dumping logic where I was forgetting to store anything in the mapping from instruction to their names, so that the name of an instruction was getting incremented each time it was referenced. Testing ------- There are three different tests added as part of this change: * The `compute/func-resource-param` test covers the basic `RWStructuredBuffer` case above, which we expect to work fine for D3D11/12, but fail for Vulkan without specialization. * The `cross-compile/func-resource-param-array` test covers the case where we don't just have one resource, but an array of them. This is not an end-to-end compute test primarily because our `render-test` application doesn't yet handle arrays of resources correctly in its binding logic. * The `compute/func-cbuffer-param` test covers the case of a function with a `ConstantBuffer<T>` parameter, which requires specialization to become valid for any of our targets. * fixup: warnings/errors from other compilers * fixup: typos and cleanup * fixup: typos
2018-12-17Feature/test tool shared libraries (#758)jsmall-nvidia
* Remove circular reference to renderer on Vk & D3D12 DescriptorSetImpl * Refactor Stbi image loading such that memory is correctly freed when goes out of scope. Added Crt memory dump at termination. Reduced erroneous reporting by scoping TestContext. * Used capitalized acronym for STBImage to keep Tim happy. * Split out TestReporter - to just handle reporting test results Split out Options Made TestContext hold options, and the reporter Removed remaining memory leaks. * Small optimization for rawWrite, such that it directly writes over print.. * Improve comments on TestCategorySet * Fix typos in TestCategorySet * Made slangc a cpp file as part of slang-test (removing need for separate project/shared library). * * Made all test tools only available as dlls. * Made possible to invoke test tool dll from command line slang-test slangc [--bindir xxx] options to slangc * Fix Visual Studio projects that are no longer needed.
2018-12-14Fix memory leaks around slang-test (#757)jsmall-nvidia
* Remove circular reference to renderer on Vk & D3D12 DescriptorSetImpl * Refactor Stbi image loading such that memory is correctly freed when goes out of scope. Added Crt memory dump at termination. Reduced erroneous reporting by scoping TestContext. * Used capitalized acronym for STBImage to keep Tim happy. * Split out TestReporter - to just handle reporting test results Split out Options Made TestContext hold options, and the reporter Removed remaining memory leaks. * Small optimization for rawWrite, such that it directly writes over print.. * Improve comments on TestCategorySet * Fix typos in TestCategorySet
2018-12-14Represent global shader parameters explicitly in the IR (#756)Tim Foley
Before this change, global shader parameters were represented in the IR as just being ordinary global variables. The only indication that a particular global represented a parameter was when it got a layotu attached to it (as part of back-end processing), and we've had a number of bugs related to layouts being dropped so that what should have been a shader parameter turned into an ordinary global variable in the output. This change is more strongly motivated by the fact that making shader parameters look like globals means that we cannot easily reason about their value when doing IR transformations. If we see two `load`s from the same global variable can we assume they yield the same value? In the general case we cannot, and this means that any transformation that wants to rely on the fact that an input `Texture2D` shader parameter can't actually change over the life of the program needs to do extra work. The fix here is to introduce a new kind of IR instruction that represents a global shader parameter directly (not a pointer to it as a global would), at which point there isn't even such a notion as a "load" from the parameter, since it represents the value directly. In several cases logic that used to apply to global variables in case they were shader parameters (by looking for a layout) is now moved to apply to these global parameters. The biggest source of issues in this change was that switching from pointers to plain values to represent these shader parameters stresses different cases in type legalization. I also had to deal with the case of legalization for GLSL where we actually *do* need global shader parameters that are writable (since varying output goes in the global scope), but in that case I borrowed the use of pointer-like `Out<...>` and `InOut<...>` types to represent that intent, which we were already using for function parameters representing outputs. A few tests started failing because the changes lead to a slightly different order of code emission, which in some HLSL tests resulted in a function parameter named `s` getting emitted before a global parameter named `s`, leading to the latter getting the name `s_1` instead of `s_0`. A few SPIR-V tests started failing because the new approach means that we no longer end up performing a load from all varying input parameters at the start of `main` and instead reference the varying inputs directly. The resulting code is more idomatic, but it differed from the baselines for those tests.
2018-12-13Merge branch 'master' into dictfixYong He
2018-12-13Move mangled name out of IRGlobalValue (#752)Tim Foley
* Move mangled name out of IRGlobalValue Previously the `IRGlobalValue` type was used as a root for all IR instructions that can have "linkage," in the sense that a definition in one module can satisfy a use in another module. The mangled symbol name was stored in state directly on each `IRGlobalValue`, which created some complications, and also forced IR instructions that wanted to support linkage to wedge into the hierarchy at that specific point. This change moves the mangled name out into a decoration: either an `IRImportDecoration` or an `IRExportDecoration`, both of which inherit from `IRLinkageDecoration` which exposes the mangled name. This change has a few benefits: * We can now have any kind of instruction be exported/imported, without having to inherit from `IRGlobalValue`. This could potentially let `IRStructType` and `IRWitnessTable` be simplified to just have operand lists instead of dummy chldren as they do today. * We can now easily have "global values" like functions that explicitly *don't* get linkage, instead of using a null or empty mangled name as a marker. * We can use the exact opcode on a linkage decoration to distinguish imports from exports, which could be used to more accurately resolve symbols during the linking step. Other than adding the decorations and making sure that AST->IR lowering adds them, the main changes here are around any code that used `IRGlobalValue`. Variables and parameters of type `IRGlobalValue*` were changed to `IRInst*` easily, so the main challenge was around code that *casts* to `IRGlobalValue*. In cases where a cast to `IRGlobalValue` also performed a test for the mangled name being non-null/non-empty, we simply switched the code to check for the presence of an `IRLinkageDecoration`, since that is the new way of indicating a value with linakge. Most of the serious complications arose in `ir.cpp` around the "linking"/target-specialization and generic specialization steps. The "linking" logic was checking for `IRGlobalValue` to opt into some more complicated cloning logic, and just checking for a linkage decoration here wasn't sufficient since the front-end *does* produce global values without linkage in some cases (e.g., for a function-`static` variable we produce a global variable without linkage). This logic was updated to just check for the cases that used to amount to `IRGlobalValue`s directly by opcode. It might be simpler in the short term to have kept `IRGlobalValue` around to make the existing casts Just Work, but I'm confident that this logic could actually be rewritten for much greater clarity and simplicity and that is the better way forward. The generic specialization logic was using some really messy code to generate a new mangled name to represent the specialized symbol, and then searching for an existing match for that name. The original idea there was that an IR module could include "pre-specialized" versions of certain generics to speed up back-end compilation by eliminating the need to specialize in some cases, but this feature has never been implemented so the overhead here is just a waste. Instead, I moved generic specialization to use a simpler dictionary to map the operands to a `specialize` instruction over to the resulting specialized value. This allows for some simplifications in the name mangling logic, because it no longer needs to figure out how to produce mangled names from IR instructions representing types/values. As part of this change I also overhauled the IR emit logic to produce cleaner output by default, borrowing some of the ideas from the logic in `emit.cpp`. IR values are now automatically given names based on their "name hint" decoration, if any, to make the code easier to follow, and I also made it so that types and literals get collapsed into their use sites in a new "simplified" IR dump mode (which is currently the default, with no way to opt into the other mode without tweaking the code). The resulting IR dumps are much nicer to look at, but as a result the one test that involves IR dumping (`ir/string-literal`) doesn't really test what it used to. One weird issue that came up during testing is that the `transitive-interface` test had previously been producing output that made no sense (that is, the expected output file wasn't really sensible), and somehow these changes were altering its behavior. Changing the test to use `int` values instead of `float` was enough to make the output be what I'd expect, and hand inspection of generating DXBC has me convinced we were compiling the `float` case correctly too. There appears to be some issue around tests with floating-point outputs that we should investigate. * fixup: C++ declaration order
2018-12-13Fix dictionary performance issueYong He
2018-12-12Remove the valist from the ISlangWriter interface. Replace with ↵jsmall-nvidia
begin/endAppendBuffer. (#751)
2018-12-12Make shader parameters not writable (#750)Tim Foley
There was already logic in place to make `const` variables not be writable, and this change extends it to also consider global shader parameters (in HLSL, any global variable that is *not* marked `static` or `groupshared`) to be rvalues instead of lvalues. As noted in the comments on this change, we could lift that restriction later on, if we have good handling of global variables with resource types in the back-end, but for now it is safer to just rule this out in the front-end.
2018-12-12Running tests in slang-test process (#740)jsmall-nvidia
* First pass at having an interface to write text to that can be replaced. Simplifed and made more rigerous the interface used to write formatted strings. * Added AppContext to simplify setting up and parsing around of streams. * Added more simplified way to get the std error/out from AppContext. * Work in progress using dll for tools to speed up testing. * First pass at ISlangWriter interface. * Added support for writing VaArgs. Added NullWriter. * Use ISlangWriter for output. * Use ISlangWriter for output - replacing OutputCallback. Make IRDump go to ISlangWriter * SlangWriterTargetType -> SlangWriterChannel Improvements around AppContext * Shared library working with slang-reflection-test. * Dll testing working for render-test. * Include va_list definintion from header. * Fix errors from clang. * Fix typo for linux. * Added -usexes option * Fix typo. * Fix arguments problem on linux. * Fix typo for linux. * Add windows tool shared library projects. * Fix warning from x86 win build. Fix signed warning from slang-test/main.cpp * First attempt at getting premake to work on travis, and run tests. * Try moving build out into script. * Invoke bash scripts so they don't have to be executable. * Drive configuration/tests from env parameters set by travis * Try using source to run travis tests. * Remove the build.linux directory - but doing so will overwrite Makefile. * Made -fno-delete-null-pointer-checks gcc only. * Try to fix warning from -fno-delete-null-pointer-checks * Turn of warnings for unknown switches. * Try to make premake choose the correct tooling. * Disabled missing braces warning. * Disable -Wundefined-var-template on clang. * -Wunused-function disabled for clang. * Fix typo due to SlangBool. * Remove this nullptr tests. * "-Wno-unused-private-field" for clang. * Added "-Wno-undefined-bool-conversion" * Add DominatorList::end fix. * Split scripts into travis_build.sh travis_test.sh * Fix gcc/clang template pre-declaration issue around QualType. * Fix premake to build such that pthread correctly links with slang-glslang