Making it easier to work with shaders https://git.yummers.dev/slang.git/atom?h=master 2019-05-31T21:20:37+00:00 2019-05-31T21:20:37+00:00 jsmall-nvidia jsmall@nvidia.com 2019-05-31T21:20:37+00:00 urn:sha1:6cbc3929a54d37bd23cb5efa8e3320ba02f78b2f * Prefixing source files in source/slang with slang- * Prefix source in source/slang with slang- prefix. * Rename core source files with slang- prefix. * Update project files. * Fix problems from automatic merge. 2019-05-20T17:40:38+00:00 Tim Foley tfoleyNV@users.noreply.github.com 2019-05-20T17:40:38+00:00 urn:sha1:71e35b6822b9e2846e129a888774d45a5e0827da * A few changes required for application adoption of interface-type parameters There are a few small changes here that are all related in that they arose from trying to integrate support for specialization via global interface-type shader parameters into a real application. Allow querying the "pending" layout via reflection API ------------------------------------------------------ The naming here isn't ideal, and could probably use a round of "bikeshedding" to arrive at something better, but the basic idea is that when you have a type like: ``` struct MyStuff { int a; IFoo foo; int b; } ``` the fields `a` and `b` get allocated space directly in the "primary" layout for `MyStuff` (at offsets 0 and 4, with `sizeof(MyStuff) == 8`), but the `foo` field can't be allocated space until we know what concrete type will get plugged in there. If we have a concrete type in mind: ``` struct Bar : IFoo { int bar; } ``` then we can know how much space the `foo` field will take up, but we still can't allocate it space directly in `MyStuff`, because we already decided that `sizeof(MyStuff) == 8`. Now imagine we place some `MyStuff` values into constant buffers: ``` cbuffer X { MyStuff x; } cbuffer Y { MyStuff y; float4 z; } ``` In each case we know that we want to place the `MyStuff::foo` field at the end of the containing constant buffer so that it doesn't disrupt the layout of the existing fields. But that means that the offset of `MyStuff::foo` relative to the start of the `MyStuff` isn't fixed, because of unrelated fields like `z` that need to get in between. In our layout code, we handle this by having a notion of a "pending" layout. Once we know how `MyStuff::foo` will be specialized, we can compute both a "primary" and a "pending" layout for `MyStuff`, which basically treats it as if it were two distinct types: ``` struct MyStuff_Primary { int a; int b; } struct MyStuff_Pending { Bar foo; } ``` Layout for an aggregate type like the `X` or `Y` constant buffer then proceeds by computing an aggregate primary layout and an aggregate pending layout, and then finally a constant buffer or parameter block "flushes" all or part of the pending data by appending it to the primary data to get the final layout. What all this means is that a type like `MyStuff` will have two different layouts (a default one for the primary data and a "pending" one for any specialized interface-type fields), and a variable like `Y::y` will also have two variable layouts that specify offsets (one set of offsets for its primary part, and one set of offsets for its pending part). In order to handle interface-type fields with these layout rules, an application needs a way to query the "pending" part of a type or variable layout, which luckily gives it back just another type/variable layout. The API change here is minimal, although actually exploiting the new API correctly in application code could prove challenging. Allow creating of explicitly specialized types ---------------------------------------------- This feature isn't actually implemented all the way through the compiler (I just needed enough to make the API calls go through), but I've added support for specializing a type that has interface-type fields through the reflection API. This maps to an `ExistentialSpecializedType` in the AST, and I'm lowering it to the IR as a `BindExistentialsType`, although that isn't 100% correct for the future. This feature will require a future PR to actually flesh out the implementation work, but I'll wait until that is the sticking point on the application side before I do that. Introduce a tiny `Hasher` abstraction ------------------------------------- While implementing all the boilerplate for a new `Type` subclass (we really need to reduce that work...), I got fed up with how we do hash-code computation and introduced a small utility `Hasher` type that is intended to wrap up the idiom of combining hashes. For now this isn't a major change, but in the future I'd like to expand on the design a bit to clean up some of the warts around how we handle hashing: * The `Hasher` implementation can and should switch from maintaining a single `HashCode` as its state to something that contains a more complete state (larger than the hash code) and just hashes new bytes into that state as it goes. This should make it possible to implement a `Hasher` for more serious hash functions, whether MD5, CityHash, or whatever we decide is good default. * Things that are hashable shouldn't have a `getHashCode()` method, but instead should have something like a `hashInto(Hasher&)` method. This change would have the dual benefits that (1) a composite type can easily hash all the fields that contribute to its identity into the hasher with minimal fuss/boilerplate, and (2) the hashes for composite types will be of higher quality because they can exploit all the bits of the hasher's state to combine the fields, instead of restricting each sub-field to just the bits in a hash code. We should be able to incrementally improve the quality of our design there over future changes, but for now it probably isn't a critical priority. Fixes for legalization of existential types ------------------------------------------- There were some missing cases in the handling of type legalization, such that a global interface-type shader parameter that got specialized to a type that contains *only* resource-type fields would cause a crash in the legalization step. I added a test for this case, and then made `ir-legalize-types.cpp` account for this case (the code to handle it ias a bit of a kludge, and shows that the `declareVars()` routine there is getting to a level of complexity that is worrying. * fixup: review feedback 2019-04-29T21:03:46+00:00 jsmall-nvidia jsmall@nvidia.com 2019-04-29T21:03:46+00:00 urn:sha1:4880789e3003441732cca4471091563f36531635 * List made members m_ Tweaked types to closer match conventions. * Use asserts for checking conditions on List. Other small improvements. * List<T>.Count() -> getSize() * List<T> Add -> add First -> getFirst Last -> getLast RemoveLast -> removeLast ReleaseBuffer -> detachBuffer GetArrayView -> getArrayView * List<T>:: AddRange -> addRange Capacity -> getCapacity Insert -> insert InsertRange -> insertRange AddRange -> addRange RemoveRange -> removeRange RemoveAt -> removeAt Remove -> remove Reverse -> reverse FastRemove -> fastRemove FastRemoveAt -> fastRemoveAt Clear -> clear * List<T> FreeBuffer -> _deallocateBuffer Free -> clearAndDeallocate SwapWith -> swapWith * List<T> SetSize -> setSize Reserve -> reserve GrowToSize growToSize * UnsafeShrinkToSize -> unsafeShrinkToSize Compress -> compress FindLast -> findLastIndex FindLast -> findLastIndex Simplify Contains * List<T> Removed m_allocator (wasn't used) Swap -> swapElements Sort -> sort Contains -> contains ForEach -> forEach QuickSort -> quickSort InsertionSort -> insertionSort BinarySearch -> binarySearch Max -> calcMax Min -> calcMin * Initializer::Initialize -> initialize List<T>:: Allocate -> _allocate Init -> _init IndexOf -> indexOf * * Put #include <assert.h> in common.h, and remove unneeded inclusions * Small refactor of ArrayView - remove stride as not used * getSize -> getCount setSize -> setCount unsafeShrinkToSize->unsafeShrinkToCount growToSize -> growToCount m_size -> m_count * Some tidy up around Allocator. * Use Index type on List. * Refactor of IntSet. First tentative look at using Index. * Made Index an Int Did preliminary fixes. Made String use Index. * Partial refactor of String. * String::Buffer -> getBuffer ToWString -> toWString * Small improvements to String. String:: Buffer() -> getBuffer() Equals() -> equals * Try to use Index where appropriate. * Fix warnings on windows x86 builds. 2019-04-08T18:09:03+00:00 Tim Foley tfoleyNV@users.noreply.github.com 2019-04-08T18:09:03+00:00 urn:sha1:dc54f1dd1b694b087816857a791e9d37dc25de6d * Add better control over image formats for GLSL/SPIR-V targets Currently Slang emits GLSL code assuming all R/W images need to have explicit formats, and thus we try to infer a format from the element type of the image. E.g., given a `RWTexture2D<half4>` we might infer that a qualifier of `layout(rgba16f)` should be used. This strategy has two notable shortcomings: * Sometimes the user will want a format that doesn't match an existing HLSL type. E.g., if they want the equivalent of `layout(r11f_g11f_b10f)`, then what should they put in their `RWTexture2D<...>` to make the inference do what they need? * Sometimes the user knows that they don't need to specify a format *at all*, because using the `GL_EXT_shader_image_load_formatted` extension, they can still perform non-atomic load/store on images with no format specified in the SPIR-V. This change adds two features directed at these challenges. First, we add an explicit `[format(...)]` attribute that can be used to specify an explicit image format, including ones that don't match any HLSL type. An example of using this new attribute is: ```hlsl [format("r11f_g11f_b10f")] RWTexture2D<float3> myImage; ``` For simplicity in initial bring-up, the new formats all use the same naming as formats in GLSL (this should make it easy for a programmer who knows what they expect to get in the GLSL output). We can change the naming convention for formats at a later time, so long as we keep these existing names in as a compatibility feature. Note that this is *not* given a `vk::` prefix since the attribute should signal the programmer's intent to provide an image with that format on *all* targets (although only some targets might act on that information). Also note that the attribute takes a string (`[format("rgba8")`) instead of a bare identifier (`[format(rgba8)]`) because this is consistent with the existing convention for attributes in HLSL. When `[format(...)]` is left off, the default compiler behavior will still be to infer a format, but this behavior can be overidden for a single image using an explicit format of `"unknown"`: ```hlsl [format("unknown")] RWTexture2D<float4> mysteryMachine; ``` The second new feature is that if a user knows they are coding for a GPU that supports the `"unknown"` format in all non-atomic cases, then they can opt into making that the default for images without an explicit `[format(...)]`, using the new `-default-image-format-unknown` command-line option for `slangc`. The new test case included with this change confirms that we correctly see the explicit formats in the output GLSL and *no* formats for images without explicit `[format(...)]` when using the new command-line option. The test stresses images declared at global scope, in parameter blocks, and in entry-point parameter lists, to try and make sure that all the relevant IR passes in the compiler preserve the format information. * fixup: missing file 2019-03-27T20:11:31+00:00 Tim Foley tfoleyNV@users.noreply.github.com 2019-03-27T20:11:31+00:00 urn:sha1:c21bffecd9da150576f62ecf8a73a1660717abe9 Fixes #782 There is logic in the compiler to confirm that the argument expression for an `out` or `inout` parameter is an l-value. That logic was producing an internal compiler error if it ran out of arguments while processing the parameter list, on the assumption that this would mean an `out` parameter had a default argument expression (which isn't something we want to support). The problem was that the checking for call expressions will diagnose a call with too few arguments, and then leave the call in the AST to support subequent checking. This meant that any call where the user didn't supply enough arguments *and* one of the trailing argument is `out` or `inout` would produce the error for the original problem (not enough arguments), but then *also* produce the internal error because there is seemingly no argument to match with the `out` or `inout` parameter. The right fix is to not take responsibility for diagnosing this problem at the call site, and instead to rule out default value expressions for `out` and `inout` parameters at the declaration site instead. 2019-03-27T14:50:46+00:00 Tim Foley tfoleyNV@users.noreply.github.com 2019-03-27T14:50:46+00:00 urn:sha1:047adbd4dd3dd6e3098adcb63a8ac475ae06d20d * Overhaul the core routines for implicit conversion The main user-visible change is that we have fixed the bug where conversions that should only be allowed explicitly were being allowed implicitly. This might be seen as a regression by users, so we'll have to be careful when rolling out the fix. The core of that fix involves checking whether an `init` declaration that will be invoked as an implicit conversion actually supports implicit conversions. The main visible change in the code is some renamings to try and help make the core type-coercion routines better fit our naming conventions. The main cleanup is to enforce the invariant that any of the implicit-conversion core routines will always emit a diagnostic (or have a subroutine it calls do so) when conversion fails and the `outToExpr` parameter is non-null. This is a small change, but should improve the user experience if an implicit conversion fails in the context of a single element of an initializer list (the error should point at the line in question, and not at the whole list). The big thing that is impacted by removing the ability to use explicit conversions implicitly is conversion of `enum` types to integers. This was intended to be explicit (a la `enum class` in C++), but the bug made it so that implicit conversion was allowed. Closing up that gap meant that some of the checking around user-defined attributes got wonky, because we attempt to check that the attribute argument is an integer constant expression, but an `enum` case can't possible be an integer constant - it is a value of the `enum` type. I added code to work around that issue by having a parallel path for checking compile-time-constant expressions of `enum` type, but it is clear that a more general solution is needed eventually. * fixup: test case needs explicit cast 2019-03-09T00:24:02+00:00 Tim Foley tfoleyNV@users.noreply.github.com 2019-03-09T00:24:02+00:00 urn:sha1:4f94dd46a2d885e570814dd14a5e46f8e0814802 * Improve support for interfaces as shader parameters This change adds two main things over the existing support: 1. It is now possible to plug in concrete types that actually contain (uniform/ordinary) fields for the existential type parameters introduced by interface-type shader parameters. The `interface-shader-param2.slang` test shows that this works. 2. There is a limited amount of support for doing correct layout computation and generating output code that matches that layout, so that interface and ordinary-type fields can be interleaved to a limited extent. The `interface-shader-param3.slang` test confirms this behavior. There are several moving pieces in the change. * When it comes to terminology, we try to draw a more clear distinction between existial type parameters/arguments and existential/object value parametes/arguments. A simple way to look at it is that an `IFoo[3]` shader parameter introduces a single existential type parameter (so that a concrete type argument like `SomeThing` can be plugged in for the `IFoo`) but introduces three existential object/value parameters (to represent the concrete values for the array elements). * At the IR level, we support a few new operations. A `BindExistentialsType` can take a type that is not itself an interface/existential type but which depends on interfaces/existentials (e.g., `ConstantBuffer<IFoo>`) and plug in the concrete types to be used for its existential type slots. * Then a `wrapExistentials` instruction can take a type with all the existentials plugged in (possibly by `BindExistentialsType`) and wrap it into a value of the existential-using type (e.g., turn `ConstantBuffer<SomeThing>` into a `ConstantBuffer<IFoo>`). * The IR passes for doing generic/existential specialization have been updated to be able to desugar uses of these new operations just enough so that a `ConstantBuffer<IFoo>` can be used. * When we specialize an IR parameter of an interface type like `IFoo` based on a concrete type `SomeThing`, we turn the parameter into an `ExistentialBox<SomeThing>` to reflect the fact that we are conceptually referring to `SomeThing` indirectly (it shouldn't be factored into the layout of its surrounding type). * Parameter binding was updated so that it passes along the bound existential type arguments in a `Program` or `EntryPoint` to type layout, so that we can take them into account. The type layout code needs to do a little work to pass the appropriate range of arguments along to sub-fields when computing layout for aggregate types. * Type layout was updated to have a notion of "pending" items, which represent the concrete types of data that are logically being referenced by existential value slots. The basic idea is that these values aren't included in the layout of a type by default, but then they get "flushed" to come after all the non-existential-related data in a constant buffer, parameter block, etc. * The logic for computing a parameter group (`ConstantBuffer` or `ParameterBlock`) layout was updated to always "flush" the pending items on the element type of the group, so that the resource usage of specialized existential slots would be taken into account. * The type legalization pass has been adapted so that we can derive two different passes from it. One does resource-type legalization (which is all that the original pass did). The new pass uses the same basic machinery to legalize `ExistentialBox<T>` types by moving them out of their containing type(s), and then turning them into ordinary variables/parameters of type `T`. Big things missing from this change include: - Nothing is making sure that "pending" items at the global or entry-point level will get proper registers/bindings allocated to them. For the uniform case, all that matters in the current compiler is that we declare them in the right order in the output HLSL/GLSL, but for resources to be supported we will need to compute this layout information and start associating it with the existential/interface-type fields. - Nothing is being done to support `BindExistentials<S, ...>` where `S` is a `struct` type that might have existential-type fields (or nested fields...). Eventually we need to desugar a type like this into a fresh `struct` type that has the same field keys as `S`, but with fields replaced by suitable `BindExistentials` as needed. (The hard part of this would seem to be computing which slots go to which fields). As a practial matter, this missing feature means that interface-type members of `cbuffer` declarations won't work. The current tests carefully avoid both of these problems. They don't declare any buffer/texture fields in the concrete types, and they don't make use of `cbuffer` declarations or `ConstantBuffer`s over structure types with interface-type fields. * fixup: add override to methods * fixup: typos 2019-03-07T01:16:20+00:00 jsmall-nvidia jsmall@nvidia.com 2019-03-07T01:16:20+00:00 urn:sha1:c850ba44164ac2bee895137abdd184beb4123090 * Added test for scope operator 2019-03-05T22:24:44+00:00 jsmall-nvidia jsmall@nvidia.com 2019-03-05T22:24:44+00:00 urn:sha1:dcd9e574782b87d6280f1db8ee9ba6dbb7c96c8b * Add diagnostic for vk::binding failure. * Add test for vk::binding failure. * Add the expected output for glsl-layout-define.hlsl * * Copy over initialize expr if available when validating unchecked * Fix unloop - because now it always has one parameter (when before it could have none) * Split vk::binding and layout tests with invalid parameters * Removed the diagnostic for 2 ints expected * Added vk::binding that doesn't specify set in vk-bindings.slang * * Fix typo * Improve comments. 2019-03-01T17:43:47+00:00 Tim Foley tfoleyNV@users.noreply.github.com 2019-03-01T17:43:47+00:00 urn:sha1:620af1c60a2e84bbbc0e74f11cb9bc6a6976d9e4 There's a certain amount of logic in `parameter-binding.cpp` that just has to do with the basic problem of enumerating the shader parameters of a `Program`. The main source of complexity is that for legacy/compatibility reasons we need to consider two shader parameters with the same name as being the "same" parameter for layout purposes, and then we need to do a bunch of validation to ensure that these parameters have compatible types. The biggest part of this change is moving that logic to `Program`, so that it builds up a list of its shader parameters during the front-end work, so that any errors related to bad redeclarations will now come up even if we aren't generated target-specific layouts/code. All of the code for `getReflectionName`, `StructuralTypeMatchStack`, etc. is pretty much copy-pasted from `parameter-binding.cpp` over to `check.cpp`, with the `ParameterBindingContext` replaced with a `DiagnosticSink`. The `Program::_collectShaderParameters` function (renamed from `_collectExistentialParams`) then deals with the enumeration and deduplication logic that used to happen in `collectGlobalScopeParameters()`. The new declarations in `compiler.h` reveal the underlying reason for this change: by letting `Program` and `EntryPoint` handle the canonical enumeration of parameters, we can associate each parameter with the range of existential type slots it uses, which will simplify certain work around interfaces (not in this change...). Moving the code out of parameter binding and into `check.cpp` revealed some unused GLSL-related code that I removed while I was at it. I also found that the `IsDeclaration` case of `VarLayoutFlag` wasn't actually being used, so I went ahead and removed it (we can easily re-add it if we ever find a need for it). Overall this isn't a big cleanup (mostly just code moving, rather than being eliminated), but it will facilitate other changes, and it seems cleaner overall to do this work once in target-independent logic, rather than per-target.