Skip to main content

Browse the archive

https://github.com/shader-slang/slang
06 April 2024, 09:30:39 UTC
sort by:
Revision Author Date Message Commit Date
d0c9571 jsmall-nvidia 22 June 2018, 17:09:01 UTC Expose macros/functionality for defining interfaces (#604) * Added Result definitions to the slang.h * Removed slang-result.h and added slang-com-helper.h * Move slang-com-ptr.h to be publically available. * Add SLANG_IUNKNOWN macros to simplify implementing interfaces. Use the SLANG_IUNKNOWN macros to in slang.c * Removed slang-defines.h added outstanding defines to slang.h 22 June 2018, 17:09:01 UTC
e66d66b Tim Foley 14 June 2018, 18:56:31 UTC Add support for "blobs" and a file-system callback (#596) * Add support for "blobs" and a file-system callback The most obvious change here is that the Slang header now includes a few COM-style interfaces that can be used for communication between the application and compiler. In order to support the declaration of COM-like interfaces, several platform-detection macros were lifted out of `slang-defines.h` and into the public `slang.h` header. As it exists right now, this change makes the Slang API C++-only, but a C-compatible version can be defined later with the help of lots of macros (and/or something like an IDL compiler). The two big interfaces introduced are: * The `ISlangBlob` interface, which is compatible with `ID3DBlob`, `IDxcBlob`, etc. This is used to pass ownership of source/compiled code across the API boundary without copies. New versions of various entry points have been added to allow passing blobs: e.g., `spAddTranslationUnitSourceBlob` and `spGetEntryPointCodeBlob`. * The `ISlangFileSystem` interface, which is used to allow applications to intercept any attempt by the Slang compiler to load a file (input source files, include files, etc.). This is *not* the same as the `IDxcIncludeHandler` interface, because it assumes UTF-8 encoded path names, instead of the 16-bit encoding that dxc/Windows prefer. It is also not very similar to `ID3DInclude` as used by fxc, because this callback interface is *not* responsible for handling the search through include paths, etc. - it is just a file-system abstraction layer. Internally, a few different parts of the compiler were changed to either store data in blob form all the time, or to be able to synthesize a blob on-demand. Because our internal `String` type is a reference-counted copy-on-write type, using a `SlangStringBlob` to hold string data should achieve transfer of ownership back to the application without extraneous copies. There is plenty of room to clean up the architecture of some of these internal pieces if they *know* that their data will end up in a blob. The existing Slang testing doesn't touch any of the APIs introduced here, so they can only confirm that existing functionality hasn't been broken. The new ability to return code blobs has been tested by integration of that feature into Falcor, but there has been zero testing of the ability to pass *in* source code as blobs, and the ability to hook file loading. Future changes will need to add test coverage for the new features. * fixup: define SLANG_NO_THROW for non-Windows builds * fixup: header copy-paste error caught by clang/gcc * Cleanup: return reference-counted objects via output parameters Returning a reference-counted object through the API as a raw pointer creates challenges. The "obvious" answer is that the returned pointer should have an added reference (it is returned at "+1"), and the caller is responsible for releasing that reference. This makes sense when using raw pointers on the calling side: ```c++ IFoo* foo = spGetFoo(...); ... foo->Release(); ``` However, as soon as smart pointers start getting involved (to handle releasing reference counts when we are done with things), the picture gets more complicated: ```c++ MySmartPtr<IFoo> foo = spGetFoo(...); ... ``` The intention of code like that is that `foo` gets released when the smart pointer goes out of scope, but this probably doesn't happen with most smart pointer implementations. If the `MySmartPtr` constructor that takes a raw pointer retains it, then the destructor will only release *that* reference, and so the object will leak. It is possible that the user will have a smart pointer type where the constructor that takes a raw pointer doesn't retain it, but in general such types introduce the potential for errors of their own, and no matter what the Slang API shouldn't go in assuming any particular policy. This change makes it so that any reference-counted objects that are logically returned from a call are returned through output pointers. This design makes the leak-free cases easy (enough) to implement with raw pointers or smart pointers: ```c++ // raw pointer IFoo* foo = nullptr; spGetFoo(..., &foo); ... foo->Release(); // smart pointer MySmartPtr<IFoo> foo; spGetFoo(..., foo.writeableRef()); ... ``` The only assumption here is that any COM smart-pointer type needs to provide an operation like `writableRef` that is suitable for using that pointer as an output parameter. Given that COM *loves* output parameters, this seems like a safe assumption (at the very least, anybody who interacts with COM would be used to this convention). Future changes might introduce inline convenience methods for various operations that return results more directly, possibly by introducing a minimal smart-pointer type in the `slang.h` header (without prescribing that clients must use it...). * fixup: another error caught by gcc/clang 14 June 2018, 18:56:31 UTC
126e75d Tim Foley 14 June 2018, 14:29:45 UTC Improve generic argument inference for builtins (#598) Fixes #487 The basic problem here is that the user writes something like: ```hlsl float invSqrt2 = 1 / sqrt(2); ``` In this case the user knows that `sqrt()` is only defined for floating-point types, so they expect this to compile something like: ```hlsl float invSqrt2 = float(1) / sqrt(float(2)); ``` The challenge this creates for the Slang compiler is that we use generics to streamline our declarations of all the builtins, so that the scalar `sqrt()` function is actually declared as: ```hlsl T sqrt<T:__BuiltinFloatingPointType>(T value); ``` The `__BuiltinFloatingPointType` is an `interface` defined as part of the standard library, such that only built-in floating-point types conform to it (that is, `half`, `float`, and `double`). When generic argument inference applies to a call like `sqrt(2)`, we see an argument of type `int`, and try to infer `T=int`, which leads to a failure because `int` does not conform to `__BuiltinFloatingPointType`. The point where this currently fails in in the logic to "join" two types for inference, which is supposed to pick the best type that can represent both of two input types. E.g., a join between `float` and `int3` would be `float3`, since both of those types can convert to it, and it is the "minimal" type with that property. So, the goal here is simple: we want a "join" between `int` and `__BuiltinFloatingPointType` to yield the `float` type. The way we handle that in this change is to special case the join of a basic scalar type and an interface, by enumerating all the basic scalar types, filtering them for ones that support the chosen interface and can be implicitly converted from the argument type, and then picking the "best" of them (the comments in the code explain what "best" means in this context). The technique used here could be generalized in the future to deal with user-defined types or more cases, but that would risk slowing down overload resolution even more, which is already the most expensive part of our semantic checking pass. A test case has been added for the specific case of `sqrt()` applied to an `int` argument. 14 June 2018, 14:29:45 UTC
77562ef Tim Foley 13 June 2018, 22:39:04 UTC Make render-test use Slang for all shader compilation (#597) * Make render-test use Slang for all shader compilation This streamlines the code for render-test by having all its shader compilation go through the Slang API, so that it doesn't have to deal with custom logic to compile HLSL->DXBC and HLSL->DXIL. We were already leaning on Slang to generate SPIR-V for Vulkan, so this makes all the paths more consistent. My original plan with this change was to make the D3D12 render path start using DXIL at this point, since the change would make that easy, but it turns out that some aspects of how we handle parameter binding are not compatible with that right now, so it would need to come as a later change. There's a lot of details here, so I will try to walk through the changes, including the incidental ones: * Add logic to `premake5.lua` so that we copy the necessary libraries for HLSL shader compilation to our target directory from the Windows SDK. This is necessary so that our tests can actually invoke `dxcompiler.dll` * Re-run Premake to generate new project files. This moves around a few files that I manually added in previous changes without re-running Premake. * When invoking `fxc` as a pass-through compiler, be sure to pass along any macros defines via API or command-line. This isn't a strictly required change with how things worked out, but it is a positive one anyway, because it makes `slangc -pass-through fxc` more useful. * Don't print output from a downstream `fxc` invocation if it produces warnings but no errors. The main reason for this is so that our tests don't fail because of `fxc` warnings on Slang's output (which then don't match the baselines), but it can also be rationalized as not wanting to confuse users with warnings that don't come from the "real" compiler they are using. This probably needs fine-tuning as a policy. * Add the HLSL `NonUniformResourceIndex` function. This was an oversight because it isn't documented as a builtin on MSDN, and only gets mentioned obliquely when they talk about resource indexing. * Add `glsl_<version>` profiles to match our `sm_<version>` profiles, so that it is easy for a user to use the profile mechanism to request a specific GLSL version without also specifying a stage name. * Update the render-test logic so that there is a single `ShaderCompiler` implementation that *always* uses Slang, and get rid of all of the renderer-specific `ShaderCompiler` implementations. * Update logic in render-test `main.cpp` to select the options to use for the eventual Slang compile based on the choice of renderer and input language. I didn't change the options that render-test exposes, even though they are getting increasingly silly (e.g., `-glsl-rewrite` doesn't use GLSL as its input...). * Note: the D3D12 renderer will still use fxc, DXBC, and SM 5.0 for now, since trying to update it to switch to dxc, DXIL, and SM 6.0 didn't work well at the time. * Add a bit of supporting D3D12 code to make sure that we don't allocate a structured buffer when a buffer has a format. * Make sure to *also* define the `__HLSL__` macro when compiling Slang code, because otherwise a bunch of tests don't work (I'm not clear on how it worked before...). * fixup: missing file 13 June 2018, 22:39:04 UTC
a4dd936 Tim Foley 13 June 2018, 20:56:30 UTC Fix some issues around codegen for l-values and assignment (#601) The problem here arose when a complicated l-value was formed like: ```hlsl struct Foo { float4 a; } RWStructuredBuffer<Foo> gBuffer; gBuffer[index].a.xz += whatever; ``` In this case the `gBuffer[index].a.xz` expression is a complex l-value in multiple ways: * The `gBuffer[index]` subscript could be routed to either a `get` accessor or a `ref` accessor (and maybe also a `set` accessor if we add one to the stdlib definition), and we defer the choice of which to call until as late as possible in codegen today. * The `_.a` part then becomes a "bound member acess" because we can't actually produce a direct pointer until we've resolved how to implement the subscript operation. * The `_.xz` part becomes a "swizzled l-value" because there is *no* way to materialize it as a pointer to contiguous storage in the orignal object (the `x` and `z` components of a vector aren't contiguous). Recent changes to support atomic operations on buffer elements introduced the `ref` accessor on `RWStructuredBuffer`, which made it possible to form a pointer to a buffer element in the IR. This interacted with some code for the "bound member" case that was trying to only introduce a temporary when absolutely necessary, and was doing so by assuming anything with an address didn't need to be moved into a temporary. The first fix is to clean up that logic in the bound-member case for assignment: always create a temporary, rather than do it conditionally. The second fix here is more systemic: we add logic to try to coerce the representation of an l-value during codegen into being a simple address, and employ that in cases where we know an address is desired. In a case like the above this helps to get things into the form that is required, so that a swizzled store can be issued. There is still some potential for cleanup in this logic, but I don't want to introduce more changes than seem necessary to fix the original problem. 13 June 2018, 20:56:30 UTC
860b0d6 Tim Foley 13 June 2018, 18:18:44 UTC Fixes related to handling of empty types (#600) PR #577 tries to eliminate empty `struct` types by replacing them with a `LegalType::tuple` with zero elements, but this seems to run into problems in some cases, where we end up trying to match up `::none` values with empty `::tuple`s. An alternative way to handle this is to never create empty `LegalType::tuple`s (and the same for `LegalVal::tuple`), and instead create `LegalType::none` and `LegalVal::none`. PR #577 avoided this because there were various cases in the legalization logic that didn't robustly handle `LegalType::Flavor::none`. This PR thus includes two main changes: 1. Construct a `::none` type when we have an empty `struct` type. 2. Survery all places that handle the `::tuple` case and extend them to handle the `::none` case if it was missing. This fixes an issue filed in Falcor's internal GitLab as number 424. 13 June 2018, 18:18:44 UTC
167d857 Tim Foley 12 June 2018, 21:59:13 UTC Initial support for enum declarations (#599) Slang `enum` declarations will always be scoped, e.g.: ```hlsl enum Color { Red, Green = 2, Blue, } Color c = Color.Red; // Not just `Red` ``` A user can write `enum class` as a placebo for now (to ease sharing of headers with C++). Slang does not currently support the `::` operator for static member lookup, so it must be `Color.Green` and not `Color::Green`. Support for `::` as an alternate syntax could be added later if there is strong user demand. An `enum` type can have a declared "tag type" using syntax like C++ `enum class`: ```hlsl enum MyThings : uint { First = 0, // ... } ``` The `enum` cases will store their values using that type. An `enum` that doesn't declare a tag type will use the type `int` by default. Enum cases are assigned values just like in C/C++: cases can have explicit values, but otherwise default to one more than the previous case, or zero for the first case. All `enum` types will automatically conform to a standard-library `interface` called `__EnumType`, which is used so that basic operators like equality testing can be defined generically for all `enum` types. This change only adds one operator at first (the `==` comparison), but other should be added later. An `enum` case needs to be explicitly converted to an integer where needed (e.g., `int(Color.Red)`). This is implemented by having the main integer types (`int` and `uint`) support built-in initializers that can work for *any* `enum` type (or rather, anything conforming to `__EnumType`). Eventually these will be restricted so that an `enum` type can only be converted to its associated tag type. IR code generation completely eliminates `enum` types and their cases. The `enum` type will be replaced with its tag type, and the cases will be replaced with the tag values. Currently this could leave some mess in the IR where cast operations are applied between values that actually have the same type. 12 June 2018, 21:59:13 UTC
7852a2b Tim Foley 06 June 2018, 18:59:32 UTC Add basic support for Shader Model 6.3 profiles (#594) * Add basic support for Shader Model 6.3 profiles This adds `vs_6_3` and friends as available profiles, but doesn't add any new builtins specific to Shader Model 6.3. In order to better support the ray tracing shader stages, Slang will not automatically map any attempt to compile a DXR shader up to SM 6.3 (the shader model officially required for these stages) and to the `lib_*` profiles (because there are no stage-specific profiles for these cases). As an added detail, when invoking `dxcompiler.dll` to generate DXIL for DXR shaders, specify an empty entry-point name, since that is expected for `lib_*` profiles. * Fixup: don't drop [shader(...)] attributes The previous change makes the "effective profile" for DXR compiles no longer include a stage, but we had been using the stage stored on the effective profile in exactly one place: when determining what to output for a `[shader("...")]` attribute. This fixup makes it so that we use the stage from the profile on the entry-point layout instead, which seems like the right choice anyway, if we are ever going to emit multiple entry points at once. 06 June 2018, 18:59:32 UTC
1a69812 Tim Foley 06 June 2018, 04:35:48 UTC Fix atomic operations on RWBuffer (#593) * Fix atomic operations on RWBuffer An earlier change added support for passing true pointers to `__ref` parameters to fix the global `Interlocked*()` functions when applied to `groupshared` variables or `RWStructureBuffer<T>` elements. That change didn't apply to `RWBuffer<T>` or `RWTexture2D<T>`, etc. because those types had so far only declared `get` and `set` accessors, but not any `ref` accessors (which return a pointer). The main fixes here are: * Add `ref` accessors to the subscript oeprations on the `RW*` resource types * Adjust the logic for emitting calls to subscript accessors so that we don't get quite as eager about invoking a `ref` accessor, and instead try to invoke just a `get` or `set` accessor when these will suffice. This is important for Vulkan cross-compilation, where we don't yet support the semantics of our `ref` accessors. * Add a test case for atomics on a `RWBuffer` * Fix up `render-test` so that we can specify a format for a buffer resource, which allows us to use things other than `*StructuredBuffer` and `*ByteAddressBuffer`. The work there is probably not complete; I just did what I could to get the test working. * A bunch of files got whitespace edits thanks to the fact that I'm using editorconfig and others on the project seemingly arent... * fixup: remove ifdefed-out code 06 June 2018, 04:35:48 UTC
8b16bbf Tim Foley 04 June 2018, 22:12:56 UTC Emit directives to control matrix layout (#590) The HLSL/GLSL output by Slang should try to be robust against whatever flags somebody uses to compile it. Therefore, we will go ahead and output a target-language-specific directive to control the default matrix layout mode so that we can override whatever might be specified via flags. Also, as long as we are at it, this change goes ahead and makes Slang unconditionally emit row/column-major layout modifiers on all matrices (and arrays of matrices) whereas before these were only being output sometimes (the code to do it seemed buggy to me...). 04 June 2018, 22:12:56 UTC
698ba86 jsmall-nvidia 01 June 2018, 14:41:13 UTC 1st stage renderer binding refactor (#587) * First pass at support for textures in vulkan. * Binding state has first pass support for VkImageView VkSampler. * Split out _calcImageViewType * Fix bug in debug build around constant buffer being added but not part of the binding description for the test. * Offset recalculated for vk texture construction just store the texture size for each mip level. * When outputing a vector type with a size of 1 in GLSL, it needs to be output as the underlying type. For example vector<float,1> should be output as float in GLSL. * Vulkan render-test produces right output for the test tests/compute/textureSamplingTest.slang -slang -gcompute -o tests/compute/textureSamplingTest.slang.actual.txt -vk * Small improvement around xml encoding a string. * More generalized test synthesis. * Fix image usage flags for Vulkan. * Improvements to what gets synthesized vulkan tests. * Do transition on all mip levels. * Fixing problems appearing from vulkan debug layer. * Disable Vulkan synthesized tests for now. * Add Resource::Type member to Resource::DescBase. * Removed the CompactIndexSlice from binding. Just bind the indices needed. * BindingRegister -> RegisterSet * RegisterSet -> RegisterRange * Typo fix for debug build. * Remove comment that no longer applied. 01 June 2018, 14:41:13 UTC
8d77db3 Tim Foley 31 May 2018, 17:50:28 UTC Add options to control matrix layout rules (#583) * Add options to control matrix layout rules Up to this point, the Slang compiler has assumed that the default matrix layout conventions for the target API will be used. This means column-major layout for D3D, and *row major* layout for GL/Vulkan (note that while GL/Vulkan describe the default as "column major" there is an implicit swap of "row" and "column" when mapping HLSL conventions to GLSL). This commit introduces two main changes: 1. The default layout convention is switched to column-major on all targets, to ensure that D3D and GL/Vulkan can easily be driven by the same application logic. I would prefer to make the default be row-major (because this is the "obvious" convention for matrices), but I don't want to deviate from the defaults in existing HLSL compilers. 2. Command-line and API options are introduced for setting the matrix layout convention to use (by default) for each code generation target. It is still possible for explicit qualifiers like `row_major` to change the layout from within shader code. I also added an API to query the matrix layout convention that was used for a type layout (which should be of the `SLANG_TYPE_KIND_MATRIX` kind), but this isn't yet exercised. I added a reflection test case to make sure that the offsets/sizes we compute for matrix-type fields are appropriately modified by the flag that gets passed in. In a future change we could possibly switch the default convention to row-major, if we also changed our testing to match, since there are currently not many clients to be adversely impacted by the change. * Fixup: silence 64-bit build warning 31 May 2018, 17:50:28 UTC
8c593ae jsmall-nvidia 30 May 2018, 15:32:26 UTC GroupMemoryBarrierWithGroupSync only works on groupshared memory - it doesn't block on global memory accesses. The fix is to copy the values to be processed by InterlockedAdd into shared array. The previous test ran successfully on Dx11, but broke on Dx12. (#586) 30 May 2018, 15:32:26 UTC
8b67c7b jsmall-nvidia 29 May 2018, 20:48:04 UTC Feature/vulkan texture (#579) * First pass at support for textures in vulkan. * Binding state has first pass support for VkImageView VkSampler. * Split out _calcImageViewType * Fix bug in debug build around constant buffer being added but not part of the binding description for the test. * Offset recalculated for vk texture construction just store the texture size for each mip level. * When outputing a vector type with a size of 1 in GLSL, it needs to be output as the underlying type. For example vector<float,1> should be output as float in GLSL. * Vulkan render-test produces right output for the test tests/compute/textureSamplingTest.slang -slang -gcompute -o tests/compute/textureSamplingTest.slang.actual.txt -vk * Small improvement around xml encoding a string. * More generalized test synthesis. * Fix image usage flags for Vulkan. * Improvements to what gets synthesized vulkan tests. * Do transition on all mip levels. * Fixing problems appearing from vulkan debug layer. * Disable Vulkan synthesized tests for now. 29 May 2018, 20:48:04 UTC
e7a8332 Tim Foley 29 May 2018, 18:39:55 UTC Fix global atomic functions (#582) Fixes #581 This change adds a new parameter passing mode `__ref` to exist alongisde `in`, `out`, and `inout`. The `__ref` modifier indicates true by-reference parameter passing (whereas `inout` is copy-in-copy-out). This is not intended to be something that users interact with directly, but rather a low-level feature that lets us provide a correct signature for the `Interlocked*()` operations in the standard library. Most of the support for passing what are logically addresses around already exists in the IR, so the majority of the work here is just in introducing the new type `Ref<T>` and then using it appropriately when lowering `__ref` parameters/arguments to the IR. 29 May 2018, 18:39:55 UTC
ace9a8d Yong He 25 May 2018, 14:01:34 UTC Fixes 574. Eliminate empty structs during type legalization (#577) 25 May 2018, 14:01:34 UTC
18709fb Tim Foley 25 May 2018, 02:20:11 UTC A bunch of work to resolve #569 (#576) * render-test should not fail on HLSL compiler *warnings* The logic in `render-test` that invokes `D3DCompile` was causing a test to fail if it produced any warnings (not just if compilation fails). Warning output can be dealt with by the test runner, since it will compare output between runs anyway, and it is useful to be able to run something through `render-test` that compiles with warnings. * Be more careful about deleting IR instructions There was an `IRInst::deallocate()` method that had a precondition that the instruction should already be removed from its parent and clear out all its operands before calling, but it wasn't checking this and the few call sites weren't doing things right either. I consolidated things on `IRInst::removeAndDeallocate()` which does all the things: removes from the parent, clear out operands, and then deallocates. I also made sure to clear out the type operand. This clears up some crashing issues where passes were removing instructions but those instructions would still show up as users of other instructions. * Don't emit bitwise not for non-Boolean types It seems like the logic in `emit.cpp` messed things up and decided that `Not` (the IR instruction that is equivalent to `!` in the AST) should emit as `!` for Boolean types and `~` for other types, but this makes no sense (e.g., `~(a & 1)` is very different from `!(a & 1)`, even when interpreted as a condition). It seems like this logic was intended for the `BitNot` case, where `~a` and `!a` are actually equivalent for Boolean values (but a target language might not like `~a` on `bool` values). Maybe the original plan was that the `Not` instruction should only apply to Boolean values in the first place, and that other values should be converted to `bool` (or a vector of `bool`) before applying `Not`, but even in that case the emit logic makes no sense. This caused an actual problem for one of my test cases, so it was important to fix it now. * Fix issue with cached resolution for overoaded operators The basic problem was that the lookup logic was forming a key based on the *first* definition it found for the overloaded operator, but that means that when processing a prefix `++a` call we might look up the *postfix* definition of `operator++` and decide to use its opcode as the key. This "fixes" the logic by looking for the first definition with a "compatible" definition (e.g., a `__prefix` function if we are checking a `PrefixExpr`), and then uses its opcode. A better fix in the long run would be to make the cache just be keyed on the operator name and the "fixity" of the expression (prefix, postfix, or infix). * Introduce an intermediate structured control-flow representation The code previously used a single function called `emitIRStmtsForBlocks` in `emit.cpp` that would take a logical sub-graph of the CFG and emit it as high-level statements. It would do this by recognizing operations like coniditional branches that it could turn into high-level `if` statements, etc. The main problem with this function was that it mixed together the logic for how we restructure the program with the logic for how we emit high-level code from that structure. This change splits those two parts of the algorithm by introducing an intermediate data structure: a tree of `Region`s, which represent single-entry regions of the CFG. There are subclasses of `Region` corresponding to various structured control-flow constructs, and then a leaf case that wraps a single `IRBlock`. The new function `generateRegionsForIRBlocks()` (in `ir-restructure.cpp`) now handles the restructuring work, by building one or more `Region`s to represent a sub-graph, while `emitRegion()` handles emitting HLSL/GLSL source code from a region. Splitting things in this way opens up some opportunities for future changes: * We can expand the set of IR control-flow constructs allowed, so long as we can still generate structure `Region`s from them, without having to mess with the emit logic (e.g., we could start to support multi-level `break` by introducing temporaries as needed). In the limit we can generate our `Region`s using something like the "Relooper" algorithm. * We can emit to other representations while retaining the same control-flow restructuring support. E.g., if we drop the structured information from the IR, then emitting to SPIR-V for Vulkan would require us to use the strucured control-flow information from these `Region`s. * We can do analysis that needs to understand `Region` structure. This is relevant to issue #569, which was what prompted me to start on this work. Now that we have a representation of the nesting of `Region`s, we can use it to reason about visibility of values between blocks. During development of this change I ran into a gotcha, in that I had been assuming each IR block would map to a single `Region`, forgetting that our current lowering of "continue clauses" in `for` loops leads to them being duplicated. The `Region` representation handles this by having a linked-list struct mapping IR blocks to the `SimpleRegion`s that represent them. I added a test case that includes a `for` loop with a continue clause that is reached along multiple paths just to make sure that we continue to support that case. The compiler output should not change as a result of this work; this is supposed to be a pure refactoring change. * Add a pass to resolve scoping issues in generated code Fixes #569 The basic problem arises because the structured control flow that we output in high-level HLSL/GLSL doesn't match the "scoping" rules of an SSA IR. In particular, SSA says that a value can be used in any block that is dominated by the definition, but in the presence of `break` and `continue` statements it is easy to construct cases where a block dominates something that is not in its scope for structured control flow. Consider: ```hlsl for(;;) { int a = xyz; if(a) { int b = a; break; } int c = a; } int d = b; ``` This program is invalid as HLSL, because the variable `b` is referenced outside of its scope, but if we look at the CFG for this function, it is clear that the block that computes `b` dominated the block that computes `d`. IR optimizations can easily create code like this, so we need to be ready for it. The previous change added an explicit `Region` structure to represent the structured control flow that we re-form out of the IR, and this change adds a pass that exploits the structuring information to detect cases like the above and introduce temporaries to fix the scoping issue. For example, the pass would change the earlier code block into something like: ```hlsl int tmp; for(;;) { int a = xyz; if(a) { int b = a; tmp = b; break; } int c = a; } int d = tmp; ``` That is, we introduce a new `tmp` variable at a scope "above" both the definition and use of `b`, and then we copy `b` into that temporary right where it is computed, and then use the temporary instead of the original `b` at the use site. A few details that came up during the implementation: * Downstream compilers may get confused by code like the above, and complain that `tmp` may be used before it is initialized, even though the very definition of dominators in a CFG means we don't have to worry about it. Still, I introduced some one-off code to initialize the temporaries just to silence spurious warnings coming from fxc. * We need to be careful not to apply this logic to "phi nodes" (the parameters of basic blocks) since they will already be turned into temporaries by the emit logic, and trying to introduce temporaries with this pass led to broken code (I still need to investigate why). It may be that a future version of this pass should also take the code out of SSA form, so that we can introduce both kinds of temporaries in a single pass (and maybe eliminate some unnecessary variables by doing basic register allocation). There is another transformation that could fix some issues of this kind, by moving code out of a structured control-flow construct and to the "join point" after it. For example, we could turn our loop from the start of this commit message into: ```hlsl for(;;) { int a = xyz; if(a) { break; } int c = a; } int b = a; int d = b; ``` Moving the definition of `b` to after the loop is possible because there is no way to get out of the loop without executing that code anyway. Now the scoping issue for `d`'s use of `b` has gone away, but of course we've introduced a *new* scoping issue for `a`, when it gets used by `b`. Adding a pass to re-arrange control flow like this could reduce the cases where we have to apply the current pass, but it wouldn't eliminate them entirely. That means such a pass can be deferred to future work. This change includes a test case the reproduces the original issue, so that we can confirm the fix works. 25 May 2018, 02:20:11 UTC
d7515c3 Tim Foley 23 May 2018, 18:49:18 UTC Fix Slang->GLSL translation for entry point with multiple `out` parameters (#573) Fixes #568 The problem occurs when an entry point declares multiple `out` parameters: ```hlsl void myVS( out float4 a : A, out float4 b : B ) { ... a = whatever; b = somethingElse; ... if(done) { return; // explicit return } ... // implicit return } ``` Slang translates code like this by introducing a GLSL global `out` parameter for each of `a` and `b`, rewriting the logic inside the entry point to use a local temporary instead of the real parameters, and then assigning from the locals to the globals at every `return` site: ```glsl out vec4 g_a; out vec4 g_b; void main() { // insertion location (1) vec4 t_a; vec4 t_b; ... t_a = whatever; t_b = somethingElse; ... if(done) { // insertion location(2) g_a = t_a; g_b = t_b; return; // explicit return } ... // insertion location (3) g_a = t_a; g_b = t_b; // implicit return } ``` Note that there are three different places (for this example) where code gets inserted to make the translation work. We insert declarations of local variables at the top of the function, and then insert the copy from the temporariesto the globals at each `return` site (implicit or explicit). The bug in this case was that the pass was setting the insertion location to (1) outside of the loop for parameters, so that when it was done with `a` and moved on to `b`, it would end up inseting the temporary `t_b` at the last location used (location (3) in this example), and this would result in invalid code, because `t_b` gets used before it is declared. This bug has been around for a while, but it has largely been masked by the fact that so few shaders use multiple `out` parameters, and also because Slang's SSA-ification pass would often be able to eliminate the local variable anyway, so that the bug never bites the user. The reason it surfaced now for a user shader was because we introduced `swizzledStore`, which currently inhibits SSA-ification, so that some temporaries that used to get eliminated are now retained so that they can break things. The fix in this case is small: we use the existing `IRBuilder` only for insertions at location (1) and construct a new builder on the fly for all the insertions at `return` sites. I have not included a test case yet, because our end-to-end Vulkan testing is not yet ready, so this may regress again in the future. 23 May 2018, 18:49:18 UTC
76652fa jsmall-nvidia 23 May 2018, 16:35:43 UTC When outputing a vector type with a size of 1 in GLSL, it needs to be output as the underlying type. For example vector<float,1> should be output as float in GLSL. (#572) 23 May 2018, 16:35:43 UTC
10190da Tim Foley 21 May 2018, 22:53:01 UTC Handle structure initializers in IR type legalization (#567) Fixes #566 The basic problem here is that the front-end translates a structure initializer-list expression into a `makeStruct` instruction (with one argument per field), but the IR type legalization logic wasn't handling the case where a `makeStruct` is used to construct a struct value that needs to get split by legalization. The implementation is relatively straightforward, and like the other cases of instruction legalization for compound types, it follows the shape of the `LegalType`/`LegalVal` cases. The one interesting bit is that we need to be a bit careful and filter the single argument list for `makeStruct` into two in the case where we generate a "pair" type for something that has both "ordinary" and "special" (resource) fields. Luckily the `PairInfo` data that was generated by type legalization has exactly the information we need (by design). This change does not address several issues that could be handled in follow-on changes: * The `makeArray` instruction will face similar issues if it is applied to a type that requires legalization: we'd need to turn an array of `LegalVal`s into a bunch of distinct arrays. * The error message when we hit the unimplemented case here isn't great. Ideally we should provide the line number of the instruction that fails in an error message when legalization fails. This change tries to focus narrowly on the bug at hand, and leave these issues for later changes. 21 May 2018, 22:53:01 UTC
e2c2c22 Tim Foley 11 May 2018, 23:34:19 UTC Generate Visual Studio projects using Premake (#557) * Generate Visual Studio projects using Premake This change adds a `premake5.lua` file that allows us to generate our Visual Studio solution using Premake 5 (https://premake.github.io/). The existing Visual Studio solution/projects are now replaced with the Premake-generated ones, and project contributors will be expected to update these by running premake after adding/removing files. I have *not* changed the Linux `Makefile` build at all, because that file is also used for things like running our tests, so that clobbering it with a premake-generated `Makefile` would break our continuous testing. Hopefully future changes can switch to a generated `Makefile` and perhaps even add an XCode project as well. Notes: * The `build/slang-build.props` file is no longer needed/used, so it has been removed. * The `slang-eval-test` test fixture wasn't following our naming conventions for its directory path, so it was updated to streamline the Premake build configuration work. This required changes to the `Makefile` as well * Some seemingly unncessary preprocessor definitions that were specified for `core` and `slang-glslang` have been dropped. We will see if anything breaks from that. * Possible fixup for Premake vpath issue Premake's `vpath` feature seems to be nondeterministic about the order it applies filters (because Lua isn't deterministic about the order of entries in a key/value table), and as a result we can end up in a weird case where it decides that a `foo.cpp.h` file matches the `**.cpp` filter (I'm not sure why) before it tests against the `**.h` filter. This change uses an (undocumented) Premake facility to set `vpath` using a list of singleton tables, which seems to fix the order in which things get tested. * Remove support for "single-file" build of Slang The `hello` example was the only bit of code that uses the "single-file" way of building Slang, and this had already run up against limitations of the Visual Studio compilers in its Debug|x64 build. Rather than mess with Premake to make it pass through the `/bigobj` linker flag that is needed to work around the issue, it makes more sense just to stop using/supporting the feature since we wouldn't want users to depend on it anyway (our documentation no longer refers to it). While I was at it I went ahead and made sure that the `SLANG_DYNAMIC` flag doesn't need to be set manually, so that instead there is a non-default `SLANG_STATIC` option (not that we have a static-library build of Slang at the moment). 11 May 2018, 23:34:19 UTC
34ecdb7 Tim Foley 11 May 2018, 22:05:12 UTC Add tests for custom #error and #warning messages (#562) Resolves #310 The behavior was fixed in #484, but that change didn't add test cases to cover the new functionality. 11 May 2018, 22:05:12 UTC
5e604a6 Tim Foley 11 May 2018, 20:56:14 UTC Cleanups around behavior when the compiler fails (#553) * Cleanups around behavior when the compiler fails * Add another case where we try to `noteInternalErrorLoc()` if an exception in thrown. This one is the in the logic for emitting an IR instruciton. This could be improved by adding another layer at the function level (as a catch-all for instructions with no location), but something is better than nothing. * Change a bunch of `assert()`s over to `SLANG_ASSERT()`s, so that we can theoretically take more control over them (e.g., make release builds with asserts enabled) * Some other small cleanups around the assertions we perform. In the survey I made, I didn't really see many obvious "smoking gun" cases where we could produce a significantly better error message for some of the unimplemented/unexpected paths, other than to actually implement the missing functionality. * fixup 11 May 2018, 20:56:14 UTC
10c0ffa Tim Foley 11 May 2018, 16:15:04 UTC Add test for associated type from global generic parameter (#561) Resolves #357 The example shader from that issue has been added as a test case, and works with the top-of-tree Slang compiler (most likely due to the changes introduced with the IR-level type system). 11 May 2018, 16:15:04 UTC
2d96e1f Yong He 11 May 2018, 14:51:11 UTC Fixes #559 (#560) 11 May 2018, 14:51:11 UTC
b0413c1 Yong He 11 May 2018, 05:58:05 UTC Merge pull request #558 from tfoleyNV/bad-type-emit-workaround Workaround for cases where we emit illegal-but-unused types 11 May 2018, 05:58:05 UTC
4e07e22 Tim Foley 11 May 2018, 03:28:39 UTC Workaround for cases where we emit illegal-but-unused types This is a quick workaround to deal with cases where we try to emit an unreferenced IR type that contains references to pre-legalization types (which might have been removed from the IR even thought they are still referenced). The basic fix is to *not* add types to our global order of instructions to emit by default, and only add them on demand as they are referenced by other instructions. This is not a real fix for the underlying issue, which is that type legalization is only being applied to a subset of global instructions instead of all of them. A more detailed fix for that problem will need to be devised next. This fix also doesn't address the question of why an unreferenced `struct` type came to be present in the IR code passed to the back-end in the first place. It would be good to understand how this scenario is arising. 11 May 2018, 03:28:39 UTC
140e51e jsmall-nvidia 08 May 2018, 22:17:29 UTC Feature/xunit (#555) * Remove serialization of screen captures from a renderer implementation, capture now writes to a Surface. Then client code can decide to serialize (or use as needed). * Improved comment for captureScreenSurface. * First pass support for xunit output. * Controlling output to improve xunit support. * Xml encoding and writing out of error/skip for xunit. * Fixes to make build on linux. * Fix typo for linux build. 08 May 2018, 22:17:29 UTC
b67f656 Tim Foley 04 May 2018, 20:42:57 UTC Re-enable emission of #line directives and clean up output (#554) This was based on feedback from Falcor users, who felt like changing the default to have no line directives didn't work out well. Since I'd only made them disabled by default based on what I perceived to be Falcor's needs, I'm happy to turn this back on by default. I also added a few changes to clean up the output: * Don't emit a directive for a sub-expression, since that breaks up the code too much. The only directives inside a function body will be on top-level instructions that didn't get folded into a use site. * Add logic to emit a directive for top-level declarations (globals, functions, structs), and clean up their printing so that they put any extra space *after* the declaration rather than before (so the line numbers can be accurate) * Don't emit the file path part of a directive if it would be the same as the previous directive. This makes the output less noisy, at the cost of having to work your way backward to find the file if you are looking directly at the output. There are certainly more cleanups possible, but these make the output decent enough to be useful for working backwards from a downstream compiler error to the offending code. 04 May 2018, 20:42:57 UTC
07a59b6 Tim Foley 04 May 2018, 19:01:30 UTC Allow more complex compound expressions when emitting from IR (#552) The emit logic already had an idea of when an instruction should be "folded" it its use site(s), and this change just expands on that logic to try to be more aggressive. The basic idea is that instead of outputting this: ```hlsl float4 _S3 = a_0 + b_0; float4 _S4 = c_0 * _S3; d_0 = _S4; ``` we can hopefully output something like this: ```hlsl d_0 = c_0 * (a_0 + b_0); ``` The way this works is that after dealing with the various special cases that decide an instruction `I` must/cannot be folded in, we look and see if it has the following properites: * `I` has no side effects * `I` has a single user, `U` * `I` and `U` are in the same block (and `I` comes before `U` in that block) * for every instruction `X` between `I` and `U` (exclusive), `X` has no side effects If all of these conditions are true, then `I` can be folded in as a sub-expression when we emit `U`. This change doesn't affect most of our test output, but there is still a single test with SPIR-V output that we compare against a GLSL baseline, and so that baseline had to be modified to match the GLSL we now generate. Similar to #547, this change is not meant to provide a complete solution, but rather to take a concrete but low-risk step toward improving our output. Opportunities to improve the results further include: * We can/should ensure that when outputting sub-expressions we keep extra parentheses to a minimum. The old logic for emitting from an AST had support for "unparsing" expressions with minimal parentheses, and we should try to do the same. This can be error-prone, because omitting parentheses can lead to silent failures, so it must be done carefully. * We could try to be more aggressive about detecting what operations might have side effects. The most interesting case is function calls, where we should try to check if the callee is a function known to be side-effect-free. We could start by annotating most builtin functions with an attribute/decoration that indicates freedom from side effects. Deriving this attribute for user functions could be interesting, but we'd have to be careful since "nontermination" is technically a side effect. * We could try to be more aggressive about determining what side effects in instructions `X` are "safe" for the instruction `I` to move across. For example, if `I` is a load from variable `a` and `X` is a store to variable `b`, then that would seem to be safe. This starts to get into issues of instruction scheduling, though, and that is probably beyond what we want Slang to be doing. 04 May 2018, 19:01:30 UTC
ee47232 jsmall-nvidia 04 May 2018, 16:00:53 UTC Use Surface for screen capture in Renderer interface (#551) * Remove serialization of screen captures from a renderer implementation, capture now writes to a Surface. Then client code can decide to serialize (or use as needed). * Improved comment for captureScreenSurface. 04 May 2018, 16:00:53 UTC
494330d Tim Foley 04 May 2018, 00:40:26 UTC Add a pass for computing dominator trees (#541) This code is currently not used by anything, but I wanted to check in a first pass at an implementation of dominator tree construction so that we don't have to keep avoiding implementing algorithms that rely on having dominator information available. The algorithm used to construct the dominator tree is taken from "A Simple, Fast Dominance Algorithm" by Keith D. Cooper, Timothy J. Harvey, and Ken Kennedy. This is not the "best" algorithm in terms of asymptotic performance, but it is among the simplest algorithms for computing a dominator tree that still outperforms naive iterative set-based methods. The actual data structure and API for the dominator tree has a bit of "cleverness" in it to try to make the common queries reasonably fast (e.g., you can check whether A dominates B in constant time). My hope is that even if we implement a more advanced algorithm for constructing the dominator tree, we can retain compatibility with passes that might make use of this API. Because no code is currently using this logic, I have done only minimal testing by stepping through this code and validating the results on paper for some very small CFGs. More serious testing/debugging may need to wait until we have an optimization pass that needs the dominator tree we compute here. One open question I have is how best to introduce traditional unit testing into Slang, since this is an example of code that would benefit greatly from being unit tested. 04 May 2018, 00:40:26 UTC
00afea1 Tim Foley 03 May 2018, 23:34:49 UTC Pass through original names for most declarations (#547) The basic idea here is that when lowering to the IR, the front-end will attach a "name hint" to the IR instruction(s) that represent a given declaration, and then the passes that work on the IR will try to preserve and propagate those names, and then finally the emit logic will use them in place of mangled or unique names when available. This change does *not* try to deal with the issues that arise when we try to use those variable names in the output without any modification (e.g., handling cases where they might clash with keywords or builtins in the target language). Instead, it tries to establish baseline behavior for propagating through names, so that a later change can concentrate on the issue of using those names exactly when it is legal to do so. In order to avoid issues around the name "hints" causing problems we take two main steps: 1. We "scrub" each name to reduce it down to the allowed set of identifier characters in C-like languages, and then ensure that it doesn't do things that would be illegal in some downstream languages (e.g., consecutive underscores are not allowed in GLSL) or could clash with Slang's mangled names. This process isn't guaranteed to give distinct results for distinct inputs (it isn't a mangling scheme, after all). 2. We generate a unique ID for each occurence of a given name and always use that as a suffix. This means that even if a name happens to overlap with a keyword (if you somehow have a variable named `do`), we will still add a suffix that makes it not a problem (we'd output `do_0` which is fine). The logic for generating these names is mostly straightforward. For simple variables, we use their given name directly, while for other declarations we try to form a name that includes their parent declaration (e.g. `SomeType.someMethod`). Various IR passes need to propagate or preserve this information. The most interesting is type legalization, when we take a variable with an aggregate type and split some of the fields out into their own variables. In that case we generate "dotted" names like `someVar.someTexture` and rely on the emit logic to turn that into `someVar_someTexture`. During SSA generation, if we are promoting a variable to SSA temporaries, we will try to propagate the name of the variable over to the temporaries (unless they already have a name from some other place). The same applies to block parameters ("phi nodes"). Many of the test changes need their expected output to be updated for this change. Luckily in most cases the output has gotten easier to understand. 03 May 2018, 23:34:49 UTC
f847294 jsmall-nvidia 03 May 2018, 22:42:13 UTC Added Surface type - as a simple value type to hold a 2d collection of pixels. (#548) Added PngSerializeUtil allows currently for just writing Surface of RGBA format. Removes dependency on stbi_image except for in PngSerializeUtil. Removed use of gWindowWidth/Height globals - pass the height into initialize or Renderer. 03 May 2018, 22:42:13 UTC
c216f00 jsmall-nvidia 03 May 2018, 21:17:05 UTC Fixes based on review of vulkan-first-render PR #545 (#546) 03 May 2018, 21:17:05 UTC
367f3a7 jsmall-nvidia 03 May 2018, 18:25:13 UTC Feature/vulkan first render (#545) * First pass at InputLayout for Vulkan Add support for RGBA_Float32 * Use VulkanModule and VulkanApi to handle accessing Vulkan types. * First pass at Vulkan swap chain/Device queue. * Added VulkanUtil for generic function functions. * Move more functionality to VulkanApi and VulkanUtil. Make Buffer able to initialize itself. * More tidy up around VulkanDeviceQueue * First pass use of VulkanDeviceQueue in VkRenderer * First pass use of VulkanSwapChain on VkRenderer * Added depth formats. Binding for constant and vertex buffers for Vulkan. * Setting up VkImageView on backbuffers. * First pass support for setting up vkRenderPass. * Fixes to work around Vulkan swap chain/verification issues. * Added support for Pipeline and a pipeline cache. * Working without waiting - because use of pipeline cache. * Added support for VkFramebuffer in Vulkan. * First pass at creating Vulkan graphics pipeline. * More efforts to get Vulkan to render. * Small improvement for checking of Binding flags. * Removed setConstantBuffers from the Renderer interface - so that all resource binding takes place through the BindingState. To make this work required a 'hack' in render-test main.cpp - so that the constant buffer binding that is needed in some tests is only added when it doesn't clash. * RendererID -> unified into RendererType. Added getRendererType to Renderer interface. Added ProjectionStyle, and function to get from RendererType. Added getIdentityProjection to RendererUtil - to get projection that is the 'identity' - but hits the same pixels for all projection styles. * Fix build problem on Win32 on Vulkan where should use VK_NULL_HANDLE. * Improve naming, comments. Remove dead code. * Remove unwanted comment. 03 May 2018, 18:25:13 UTC
7893549 Yong He 03 May 2018, 03:01:48 UTC Merge pull request #543 from csyonghe/master Speedup type checking using cached overload resolution results. 03 May 2018, 03:01:48 UTC
235d6aa Yong He 03 May 2018, 00:39:10 UTC Merge branch 'master' into master 03 May 2018, 00:39:10 UTC
0399d99 Yong He 02 May 2018, 19:11:04 UTC Speedup type checking using cached overload resolution results. This change adds caches to built-in operator overload resolution and type coersion to avoid running these time-consuming operations every time. - Adds `TypeCheckingCache` type, which is defined in check.cpp, that contains two dictionaries for the cached results of `ResolveInvoke` and `CanCoerce` calls. - Add `destroyTypeCheckingCache` and `getTypeCheckingCache` methods to `Session` class to reuse these cached results over the entire session. 02 May 2018, 22:20:32 UTC
384df86 Tim Foley 02 May 2018, 21:44:13 UTC Add support for "swizzled stores" (#544) This was a known issue in our IR representation, which was now biting a user. The basic problem is that in code like the following: ```hlsl RWStructureBuffer<float4> buffer; ... buffer[index].xz = value; ``` we ideally want to be able to reproduce the original HLSL code exactly, but that requires directly encoding the way that this code writes to two elements of a vector, but not the others. The currently lowering strategy we had produced IR something like: ```hlsl float4 tmp = buffer[index]; tmp.xz = value; buffer[index] = tmp; ``` That transformation might seem valid, but it has some big problems: * It generates UAV reads that are not needed, which could impact performance * It performs read-modify-write operations on memory that the programmer didn't explicitly write, which could create data races The fix here is somewhat obvious: if the "base" of a swizzle operation on a left-hand side resolves to a pointer in our IR, then we can output a "swizzled store" instead of the read-modify-write dance. We currently keep the read-modify-write around since it is potentially needed as a fallback in the general case. Along the way I also tried to make sure that we handle the case where we have a swizzle of a swizzle on the left-hand side: ```hlsl buffer[index].xz.y = value; ``` That code should behave the same as `buffer[index].z = value`. I am currently detecting and cleaning up this logic in the lowering path for `SwizzleExpr`, because that is the only place in the lowering logic that "swizzled l-values" currently get created. 02 May 2018, 21:44:13 UTC
60bcc68 Tim Foley 02 May 2018, 18:40:09 UTC Add support for explicit register space bindings (#542) This change adds support for specifying explicit register spaces, like: ```hlsl // Bind to texture register #2 in space #1 Texture2D t : register(t2, space1); ``` I added a test case to confirm that the register space is properly propagated through the Slang reflection API. This change also adds proper error messages for some error/unsupported cases that weren't being diagnosed: * Specifying a completely bogus register "class" (e.g., `register(bad99)`) * Failing to specify a register index (`register(u)`) * Specifying a component mask (`register(t0.x)`) * Using `packoffset` bindings I added test cases to cover all of these, as well as the new errors around support for register `space` bindings. In order to get the existing tests to pass, I had to remove explicit `packoffset` bindings from some DXSDK test shaders. None of these `packoffset` bindings were semantically significant (they matched what the compiler would do anyway, for both Slang and the standard HLSL compiler). Removing them is required for Slang now that we give an explicit error about our lack of `packoffset` support. In a future change we might add logic to either detect semantically insignificant `packoffset`s, or to just go ahead and support them properly (as a general feature on `struct` types). 02 May 2018, 18:40:09 UTC
d3c1c8b Tim Foley 02 May 2018, 13:45:35 UTC Fix emit logic when "terminators" occur in the middle of a block (#540) Fixes #527 There were a few problem cases for the IR emit logic. The most obvious, which came up in #527 is that a function body with multiple `return` statements would generate invalid code: ```hlsl int foo() { return 1; int x = 2; return x; } ``` In that case the IR for `foo` would have a single block that has two `return` instructions, which is invalid. Another case that seems to be arising more often, but that had less obvious consequences was when one arm of an `if` statement ends in a `return`: ```hlsl if(a) { return b; } else { int c = 0; } int d = 0; ``` In that case, the `return` instruction for `return b` would be followed by a branch to the end of the `if` (the `int d = 0;` line), because that would be the normal control flow without the early `return`. The fix implemented here is to have the IR lowering logic be a bit more careful on two fronts: 1. When emitting a branch, check if the block we are emitting into has already been terminated, and if so just don't emit the branch (since we are logically at an unreachable point in the CFG. 2. Whenever we are about to emit code for a (non-empty) statement, ensure that the current block being build is unterminated. If the current block is terminated, then start a new one. Case (2) will only matter when there is unreachable code (e.g., in the function `foo()`, the declaration of `x` and the second `return` can never be reached), so I added a warning in that case, and included a test case that triggers the new warning (with a function like `foo()` above). 02 May 2018, 13:45:35 UTC
d90d73a Tim Foley 02 May 2018, 00:26:20 UTC Diagnose attempts to write to fields in methods (#530) * Diagnose attempts to write to fields in methods Work on #529 This helps to avoid the case where a Slang user writes a struct with helpful `setter` methods, and finds that it doesn't work as expected because the `this` parameter is currently handled like an `in` parameter (passed by value, but mutable in the callee). Fixing this issue actually involved making a more broad fix to how l-value-ness is propagated. The existing checking logic was assuming that l-value-ness is just a property of a particular member declaration (e.g., a field is either mutable or not), and didn't take into account whether the "base expression" was mutable. This change fixes that oversight, which might lead to additional errors being issued if we aren't correctly making things mutable when we should. A `ThisExpr` was already immutable by default, so that part didn't actually need to change. Just propagating its immutability through was enough. As an additional assistance to users, I have added an extra diagnostic that triggers when a "destination of assignment is not an l-value" error occurs and the left-hand-side expression seems to be based on `this` (whether implicitly or explicitly). This will ideally help users to understand that the "setter" idiom is not yet supported. * Fixed setRadius typo 02 May 2018, 00:26:20 UTC
809f520 Tim Foley 01 May 2018, 21:32:03 UTC Cleanups (#539) * Cleanup: remove unused files from project * Cleanup: move IRModule forward declaration into correct namespace 01 May 2018, 21:32:03 UTC
3ace6e7 Jeremie St-Amand 29 April 2018, 01:21:22 UTC Remove unused local variable in vm.cpp (#533) Unused local variable prevents compiling when warnings are treated as errors 29 April 2018, 01:21:22 UTC
b54629f Tim Foley 25 April 2018, 18:58:06 UTC Fix for global generic parameter substitution (#512) The problem here arises when multiple entry points are compiled in one pass. Each entry point has its own arguments for global generic parameters, and leads to us emitting a `bindGlobalGenericParameter(p, val)`. But once the first entry point's substitutions are applied, the second entry point's code gives `bindGlobalGenericParameter(val, val)` and the compiler crashes (in debug builds) because `val` is not a global generic parameter. This change just applies a quick fix. If we see `bindGlobalGenericParameter(x,y)` during specialization, and `x` is not a global generic parameter, then we skip it. The right long-term fix is to change the compiler's representation of global generic arguments so that they live on a `CompileRequest` instead of an `EntryPointRequest`. That is a more significant change (with impact on the public API), so I'm inclined to leave it as a cleanup for another day (given that no customers are using global generic parameters today). 25 April 2018, 18:58:06 UTC
9a7849d Tim Foley 23 April 2018, 17:37:56 UTC Improve SSA promotion for arrays and structs (#521) * Improve SSA promotion for arrays and structs Fixes #518 The existing SSA pass would only handle `load(v)` and `store(v,...)` where `v` is the variable instruction, and would bail out if `v` was used as an operand in any other fashion. The new pass adds support for `load(ac)` where `ac` is an "access chain" with a gramar like: ac :: v | getElementPtr(ac, ...) | getFieldAddress(ac, ...) What this means in practical terms is that we can promote a local variable of array or structure type to an SSA temporary even if there are loads of individual elements/fields, as along as any *assignment* to the variable assigns the whole thing. I've added a test case to confirm that this change fixes passing of arrays as function parameters for Vulkan. * Fixup: disable test on Vulkan because render-test isn't ready This is a fix for Vulkan, but I don't think our testing setup is ready for it. * Fixup: error in unreachable return case, caught by clang * Fixups based on testing These are fixes found when testing the original changes against the user code that originated the bug report. * `emit.cpp`: Make sure to handle array-of-texture types when deciding whether to declare a temporary as a local variable in GLSL output * `ir-legalize-types.cpp`: Make a not of a source of validation failures that we need to clean up sooner or later (just not in scope for this bug fix change). * `ir-ssa.cpp`: * When checking if something is an access chain with a promotable var at the end, make sure the recursive case recurses into the "access chain" logic instead of the leaf case * Add some assertions to guard the assumption that any access chain we apply has been scheduled for removal * Correctly emit an element *extract* instead of getting an element *address* when promoting an element access into an array being promoted * Eliminate a wrapper routine that was setting up an `IRBuilder` and use the one from the block being processed in the SSA pass (since it was set up for stuff just like this) * `ir-validate.cpp` * Add a hack to avoid validation failures when running IR validation on the stdlib code. This case triggers for an initializer (`__init`) declaration inside an interface, since the logical "return type" is the interface type itself, which has no representation at the IR level and thus yields a null result type in a `FuncType` instruction. 23 April 2018, 17:37:56 UTC
627de1c Tim Foley 23 April 2018, 17:37:24 UTC Fix successor computation for `switch` instruction (#520) Fixes #519 The code was leaving out the `default` label from the successor list, which would break any passes that require an accurate CFG (with the big one right now being the SSA-formation pass). 23 April 2018, 17:37:24 UTC
163d306 Tim Foley 21 April 2018, 00:54:39 UTC Better diagnostics when compilation is aborted (#517) * Improve messages when compilation is aborted. Make sure to include the information from any `Slang::Exception` that was thrown, so that the poor user can at least point us at our own message string from an assertion failure. This doesn't provide them line-number information in their code or the Slang codebase, so there is still work to be done in making the compiler more friendly about this stuff. * When aborting compilation, try to note what source location we were working on This is handled by having exception handlers on the stack at key bottleneck points in semantic checking and IR generation, which can then emit a diagnostic to note what we were working on when things failed. This is not intended to be an indiciation to the user that their code is at fault for a compiler crash (it is always our fault), but might give them a chance to work around whatever bug is blocking them. 21 April 2018, 00:54:39 UTC
2f782d4 Tim Foley 20 April 2018, 23:56:33 UTC Diagnose use of an implicit cast as an argument for an `out` parameter (#516) Work on #499 Two big fixes here: * The logic for checking constraints on `out` arguments wasn't actually triggering because it relied on function parameters being given an `OutType` if they are marked `out`, but the code wasn't actually doing that. Fixing the computation of types for functions resolved that issue. * Next, I added a specific diagnostic to follow up the "expected an l-value" error to let the user know that their argument was implicitly converted, and that is why it doesn't count as an l-value in Slang's rules. I've added a test case to ensure that we retain this diagnostic until we can do a true fix for the issue. The right long-term fix is to have an AST representation of all the implicit casts involved (e.g., in both directions for an `inout` parameter), and then have the IR generate explicit code for the conversions in each direction (the `LoweredVal` representation can handle this sort of thing). 20 April 2018, 23:56:33 UTC
c73ccbc jsmall-nvidia 20 April 2018, 18:59:17 UTC Fixes/improvements based on feature/render-binding-resource (#511) * Dx12 rendering works in test framework. * Turn on dx12 render tests. * Split out functions for construction or Renderer types into ShaderRendererUtil. Removed the serialization of buffers code into test-render * Improvements in documentation and typename in BindingState types. RegisterSet -> CompactBindIndexSlice RegisterList -> BindIndexSlice RegisterDesc -> ShaderBindSet * Fix debug build break. 20 April 2018, 18:59:17 UTC
4c751df jsmall-nvidia 19 April 2018, 21:47:04 UTC Separation of Binding/Resource construction on Renderer interface (#508) * Dx12 rendering works in test framework. * Turn on dx12 render tests. * First pass at Resource and TextureResource/BufferResource types. * Fix bug in Dx11 impl for BufferResource. * Dx12 supports TextureResource and binds using TextureResource type, and all tests pass. * Added TextureBuffer::Size type to make handling mips a little simpler. * Small improvements to Dx12 constant buffer binding Removed k prefix on an enum * First pass impl of dx11 createTextureResource Added setDefaults to TextureResource::Desc and BufferResource::Desc to simplify setup accessFlags -> cpuAccessFlags desc -> srcDesc * Split out generateTextureResource - can produce the texture using createTextureResource on the Renderer. * Added support for read mapping to Dx11 accessFlags -> cpuAccessFlags First pass at using TextureResource/BufferResource on Dx11 Some tests fail with this checkin * TextureResource working on all tests on dx11. * Construct ResourceBuffers on Dx11 and Dx12 using utility function createInputBufferResource. * First pass at OpenGl TextureResource * Small fixes to dx12 and dx11 setup. Gl working working using BufferResource and TextureResource * Tidy up around the compareSampler - looks like the previous test was incorrect. * Small documentation /naming improvements. * Fix some more small documentation issues. * First pass testing out construction of binding resources external to Renderer implementation. * Moved some BindingState::Desc types to BindingState to make easier to use. * First pass of binding using BindingState::Desc for Dx11. * First pass at binding with dx12. * Fixed issues around separating dx12 binding from ShaderInputLayout * First pass at OpenGl state binding. * BindingState::Desc::Binding::Type -> BindingType * Use Buffer to manage life of vk resources. Construction of buffers handled by createBufferResource (BindingState doesn't have specialized logic) * Remove InputLayout types from binding so can create a binding independent of it. * Added upload buffer to BufferResource - could be used for write mapping. * m_samplers -> m_samplerDescs. First pass at Vk binding with BindingState::Desc. Small tidy/doc improvements. * First pass with binding all taking place through BindingState::Desc. All tests pass. * Removed support for creating BindingState from ShaderInputLayout * Remove serializeOutput from Renderer interface and all implementations. Implement map/unmap on vulkan Implement serializeBindingOutput which uses map/unmap and BindingState::Desc to write result. * Make implementation of BindingState use the BindingState::Desc for much of state - only hold api specific in BindingDetail per implementation. * Use Glsl binding on vulkan (was using hlsl). * BindingState::Desc::Binding -> BindingState::Binding. Made possible by impls using 'BindingDetail' for their specific needs. * Fix compile problems on win32. * Fix a typo in name createBindingSetDesc -> createBindingStateDesc 19 April 2018, 21:47:04 UTC
cbedf01 Tim Foley 19 April 2018, 19:22:20 UTC Fix GS cross-compilation after IR type system change (#507) The cross-compilation logic for geometry shaders would look through the user's entry point for calls like `someStream.Emit<X>(val)` and turn that into `outputGlobals = val; EmitVertex();`. It was recognizing the `Emit()` calls by looking at the callee in all `call` instructions and seeing if it was registered to lower to GLSL as `EmitVertex()`. The logic was try to look "through" `specialize` instructions (to deal with the `<X>` bit in the call above), but this wasn't updated for the new IR encoding where the first operand to a `specialize` is the generic being specialized, and not the function nested inside it. The fix here is to properly look through both `specialize` instructions and generics. This is kind of a gross operation and we've done things like it in a few places, so it might be something we try to extract into a utility function in the future. 19 April 2018, 19:22:20 UTC
163bf58 Tim Foley 19 April 2018, 16:45:40 UTC Add type legalization support for "field extract" op (#501) The code was handling the "get field address" opcode (which takes a pointer to a struct and returns a pointer to a field), but didn't have a case for values. This was just an oversight. 19 April 2018, 16:45:40 UTC
c68c6fa Tim Foley 19 April 2018, 16:34:54 UTC Fix up DXR type emission from IR type system (#498) * There was a simple typo where we were emitting `RaytracingAccelerationStructureType` instead of `RaytracingAccelerationStructure` * The IR lowering logic was failing to handle types with an `__intrinsic_type` modifier (which maps them to a single IR opcode) that weren't in one of the various special cases. I added a catch-all case to the handling of `DeclRefType`. This notably affected the `RayDesc` type. * Even if we lower `RayDesc` to an intrinsic type, we still need to lower its *fields* too, and these were getting emitted with mangled names (as would happen for any user-defined fields). The solution I implemented was to allow for fields to have `__target_intrinsic` modifiers in the stdlib, to specify the un-mangled name they should use on each target. I'm not 100% happy with this solution, because it seems odd to have `RayDesc` be an intrinsic type, but then to also have field keys used in `getField` instructions as if it were an ordinary `struct`. It seems like a better solution would be to have it lower to an IR `struct`, just with an appropriate modifier. 19 April 2018, 16:34:54 UTC
17fa424 Tim Foley 19 April 2018, 00:22:44 UTC Fix output of `groupshared` with IR type system (#492) The basic problem was that the lowering logic was constructing (more or less) `Ptr<@GroupShared X>` instead of `@GroupShared Ptr<X>`. There were also problems with passes not propagating through rates that should have been (e.g., legalization). I've added a test case to actually validate `groupshared` support. 19 April 2018, 00:22:44 UTC
c3a27c0 Tim Foley 18 April 2018, 21:14:26 UTC Fix up name mangling/unmangling for extensions (#493) * Fix up name mangling/unmangling for extensions This is required for the unmangling we do on some builtin function names. The work here is mostly just a band-aid, and a more comprehensive pass over the name mangling/unmangling code is required to make any of this robust. * fixup: UNREACHABLE_RETURN argument 18 April 2018, 21:14:26 UTC
0450ca6 Tim Foley 18 April 2018, 19:57:34 UTC Fix some logic around legalization of sampler types (#496) The main error here was checking for `IRSamplerType` instead of `IRSamplerTypeBase`, which means the relevant logic only triggered for the `SamplerState` type and not the `SamplerComparisonState` type. The two affected places were type legalization (so that comparison samplers in `struct` types weren't being hoisted out) and the emit logic when deciding whether to introduce local temporaries (so we were emitting temporaries for comparison samplers, leading to GLSL errors). 18 April 2018, 19:57:34 UTC
00389a1 jsmall-nvidia 17 April 2018, 20:59:03 UTC Feature/renderer binding (#489) * Dx12 rendering works in test framework. * Turn on dx12 render tests. * First pass at Resource and TextureResource/BufferResource types. * Fix bug in Dx11 impl for BufferResource. * Dx12 supports TextureResource and binds using TextureResource type, and all tests pass. * Added TextureBuffer::Size type to make handling mips a little simpler. * Small improvements to Dx12 constant buffer binding Removed k prefix on an enum * First pass impl of dx11 createTextureResource Added setDefaults to TextureResource::Desc and BufferResource::Desc to simplify setup accessFlags -> cpuAccessFlags desc -> srcDesc * Split out generateTextureResource - can produce the texture using createTextureResource on the Renderer. * Added support for read mapping to Dx11 accessFlags -> cpuAccessFlags First pass at using TextureResource/BufferResource on Dx11 Some tests fail with this checkin * TextureResource working on all tests on dx11. * Construct ResourceBuffers on Dx11 and Dx12 using utility function createInputBufferResource. * First pass at OpenGl TextureResource * Small fixes to dx12 and dx11 setup. Gl working working using BufferResource and TextureResource * Tidy up around the compareSampler - looks like the previous test was incorrect. * Small documentation /naming improvements. * Fix some more small documentation issues. 17 April 2018, 20:59:03 UTC
15bff91 Tim Foley 13 April 2018, 20:36:26 UTC Propagate diagnostics when imported module has errors (#485) A previous fix avoided crashes when an `import`ed module has errors by making the "failed to import" error a fatal one. Unfortunately, the code path that handles fatal errors was failing to copy diagnostic output from the sink over to the member variable on the `CompileRequest` that exposes the output through the API. This meant that API users lost all context on error messages in `import`ed code. This change fixes the immediate issue by plumbing through the error output, but doesn't fix the more fundamental issue: the front-end should not crash when an `import` fails, by any means. 13 April 2018, 20:36:26 UTC
021a492 Tim Foley 13 April 2018, 00:08:52 UTC Preprocessor cleanups (#484) * For a `#error` or `#warning`, read the rest of the line as raw text to include in the error message * When skipping tokens (e.g., in an `#ifdef`d out block), don't emit errors on invalid characters * TODO: we could clearly get more efficient and skip whole raw lines in the future * Fix an issue when a macro invocation that expands to nothing (zero tokens) is the last thing before a directive. The preprocessor was returning the `#` as an ordinary token, because it has already gone past its test for directives. 13 April 2018, 00:08:52 UTC
baf194e Tim Foley 11 April 2018, 23:18:29 UTC Introduce an IR-level type system (#481) * Introduce an IR-level type system Up to this point, the Slang IR has used the front-end type system to represent types in the IR. As a result (but ultimately more importantly) the IR representation of generics and specialization has used AST-level concepts embedded in the IR. For example, to express the specialization of `vector<T,N>` to a concrete type `float` for `T`, we needed an IR operation that could represent the specialization, with operands that somehow represented the type argument `float`. The whole thing was very complicated. The big idea of this change is to introduce a new representation in which types in the IR are just ordinary instructions, so that using them as operands makes sense. The hierarchy of IR types closely mirrors the AST-side hierarchy for now, and that will probably be something we should maintain going forward. In order to make these changes work, though, I also had to do major overhauls of things like the way substitutions are performed, how we check interface conformances, the way lookup through interface types is done, etc. etc. This is a big change, and unfortunately any attempt to summarize it in the commit message wouldn't do it justice. * Fix 64-bit build warning * Fix up some clang warnings/errors 11 April 2018, 23:18:29 UTC
6322983 jsmall-nvidia 11 April 2018, 19:55:44 UTC Feature/dx12 compute (#483) * Dx12 rendering works in test framework. * Turn on dx12 render tests. * Getting simpler dx12 compute tests to work. * With expected data in test - check for specialized and then for the default, so that multiple test can share the same expected data, but specialized cases can still be set. * Fixed construction and binding on dx12 textures. * Control which render apis used in test from command line. * Small aesthetic fixes in render-test/main.cpp. * Fix binding problem for uavs/srvs dx12. Previously tried to create srv/uav for StorageBuffers (like dx11 does), but the binding breaks as you can end up with two srvs using the same register. First pass at fixing problems with Texture creation for dx12 - assertions were hit with 3d or array textures. * Fixes to improve Dx12 setup shader resource views for cubemaps/arrays. * Fixed d3d12 textureSamplingTest - problem was that cubemap/array textures were not being uploaded correctly. * Changed the order of how binding of constant buffers (as just set on the Renderer) indexes. Previously they were given the lowest indices, but they clashed with the indices from the 'Binding'. Changing this means all tests run on d3d12. * Add code to allow use of warp (although not command line switchable yet). Fix problem setting up raw UAV - as identified by warp. * Added RenderApiUtil - which can detect if a render api is potentially available. * Moved render flag testing/parsing into RenderApiUtil. * Fix signed/unsigned warning. * Fixes around enums prefixed with k on the review of feature/dx12 compute branch. * Remove explicit -dx12 line in tests, as all can currently be generated from dx11 tests. 11 April 2018, 19:55:44 UTC
c4004b3 jsmall-nvidia 10 April 2018, 21:53:03 UTC Feature/dx12 compute (#482) * Dx12 rendering works in test framework. * Turn on dx12 render tests. * Getting simpler dx12 compute tests to work. * With expected data in test - check for specialized and then for the default, so that multiple test can share the same expected data, but specialized cases can still be set. * Fixed construction and binding on dx12 textures. * Control which render apis used in test from command line. * Small aesthetic fixes in render-test/main.cpp. * Fix binding problem for uavs/srvs dx12. Previously tried to create srv/uav for StorageBuffers (like dx11 does), but the binding breaks as you can end up with two srvs using the same register. First pass at fixing problems with Texture creation for dx12 - assertions were hit with 3d or array textures. * Fixes to improve Dx12 setup shader resource views for cubemaps/arrays. * Fixed d3d12 textureSamplingTest - problem was that cubemap/array textures were not being uploaded correctly. * Changed the order of how binding of constant buffers (as just set on the Renderer) indexes. Previously they were given the lowest indices, but they clashed with the indices from the 'Binding'. Changing this means all tests run on d3d12. * Add code to allow use of warp (although not command line switchable yet). Fix problem setting up raw UAV - as identified by warp. * Added RenderApiUtil - which can detect if a render api is potentially available. * Moved render flag testing/parsing into RenderApiUtil. * Fix signed/unsigned warning. * Fixes around enums prefixed with k on the review of feature/dx12 compute branch. 10 April 2018, 21:53:03 UTC
5298ccf Tim Foley 05 April 2018, 23:15:45 UTC Falcor fixes (#479) * Don't emit interpolation modifiers on GLSL fields The previous change that started passing through interpolation modifiers didn't account for the fact that the GLSL grammar doesn't allow interpolation modifiers on `struct` fields, so we shouldn't emit them in that case (and our legalization strategy for GLSL guarantees that varying input will never use a `struct` type anyway). * Try to handle SV_Position semantic on GS input HLSL allows `SV` semantics to be used for ordinary inter-stage dataflow in some cases (e.g., a VS can output `SV_Position` and it can then be read from a GS). GLSL allows similar things with `gl_Position`, but there are some wrinkles. One fix here is to correctly identify that `gl_FragCoord` should only be used as the translation of `SV_Position` for a fragment shader input (and not in the general case of *any* input). The other "fix" here is a kludge to handle the fact that the right translation for a GS input is not to an array called `gl_Position`, but to the syntax `gl_in[index].gl_Position` (array-of-structs style). I am doing this by attaching a custom decoration to the global variable we create for `gl_Position` and then intercepting it during code emit. I'm not proud of this, and would like to do something better given time. * Fix GLSL output for matrix-scalar multiplication The output logic was assuming that any use of `operator*` in the input code that yields a value of matrix type must be translated to a `matrixCompMult()` call in GLSL, but this should really only apply if both of the *operands* are matrices (not just based on the result type). As a result matrix-times-scalar operations were emitting a call to `matrixCompMult()` and GLSL was complaining because it won't implicitly promote scalars to matrices. 05 April 2018, 23:15:45 UTC
071566c jsmall-nvidia 04 April 2018, 21:23:49 UTC Switch on dx12 testing for remaining render tests. (#477) * Dx12 rendering works in test framework. * Turn on dx12 render tests. 04 April 2018, 21:23:49 UTC
357bce2 jsmall-nvidia 04 April 2018, 17:39:42 UTC Dx12 rendering works in test framework. (#476) 04 April 2018, 17:39:42 UTC
bb6045d Tim Foley 04 April 2018, 15:14:57 UTC Add an EditorConfig file (#474) Developers may need to install an appropriate plugin for their text editor. For example, Visual Studio 2015 users can install the plugin from `Tools -> Extensions and Updates...`. Visual Studio 2017 supports EditorConfig directly. The main goal of this PR is to allow VS users to more easily toggle between projects that default to spaces or tabs for indentation without having to change their VS options. One side effect, however, is that EditorConfig might get overzealous and replace tabs with spaces in any file that is edited, so that PRs may contain spurious changes for a bit. The best fix for this would be to check in some project-wide fixup changes for formatting. 04 April 2018, 15:14:57 UTC
7a8ed51 Tim Foley 04 April 2018, 14:18:33 UTC Pass AST interpolation modifiers through to codegen. (#475) This is a short-term fix, because we (1) don't have an IR-level representation of interpolation qualifiers, and (2) can't introduce one until *after* the IR-level type system is introduced (to be able to handle `struct` fields). The approach here is to find the AST-level declaration, either from layout information (in the case of an ordinary variable or function parameter), or from struct field information (because structs are being output from the AST form anyway). I've included a single end-to-end rendering test to confirm that we handle the `nointerpolation` modifier the same as HLSL. I also added the `noperspective` modifier, which seemed to be missing from our implementation. 04 April 2018, 14:18:33 UTC
3115ba7 jsmall-nvidia 03 April 2018, 16:25:51 UTC Fixes based on review of feature/dx12 PR. (#473) 03 April 2018, 16:25:51 UTC
499e258 Tim Foley 03 April 2018, 01:39:15 UTC Implement "operator comma" in IR codegen (#472) Fixes #471 03 April 2018, 01:39:15 UTC
38d5ef4 jsmall-nvidia 02 April 2018, 23:59:33 UTC Feature/dx12 (#469) * Fix signed/unsigned comparison warning. * Split out d3d functions that will work across dx11 and 12. * Improve slang-test/README.md around command line options. * Make Guid comparison honor alignment for comparisons, such that mechanism work on architectures that can only do aligned accesses. * Initial setup of D3D12 Renderer, with presentFrame and clearFrame. * More support for D3D12 * Added FreeList * Added D3D12CircularResourceHeap * First attempt at createBuffer * First pass at map/unmap. * First pass binding vertex/constant buffers, and setting up InputLayout. Note that memory is not kept in scope on binding yet. * First pass of D3DDescriptorHeap * Small tidy up in render-d3d11. Added D3DDescriptorHeap to project. * First pass at D3D12 bind state. * Fix typos in D3D12Resource * Tidy up Dx11 render binding a little to match more with Dx12 style. * First pass at Dx12 BindingState * Handling of the command list d3d12. Support for submitGpuWork and waitForGpu. * First attempt at Dx12 capture of backbuffer to file. * First attempt at D3D12 binding for graphics. * D3D12 setup viewport etc - does now render triangle in render0.hlsl. * First pass at support for compute on D3D12Renderer * Use spaces over tabs in D3DUtil * Tabs to spaces in D3D12DescriptorHeap * Convert tab->spaces on render-d3d12.cpp 02 April 2018, 23:59:33 UTC
5a02738 Tim Foley 02 April 2018, 22:08:21 UTC Update to top-of-tree glslang (2018-04-02). (#470) This is an attempt to alleviate some driver crashes caused by invalid SPIR-V. Because Slang drives `glslang` with GLSL source code, any invalid output is likely due to `glslang` bugs. I chose top-of-tree for `glslang` because it wasn't clear what their release process is. Hopefully we can go another year without having to update this dependency. The build setup we use for `glslang` had to change to account for one more `#define` that the code expects to have passed in externally. 02 April 2018, 22:08:21 UTC
bd66d4f Tim Foley 30 March 2018, 23:53:07 UTC Fix several issues discovered by Falcor (#467) Fixes #466 Most of these are Vulkan-related regressions. * Kludge the definition of `GroupMemoryBarrierWithGroupSync()` for GLSL so that it works around parentheses that the emit logic now introduces. * Don't emit `static` for global constants when targetting GLSL * Emit the `flat` modifier for varying input/output with integer type, when targetting GLSL * Avoid checking parameter default-value expressions more than once, because this can crash when the checking introduces syntax that is not expected to appear in the input AST 30 March 2018, 23:53:07 UTC
87c50cf Tim Foley 29 March 2018, 22:47:58 UTC Avoid crash when bad argument given to [instance(...)] attribute (#464) Fixes #463 Some of the attributes were failing to check for a `null` result from `checkConstantIntVal`, and so they crashed when a bad expression was used in an attribute. The particular way this had been triggered was that a user put an HLSL geometry shader in the same file with other code, using an entry point like: ```hlsl [instance(COUNT)] void myGeometryShader(...) {...} ``` They then defined `COUNT` as a preprocessor macro when compiling using the GS, but left it undefined otherwise. The result was that the argument to the `instance` attribute would fail to type check, and thus wouldn't count as a constant integer value, so that `checkConstantIntVal` returns `null` and results in the crash. The workaround for the user is to always define `COUNT`, even when not compiling the GS. The fix in the compiler is to guard against `null` in these cases and bail out of attribute checking. I also implemented logic so that `CheckIntegerConstantExpression` (which is invoked by `checkConstantIntVal`) will not produce an additional error message if the underlying expression failed to type check. In this casem the user will get an `undefined identifier: COUNT` error message, and we don't need to waste their time by also telling them that this isn't a compile-time constant expression. 29 March 2018, 22:47:58 UTC
b61371d Tim Foley 29 March 2018, 20:40:55 UTC Change uses of "spire" to "slang" (#461) Fixes #350 When the Slang project forked off from the Spire research effort, we renamed things as we went, but many cases seem to have slipped through the cracks. The two biggest diffs here are: - The `hello` example program was incorrectly talking about what was in the shader file (Slang no longer supports the "module" or "pipeline" constructs from Spire), and so it wasn't just a simple rename. - The files under `tests/bindings` were mistakenly using `__SPIRE__` as a preprocessor guard, which means that they weren't actually testing what they meant to. Luckily, it looks like the relevant functionality didn't regress while these tests were unintentionally deactivated. 29 March 2018, 20:40:55 UTC
8c50f9f Tim Foley 29 March 2018, 18:57:57 UTC Make IR-based output code cleaner (#458) The big change here is that rather than trying to reproduce the exact line number and indentation of names as they appeared in the original code (which had been appropriate for the AST-to-AST translation strategy), we now emit code from the IR using a simple "pretty-printing" strategy, where indentation is determined by nesting. Along the way I deleted a bunch of dead code in `emit.cpp` that was handling emit from AST declarations/statements/expressions. I probably should have pulled that out into its own change, but doing that now would be tricky. This change also makes it so that we do *not* emit `#line` directives by default. Asking for `#line` directives in the output will probably become part of a Slang "debug mode" that tries to make the output code suitable for step-through debugging. 29 March 2018, 18:57:57 UTC
b4c4dc9 Tim Foley 29 March 2018, 17:39:21 UTC Add support for default parameter values in IR codegen (#459) Fixes #61 When lowering from AST to IR, if a call site doesn't supply an argument expression for each of the parameters to the callee, then use the default value expressions (stored as the "initializer" of the parameter decl) for each omitted parameter. This relies on the front-end to have already checked the call site for validity. Along the way I also cleaned up some of the checking of parameter declarations so that it is more like the checking of ordinary variable declarations (although the code is not yet shared). I also cleaned out some dead cases in the lowering logic for when we don't actually have a declaration available for a callee (these would only matter if we supported functions as first-class values). I added a simple test case to confirm that call sites both with and without the optional parameter work as expected. The strategy in this change is extremely simplistic, and might only be appropriate for default parameter value expressions that are compile-time constants (which should be the 99% case). This may require a major overhaul if we decide to handle default parameter values differently (e.g., by generating extra functions to ensure that the separate compilation story is what we want). Another issue that could change a lot of this logic would be if we start to support by-name parameters at call sites, since we could no longer assume that the argument and parameter lists align one-to-one (with the argument list possibly being shorter). Any work to add more flexible argument passing conventions would need to build a suitable structure to map from arguments to parameters, or vice-versa. 29 March 2018, 17:39:21 UTC
184dc5c Tim Foley 28 March 2018, 15:17:48 UTC Merge from v0.9.15 (#460) * Fix bug when subscripting a type that must be split (#396) The logic was creating a `PairPseudoExpr` as part of a subscript (`operator[]`) operation, but neglecting to fill in its `pairInfo` field, which led to a null-pointer crash further along. * Allow writes to UAV textures (#416) Work on #415 This issue is already fixed in the `v0.10.*` line, but I'm back-porting the fix to `v0.9.*`. The issue here was that the stdlib declarations for texture types were only including the `get` accessor for subscript operations, even if the texture was write-able. I've also included the fixes for other subscript accessors in the stdlib (notably that `OutputPatch<T>` is readable, but not writable, despite what the name seems to imply). * Fix infinite loop in semantic parsing (#424) The code for parsing semantics was looking for a fixed set of tokens to terminate a semantic list, rather than assuming that whenever you don't see a `:` ahead, you probably are done with semantics. This meant that you could get into an infinite loop just with simple mistakes like leaving out a `;`. This change fixes the parser to note infinite loop in this case, and adds a test case to verify the fix. * Expose HLSL `shared` modifier through reflection. (#436) This is a request from Falcor, because the `shared` modifier can be used as a hint to optimize the grouping of parameters for binding. The intention is that `shared` marks shader parameters (including parameter blocks) that will us the same values across many draw calls (e.g., per-frame data, as opposed to per-model or per-instance). The mechanism I'm using here is to provide a general reflection API for exposing the `Modifier`s already attached to declarations. While the only modifier exposed is `shared`, and the only modifier information being exposed is presence/absence, this interface could be extended down the line. 28 March 2018, 15:17:48 UTC
17b66ef Tim Foley 27 March 2018, 00:44:50 UTC Unify all generic parameters, even if some mismatch (#454) * Fix decl-ref printing to handling NULL pointers If the underlying decl, or its name is NULL, then use an empty string for the declaration name. This issue was found when debugging, but could bite non-debug cases too, if we ever try to print something like a generic type constraint, which has no name. * Unify all generic parameters, even if some mismatch Fixes #449 The front end tries to infer the right generic arguments to use at a call site using a sloppily implemented "unification" approach. The basic idea is that if you pass a `vector<float,3>` into a function that operates ona `vector<T,N>` where `T` and `N` are generic paameters, then the unification will try to unify `vector<float,3>` with `vector<T,N>` which will lead to it recursively unifying `float` with `T` and `3` with `N`, at which point we have viable values to substitute in for those parameters. Where the existing approach is maybe not quite right is in how it handles obvious unification failures. So if we ask the code to unify, say, `float` with `uint`, it will bail out immediately because those can't be unified. This sounds right superficially, but in some cases with might be calling a function that takes a `vector<float,N>` and passing a `vector<uint,3>` and we'd like to at least get far enough along with unification to see that `N` should be `3` so that the front end can maybe decide to call the function anyway, with some amount of implicit conversion. Over time I've had to modify a lot of the "unification" logic so that it doesn't treat the obvious failures as a hard stop, and instead just returns the failure as a boolean status, but keeps on trying to unify things even after such a failure. When doing unification as part of inference for generic arguments, there will usually be subsequent steps (e.g., type conversions for function aguments) that will catch the type errors that arise. This specific change is to make is so that when unifying the substitutions for a generic decl-ref, we try to unify all the pair-wise arguments, and don't bail out on the first mismatch (so that the `float`-vs-`uint` failure above doesn't lead to us skipping the `3` and `N` pairing). The one case we need to watch out for in all of this is when unification is used to check if an `extension` declaration (which might be generic) is actually application to a concrete type. In that case we obviously don't want an extension for `vector<float,N>` to apply to `vector<uint,3>`, so it is important that the extension case check the return status from the unification logic (*or* in the future, it could just confirm that the substituted type is equivalent to the original as a post-process...). I've added a test case that reproduces the original failure that surfaced the bug. * fixup: add expected test output 27 March 2018, 00:44:50 UTC
6d400b6 jsmall-nvidia 26 March 2018, 20:10:43 UTC Fix signed/unsigned comparison warning. (#455) 26 March 2018, 20:10:43 UTC
74bf38b jsmall-nvidia 26 March 2018, 19:34:01 UTC Renderer resource mangement for render-test (#453) * First pass at resource based renderer using RefObject. * Correct handling of array of buffer pointers to Dx11. * Fix bug with setting viewOut incorrectly in createInputTexture. * More support for allowing com like interfaces. * Added and tidied Slang::Result - adding interface specific results * Guid added comparison support, and made base interface IComUnknown - with lowerCamel methods 26 March 2018, 19:34:01 UTC
5000d27 jsmall-nvidia 22 March 2018, 22:08:19 UTC Tidy up of Renderer (#452) * Fixed some small typos in api-users-guide.md * Fix some small typos in slang-test/main.cpp, render-test/render-d3d11.cpp * Remove exit() calls from test code. Added Slang::Result, which works in the same way as COM HRESULT. * FIx bug introduced when moving to Slang::Result - handling E_INVALIDARG on Dx11. * Fix the testing of feature levels on Dx11 renderer. * First attempt at README.md for slang-test. * Tidied up the slang-test README.md file. * Fix some small typos in tools/slang-test/main.cpp * Fix spaces -> tabs problems. Fix some small types. * Refactor Renderer implementations such that: * Class definition does not contain long implementation/s * Removed unused globals * Ordered implementation after class definition * Made renderer specific classes child classes, and use Impl postfix to differentiate * Converted tabs into spaces * First pass at Slang::ComPtr. Added slang-defines.h which sets up some fairly commonly used defines such as SLANG_FORCE_INLINE, compiler detection, os detection, and some other cross platform features. * * Fixed bug in vk renderer - where features structure not initialized on hkCreateDevice * Make member variables in Renderer implementations use prefix * Updated test README.md to document that free parameter can control what test is run * * changed setClearColor to take an array of 4 floats to make API clearer on usage * mix of type usage style - defaulted to more conventional style * * Fixed swapWith * Use SLANG_FORCE_INLINE * Don't bother initializing List data when type is POD * Added convenience macro for Result handling SLANG_RETURN_NULL_ON_ERROR 22 March 2018, 22:08:19 UTC
5e720e7 Tim Foley 22 March 2018, 18:21:45 UTC Add support for DirectX Raytracing (DXR) (#451) * Add support for DirectX Raytracing (DXR) This is an initial pass to add support to Slang for the shader stages introduced by DirectX Raytracing (DXR). * Add declarations for DXR intrinsic types and functions to the Slang standard library. The way our compilation works, these will then get propagated through the IR as intrinsics and get spit back out again as-is during HLSL code emission. * Declare the DXR-related stages. This is the main work that affects the compiler's C++ implementation rather than being something we can add via the standard library today. * Switch around the encoding of the `Profile` type so that the stage is in the low bits, allowing API users to pass an ordinary `SlangStage` to operations that expect a `SlangProfileID`. - This represents a direction I'd like to push in long term, where the user specifies stage and "feature level" separately rather than using composite profiles like `vs_6_0`. The introduction of these new stages seems like a good point to try and make a clean break here and not introduce, e.g., `rgs_6_1` for ray generatin shaders. * Upgrade "effective profile" computation so that it advances the required version based on the specified stage (e.g., DXR stages seem to require at least shader model 6.1). - This is a bit of a kludge overall, but ideally we don't want a typical user to have to think about "feature level" stuff much at all. The ideal workflow is that they just hand us a source file and we work out entry points and their required feature levels in the compiler (and let the user query it when we are done). Until we implement that for real, stopgaps like this are required. Overall these are relatively small changes for supporting some major new API behavior. Slang's design helps out here, by allowing a lot of things to be specified in the stdlib (including generic intrinsic functions), but some of this is also owed to the DXIL-influenced design of DXR - e.g., the use of global functions in place of `SV_*` semantics. * fixup: typos * Fixup: use `pixel` instead of `fragment` as primary stage name This is to match HLSL conventions when generating output code, even if the Slang project officially favors the more correct term "fragment shader." 22 March 2018, 18:21:45 UTC
d421988 jsmall-nvidia 21 March 2018, 18:28:43 UTC First pass impls on ComPtr and reorganise Renderer (#450) * Fixed some small typos in api-users-guide.md * Fix some small typos in slang-test/main.cpp, render-test/render-d3d11.cpp * Remove exit() calls from test code. Added Slang::Result, which works in the same way as COM HRESULT. * FIx bug introduced when moving to Slang::Result - handling E_INVALIDARG on Dx11. * Fix the testing of feature levels on Dx11 renderer. * First attempt at README.md for slang-test. * Tidied up the slang-test README.md file. * Fix some small typos in tools/slang-test/main.cpp * Fix spaces -> tabs problems. Fix some small types. * Refactor Renderer implementations such that: * Class definition does not contain long implementation/s * Removed unused globals * Ordered implementation after class definition * Made renderer specific classes child classes, and use Impl postfix to differentiate * Converted tabs into spaces * First pass at Slang::ComPtr. Added slang-defines.h which sets up some fairly commonly used defines such as SLANG_FORCE_INLINE, compiler detection, os detection, and some other cross platform features. 21 March 2018, 18:28:43 UTC
98b8e0c jsmall-nvidia 20 March 2018, 21:14:12 UTC SlangResult and small bug/typos fixes (#448) * Fixed some small typos in api-users-guide.md * Fix some small typos in slang-test/main.cpp, render-test/render-d3d11.cpp * Remove exit() calls from test code. Added Slang::Result, which works in the same way as COM HRESULT. * FIx bug introduced when moving to Slang::Result - handling E_INVALIDARG on Dx11. * Fix the testing of feature levels on Dx11 renderer. * First attempt at README.md for slang-test. * Tidied up the slang-test README.md file. * Fix some small typos in tools/slang-test/main.cpp * Fix spaces -> tabs problems. Fix some small types. 20 March 2018, 21:14:12 UTC
7256214 Tim Foley 19 March 2018, 19:45:23 UTC Entry point attribute (#447) * Typo * Add [shader(...)] and clean up some literal handling * Add supporting for validating the `[shader(...)]` attribute, by checking that its argument is a string literal that names a known shader stage. * Split the `ConstantExpr` class into distinct subclasses rooted at `LiteralExpr`, so we have `BoolLiteralExpr`, `IntegerLiteralExpr`, `FloatingPointLiteralExpr`, and `StringLiteralExpr` * Add a `String` type to the stdlib, to be used as the type of a string literal. This change allows code using `[shader(...)]` to be accepted by the front-end again, but it does nothing about emitting it in final HLSL. * Allow entry points to be specified via [shader(...)] Before this change, the compiler would track a list of `EntryPointRequest` objects, based on what the suer specified via API and/or command-line options. Each entry point request would get matched up with an AST `FuncDecl` as part of semantic checking, and then the back end steps (layout, codegen, etc.) would work from that information. This change makes the compiler modal, in that it can *either* continue to use an explicit list of entry point requests (this is the mode when the list is non-empty), or it can rely on user-supplied attributes on entry point functions to drive codegen (this is the mode when the list is empty). User-specified `[shader(...)]` attributes are processed at the same place where the association from `EntryPointRequest`s to `FuncDecl`s would otherwise be made, and basically does the same thing in the opposite direction: looks for `FuncDecl`s with the appropriate attribute and synthesizes an `EntryPointRequest` for them. Subsequent processing should ideally not know where a given `EntryPointRequest` came from, and should handle both methods of specifying the entry points equivalently. One design choice that might not make immediate sense is that we do *not* process a function as an entry point (applying further validation, etc.) just because it has a `[shader(...)]` modifier, unless we are in the appropriate mode (which in this case is the mode where the user didn't specify their own entry points via API or command line). This is to handle cases where the user wants to explicitly compile only one entry point, so that they (1) don't want us to spend time validating code they don't care about, (2) don't want do get output they don't expect, and (3) might actually be presenting us with code that violates the language rules due to a combination of `#define`s in effect (e.g., they might have a `[shader("vertex")]` function that transitively executes a `discard` because of how the preprocessor was configured, but they don't care because they are compiling a fragment entry point). This decision might be something we revisit over time. As part of this work, I had to add some logic to pick a "profile version" to use for a combination of a target and stage (because when you specify `[shader("vertex")]` the compiler can't tell if you want `vs_5_0`, `vs_5_1`, etc.). This isn't really complete right now, because something like `-target dxbc` *also* doesn't determine a profile, so there is a bit of a kludge at present. We need to figure out a good long-term plan here, which might involve keeping target format, feature level/version, and pipeline stage as truly orthogonal concepts, rather than conflating them. That would involve more work in the API and command-line layers to de-compose things when the user specifies, e.g., `vs_5_1`, but might make downstream logic easier to manage. * Emit [shader(...)] attribute on entry point for SM 6.1 and later This should help ensure that the output from Slang can be compiled with dxc `lib_*` profiles. * Fix warning 19 March 2018, 19:45:23 UTC
04c9476 jsmall-nvidia 16 March 2018, 19:02:55 UTC Fixed some small typos in api-users-guide.md (#446) 16 March 2018, 19:02:55 UTC
a549362 Yong He 16 March 2018, 18:28:04 UTC Small bug fixes. (#445) This commit contains two small bug fixes: 1. In `specializeProgramLayout`, we cannot assume the resourceInfo entries in a varLayout and its corresponding type layout has the same order. Should use `FindResourceLayout`. 2. When generating ir for a switch statement, make sure to remove the breakLabel from the shared context when done. For some reason if a switch statement is being lowered twice, the Dictionary::Add method will complain the statement key already exists. 16 March 2018, 18:28:04 UTC
4c23ba2 Tim Foley 16 March 2018, 16:06:01 UTC Typos (#444) 16 March 2018, 16:06:01 UTC
93ac152 Tim Foley 16 March 2018, 14:01:35 UTC Overhaul implementation of [attributes] (#443) The existing code parsed all of the square-bracket `[attributes]` into `HLSLUncheckedAttribute`, and then went on to hand-convert some of them to specialized subclasses of `HLSLAttribute`. When attributes didn't check, they were left as-is, and no error message was issued, because at the time the compiler was focused on accepting arbitrary input. This change greatly overhauls the handling of `[attributes]`. Attributes are now declared in the stdlib, with declarations like: ```hlsl __attributeTarget(LoopStmt) attribute_syntax [unroll(count: int = 0)] : UnrollAttribute; ``` In this syntax, the `unroll` part is giving the attribute name (the `[]` are just for flavor, to make the declaration look like a use site; we could drop it if we don't like the clutter), the `count` is a parameter of the attribute, which we expect to be of type `int`, and which has a default value of `0` if unspecified. The `: UnrollAttribute` part specifies the meta-level C++ class that will implement this attribute (and corresponds to a class in `modifier-defs.h`). This syntax is similar to our current `syntax` declarations. I'm starting to think we should change it to something like a `__meta_class(UnrollAttribute)` modifier, and then use that uniformly across all cases (e.g., also replacing the curreent `__magic_type(Foo)` syntax). The `__attributeTarget(LoopStmt)` is a modifier that specifies the meta-level C++ class for syntax that this attribute is allowed to attach to. It is legal to have more than one of these. Attributes continue to be parsed in an unchecked form, so that we don't tie up semantic analysis and parsing more than necessary. During checking, we look up the attribute name in the current scope, and then replace the unchecked attribute with a more specific one *if* the checking passes. Checking proceeds in generic and attribute-specific phases. The generic phase includes checking the number of arguments against those specified in the attribute declaration (I don't currently check types, or handle default arguments), and then checking that at least one `__attributeTarget(...)` modifier applies to the syntax node being modified. The attribute-specific phase then applies to the specialized C++ subclass of `Attribute`, and does the actual checking right now (e.g., that step is responsible for actually type-checking things at present). This can obviously be improved over time. With this support I went ahead and added declarations for all the HLSL attributes I could find documented on MSDN. I also added a provisional declaration for the `[shader(...)]` attribute that has been added to dxc, but which is not yet documented. One important detail here is that lookup of attribute names needs to be done carefully, so that we don't let, e.g., local variables shadow an attribute declaration: ```hlsl int unroll = 5; // This attribute should *not* get confused by the local variable `unroll` [unroll] for(...) { .. } ``` The lookup logic already has a notion of a `LookupMask` that can be used to filter declarations out of the result. In this change I surfaced that mask through the main lookup API (rather than requiring a second pass to "refine" lookup results), and made is so that the default lookup mask does *not* include attributes, while an explicit mask can be used to look up *only* attributes. (An alternatie design we discussed was to follow the approach of C# and have the declaration of an attribute like `[unroll]` actually be `unrollAttribute`, with a suffix. I decided not to follow that approach for now because it seemed like printing good error messages in that case could require us to carefully trim the `Attribute` suffix off of names at times, and using the existing mask behavior seemed simpler.) To verify that the shadowing behavior is indeed correct, I modified the `loop-unroll.slang` test case. Smaller notes: * Removed the `HLSL` prefix from several of the C++ attribute classes * Made sure to actually validate the modifiers on statements * Special-cased checking for `ParamDecl` with a null type, because I'm re-using `ParamDecl` for attribute parameters, but can't give a concrete type to some of them right now * Deleting some old, dead emit-from-AST logic around attributes, rather than try to "fix" code that doesn't run (a more complete scrub of that code is still needed) * Fixed AST inheritance hierarchy so that a `Modifier` is a `SyntaxNode` rather than a `SyntaxNodeBase`. I have *no* idea why we have both of those, and we need to clean that up soon. 16 March 2018, 14:01:35 UTC
e69c0bd Tim Foley 14 March 2018, 19:52:59 UTC When emitting from IR, skip structured types with `__builtin` modifier (#442) This allows users who call `spAddBuiltins` to specify `struct` types that are provided for a target, and that should not be part of the generated HLSL/GLSL from Slang. The existing emit logic already skips emitting any basic, vector, matrix, or resource types, so this case really only triggers when the standard library needs to declare a completely ordinary `struct`. No provision is made right now for a `struct` type that might be builtin on one target, but need to be declared on another. That is a clear next step for this feature. 14 March 2018, 19:52:59 UTC
972dda6 Yong He 12 March 2018, 19:07:40 UTC Merge pull request #441 from tfoleyNV/legalization-fix Don't use specialized field names when legalizing types 12 March 2018, 19:07:40 UTC
3f2b123 Yong He 12 March 2018, 19:07:29 UTC Merge branch 'master' into legalization-fix 12 March 2018, 19:07:29 UTC
e08f0c6 Yong He 12 March 2018, 18:02:28 UTC Stop compilation when a imported module contains errors. (#440) * Stop compilation when a important module contains errors. * Fixup test cases 12 March 2018, 18:02:28 UTC
e8f44d4 Tim Foley 12 March 2018, 17:26:17 UTC Don't use specialized field names when legalizing types The type legalization logic currently looks up layout information for the fields of an aggregate type using the mangled name of the field (since this should be consistent even when legalization changes the number of fields). The problem at present is that type layouts for specialized generic types were storing the mangled names of *specialized* field decl-refs, while the lookup was using the mangled names of the unspecialized declarations. The latter (unspecialized names) is what we want for simplicity, so this change fixes the comparison. 12 March 2018, 17:30:33 UTC
51bc468 Yong He 12 March 2018, 16:40:16 UTC Prevent duplicating global values during specialization (#439) * Prevent duplicating global values during specialization. * Fixup checking code 12 March 2018, 16:40:16 UTC
a22b752 Tim Foley 08 March 2018, 20:35:00 UTC Cleanups on slang-generate (#437) * Cleanups on slang-generate There is nothing too significant in these changes, but I'm trying to get things in place so that we can: - Clean up the stdlib code to do less explicit `StringBuilder` operations and instead to use more of the "template engine" approach - Start using slang-generate for code other than the slang stdlib, so that we can generate more of our boilerplate. The main new functionality here is that in a template/meta file, you can now enclose an expression in `$(...)` to indicate that is should be spliced into the result. E.g. instead of: class ${{ sb << someClassName; }} { ... } We can now write: class $(someClassName) { ... } The other bit of new functionality is support for a whole-line statement escape, so that instead of: ${{ for( auto a : someCollection ) { }} void $(a)() { ... } ${{ } }} We can instead write: $: for(auto a : someCollection) { void $(a)() { ... } $: } I haven't yet tried to use that functionality in the stdlib meta-code, but doing so would be an obvious next step. * Fixup: change some $P to $p The capitalization on some of the GLSL intrinsic mappings got messed up during a find-and-replace operation when removing the double `$` that used to be required to escape things. 08 March 2018, 20:35:00 UTC
ed718ba Yong He 06 March 2018, 21:12:37 UTC Add a case to `TryUnifyVals` to cover `SubtypeWitness` vals (#435) 06 March 2018, 21:12:37 UTC
1fef9b4 Tim Foley 03 March 2018, 15:16:08 UTC IR: next phase of "everything is an instruction" (#433) The main practical change here is that things that used to be `IRValue`s, like literals, are now being expressed as instructions in the global scope. In order to validate that things are actually being handled correctly, this change introduces an explicit "validation" pass that can be run on the IR to check for different invariants (although it doesn't check many of the important ones right now). I've left the validation pass turned off by default, but with a command-line flag to enable it. We may want to make it be on by default in debug builds, just to keep us honest. The main invariant for the moment is that when on IR instruction is used as an operand to another, it had better come from the same IR module. Some of the existing passes were violating this rule, in particular when it came to cloning of witness tables related to global generic parameter substitution. Those features can in theory be handled better now by allowing `specialize` instructions at other scopes, but I didn't want to over-complicate this change, so I make just enough fixes to ensure that these steps always clone witness tables they get from the "symbols" on an IR specialization context. In order for this to work when recursively specializing, I had to ensure that the logic for generic specialization had a notion of a "parent" specialization context that it would fall back to to perform cloning when necessary. This change keeps the logic that was caching and re-using the instructions for literal values within a module, but adds some logic that isn't really being tested right now for picking the right parent instruction to insert a constant instruction into. This logic doesn't trigger right now because all of the cases we are using it on have zero operands (and so they always get "hoisted" to the global scope), but eventually for things like types we want to be able to support instructions with operands (e.g., `vector<float, 4>`) and handle the case where some of those operands come from different scopes (e.g., when nested inside a generic). The final change here is mostly cosmetic: the `IRBuilder` is now more abstract about where insertion occurs: it tracks a single `IRParentInst` to insert into, and then an optional `IRInst` to insert before. In the common case, that parent is an `IRBlock`, but it could conceivably also be the global scope, or a witness table, etc. Use sites where we used to change those fields directly now use distinct methods `setInsertInto(parent)` and `setInsertBefore(inst)` which capture the two cases we care about. Accessors are also defined to extract the current block (if the current parent is a block), and the current "function" (global value with code, if the current parent is a global value with code, or a block inside one). With this work in place, it should be possible for a follow-on change to start putting `specialize` instructions at the global scope and thus clean up some of the on-the-fly specialization work. This work should also help with some of the requirements around a distinct IR-level type system and more explicit generics. 03 March 2018, 15:16:08 UTC
back to top