Revision history - None - origin: https://github.com/shader-slang/slang

visit type:

Newer
Older

Revision	Author	Date	Message	Commit Date
6d61421	Tim Foley	13 December 2017, 23:48:29 UTC	Fix parameter block binding for Vulkan (#308) Fixes #307 This ends up being a major overhaul over how type layout computation is structured and exposed. The big problems all arise around cases where both the "container" for a parameter block or CB, and the "element" type both use the same kind of resource. E.g., if you define a CB with a texture in it, then in Vulkan both the CB and the texture use the same kind of resource, and so if you query the CB's resource usage it will just tell you it uses two descriptor-table slots, but nothing more than that. Similar confusion still arises in the HLSL case, when a CB with a texture in it reports its parameter category as "mixed" so that a user might query for a category they didn't mean to. There were also cases in the existing code where a parameter block might expose both a register-space usage and another concrete resource type, which isn't right. The most important changes here are: - A `ParameterGroupTypeLayout` now has a more refined internal structure, consisting of: - A `containerTypeLayout`, which represents the resource usage of the buffer/block itself (e.g., if a constant buffer had to be allocated) - An `elementVarLayout` which stores the offsets that need to be applied to get from the `VarLayout` for an instance of this parameter-group type to the offsets of its elements. The `TypeLayout` for this variable layout should be the "raw" type of the block/CB element. - The `offsetElementTypeLayout` (formerly just `elementTypeLayout`) which represents the element type, but in the case of a `struct` element type, will have fields offset similar to the `elementVarLayout`. This is what all the old code used to use, so we need to keep it for compatibility. - When doing reflection on a `ParameterGroupTypeLayout`, we now only report the resource usage of the `containerTypeLayout`. This is technically a potentially breaking change in the public API, but I don't think Falcor will mind, since they actually want something closer to this behavior. - Add a new public API for querying the element variable layout of a parameter block of constant buffer. This could be used by savvy applications to fold the handling of CB element offsetting into some notion of a "reflection path." This would be required for applications that want to handle CBs or parameter blocks where the element type is not a `struct` type. - Remove old logic for applying an offset when creating a type layout for constant buffer element, and instead perform offsetting more uniformly later, by constructing the `offsetElementTypeLayout` from the `rawElementTypeLayout`. This is useful both because we want to keep both (the "raw" type layout becomes the type layout of the `elementVarLayout`), and also because we can decide later whether we even want to allocate a CB register for a buffer, based on whether it actually contains any uniform data. - Fix cases where we might end up with a parameter block type reporting both that it uses a whole register space (and thus should not expose the resource usage of the container/element type) and a constant-buffer register/slot. The latter should be hidden inside the regsiter space. - Clean up the `spReflectionParameter_GetBinding{Index,Space}` functions to just route to `spReflectionVariableLayout_Get{Offset,Space}`, using the "default" category of the parameter - Try to make the `GetSpace` query take into account cases where a variable also has an explicit `RegisterSpace` allocation. - This probably still needs some cleanup, since ideally we'd just move things into the `space` field on the `ReosurceInfo` and have an invariant that a variable either has a `RegisterSpace` allocation, or it has other resource infos, but never both... - Add some ad-hoc logic so that if the user queries for a binding index/space using a parameter category that doesn't actually apply (e.g., they query for a D3D `t` register when using Vulkan), we can optionally remap it to the resource type they "probably" meant. This is a mess of Do What I Mean code, but it is also what our users want right now. - Fix various bits of emit logic so that if a parameter block has a register space/set allocated to it, we properly output that as part of the binding information for it. - This is another thing that might be cleaned up if we rationale the way that things get split during legalization. - Add a GLSL case for emitting a parameter block variable as a `cbuffer`.	13 December 2017, 23:48:29 UTC
0f55649	Tim Foley	08 December 2017, 22:23:12 UTC	Cleanups to `ParameterBlock<T>` behavior. (#304) * Cleanups to `ParameterBlock<T>` behavior. These add some more realistic tests using the `ParameterBlock<T>` support, and show that it can work with the "rewriter" mode. Unfortunately, this code does not currently work with the rewriter + the IR at once. That will need to be fixed in a follow-on change, because I now see that the root problem is pretty ugly. * cleanup	08 December 2017, 22:23:12 UTC
301cdf5	Tim Foley	06 December 2017, 21:55:31 UTC	Make AST and IR share type legalization code (#303) * Make AST and IR share type legalization code A previous change already made it so that the AST-to-AST lowering/legalization pass could work together with IR-based lowering of `import`ed code, but that change didn't take into account the case where a function written in the AST needed to call an IR function and pass in a type that required legalization. Both the IR-based and AST-based passes had their own approaches to type legalization, that mostly agreed on the desired output, but they ended up creating their own representations for legalized types which would mean that for a function call the caller and callee might end up legalizing the parameter list to use different types. This change tries to fix this issue (and adds a new test case that relies on the fix) by massively overhauling the AST-based legalization pass so that it uses the same type legalization code as the IR. The shared code has been moved out into `legalize-types.{h,cpp}`. Notes: - I eliminated the `FilteredTupleType` type, since it was starting to cause code duplication in a lot of places. Instead, type legalization just creates new `struct` types to represent the result of filtering. - One big consequence of this is that the `LegalType::pair` case needs to remember for each field in the original type which field (if any) in the new `struct` type it maps to - A big source of complexity (and probably bugs) in this code is trying to figure out how to parent these new `struct` definitions effectively. A good follow-on change would be something that outputs declarations on-demand during the AST emit logic (as we do for the IR), just to avoid some of this song and dance. - The old AST type legalization had a notion of both a "tuple" type and a "varying tuple" type. The "tuple" case was quite complex, and combined behavior currently handled by `LegalType::pair` (for splitting into ordinary and special sides) and `LegalType::tuple` (for holding multiple distinct elements to represent the fields of an aggregate). The "varying tuple" case was closer to `LegalType::tuple`, so I tried to just re-use the existing logic for that too. The one place this potentially gets messy is in `reifyTuple()`. - The messiest bit of handling the "varying tuple" concept (which is used for GLSL shader inputs/outputs since they have to be scalarized) is that when passing them as function arguments we need to reify the tuple back into a structured value. Because the `LegalExpr` hierarchy doesn't have type information, but constructing a value of the "original" type requires such information, things get a little messy. - I did not try to deal with any of the logic related to handling system inputs/outputs for cross-compilation purposes. Of course, the long-term goal is that any actual cross-compilation is handled via the IR, but this change can't afford to break the AST-based path just yet. As a result, there is still quite a bit of complexity in the handling of assignment, to deal with cases where "fixups" are required. * fixup: bad code in macro, not caught by Visual Studio compiler * fixup: more stuff missed by VS compiler * fixup: VS continutes to miss stuff in UNREACHABLE_RETURN	06 December 2017, 21:55:31 UTC
b487516	Tim Foley	29 November 2017, 19:45:15 UTC	Add API to query stage of varying parameter (#302) Fixes #301 The problem here is that if you have input GLSL code like: ```glsl // example.vs in vec3 pos; ``` and: ```glsl // example.fs in vec3 worldPos; ``` Then both `pos` and `worldPos` are reflected as global variables (parameters of the program), which both get bound to "varying input" resources, but there is no way to tell through the API that `pos` is a vertex parameter while `worldPos` is a fragment one. The original request in issue #301 was to expose parameters like this not as a global variables, but rather as parameters of the entry point in their specific file. That is, treat it as if the user had written, e.g.: ```glsl // example.vs void vsMain(in vec3 pos) { ... } ``` Doing that would unify the GLSL and HLSL/Slang cases a bit, but would require the Slang reflection API to lie about the structure of code the user wrote. At a more basic level, that would have been hard to implement because the current reflection API just exposes the underlying AST, and the AST needs to leave `pos` at the global scope so that when we go and spit GLSL back out we retain the original structure. This PR implements a more simplistic solution, where the user is allowed to query the stage that a varying parameter "belongs" to. For right now I'm only enabling this to work for varying parameters (but it doesn't care if they are entry-point or global-scope varyings). Despite what I said on #301, this should work for both the top-level parameter's variable layout, and any variable layouts for fields within its type reflection. In terms of implementation, I took the simple but wasteful route: every `VarLayout` now has a `stage` field that is by default initialized to `SLANG_STAGE_NONE`. When collecting varying parameters, I take advantage of the fact that everything bottlenecks through `processEntryPointParameter()` which takes an `EntryPointParameterState` so that I can set the `VarLayout::stage` field for any varying parameter in one place. While I was making this change, I also did a bit of cleanup so that the "official" names for the varying parameter categories are `VARYING_INPUT` and `VARYING_OUTPUT`, with `VERTEX_INPUT` and `FRAGMENT_OUTPUT` being "deprecated" in principle. I didn't do the bulk rename inside the codebase yet.	29 November 2017, 19:45:15 UTC
7139380	Tim Foley	29 November 2017, 03:49:21 UTC	Enable HLSL/GLSL "rewrite" + IR-based Slang codegen (#300) The big picture here is that the AST-to-AST pass in `ast-legalize` will now detect when a declaration being referenced comes from an `import`ed module, and (if IR codegen is enabled), it will trigger cloning of the IR for the chosen symbol into an IR module that will sit alongside the legalized AST. Then, during HLSL/GLSL code emit, we emit all the IR-based code first, and then the AST-based code. Whenever the AST code references a symbol that was lowered via IR (we keep track of these) we emit the mangled name of the IR symbol. Notes/details: - A lot of the logic for cloning IR symbols referenced by the AST matches the same logic that would clone them for completely IR-based codegen, so I tried to hoist out the common logic and share it (e.g., so that we apply the same guaranteed transformations in both cases). This required basically rewriting the logic in `emit.cpp` that decomposed the various cases. - There is a new compute test case added to test this functionality. `tests/compute/rewriter.hlsl` confirms that we can use the `-no-checking` mode for the HLSL code, but still make use of a library of Slang code that employs generics, etc. - Adding this test case required adding a new compute test mode that invokes `render-test` with the `-hlsl-rewrite` flag. - It turns out that the existing `tests/render/cross-compile0.hlsl` test should have been using this functionality already. It was opting into the use of the IR via `-use-ir`, and the `render-test` application already tries to set `-no-checking` for non-Slang input languages by default. Fixing the code path this test triggers means that it is now a second test of rewriter+IR codegen. - The `translateDeclRef` logic in `ast-legalize.cpp` seemed sloppy in places, and would potentially clone declarations, when declaration references were desired. I tried to clean a bit of this up, so some call sites are now changed. - This change tries to clean up some work around cloning of global values - All global value kinds (not just functions) now go through the logic of trying to pick a "best" definition, so that they can be used when we are linking multiple modules - The logic for registering cloned values has been unified a bit, so that clients always pass in an `IROriginalValuesForClone` that either wraps a single value (maybe just null), or an `IRSpecSymbol` that gives a list of values to regsiter the new value as a clone for. - I made one piece of code that was cloning witness tables as part of generic specializations not* register a clone. I think this is correct because we may specialize the same generic multiple ways, so registering any values we clone is not the right idea, but I might be missing something... - I also reorganized this logic so that it would be easier to clone a global value when we only know its mangled name (which is the case when it is the AST that triggers cloning) - I made sure that when loading a module via `import`, the translation unit for the new module copies the `-use-ir` flag from the overall compile request, if it is present (otherwise we wouldn't generate IR for loaded modules at all... oops). - Note that `getSpecializedGlobalValueForDeclRef()`, which is the main routine used by the AST legalization to trigger cloning of an IR value does not currently handle declaration references that require specialization. - This change does not deal with trying to unify the type legalization logic between the AST-to-AST rewriter and the IR-based codegen, so if you call an imported function with types that require legalization, Bad Things are expected to happen right now.	29 November 2017, 03:49:21 UTC
4951003	Tim Foley	28 November 2017, 14:42:13 UTC	Generate IR per-module for loaded modules (#299) The basic idea here is that for each module that gets loaded via `import`, we should also generate the initial IR for the declarations in that module at the time it gets loaded. Furthermore, when we generate initial IR for a module, we will only generate IR declarations (not definitions) for any functions/variables in modules it imports. Later, when cloning IR to begin code generation for an entry point, we will effectively "link" all of the loadedm modules together, so that a given global value can get its definition from any of the IR modules present. - Change the `loadedModulesList` and related data structures to hold a new `LoadedModule` type, instead of just the AST (and then have a `LoadedModule` own both the AST and the IR module) - Share some logic between the `import` and `#import` cases, so that we always try to generate IR for modules we load. - Make sure that IR generation always gets skipped if the command-line flags tell us not to use the IR. - A few small fixups for cases that didn't arise in IR lowering so far, but come up when we try to actually generate IR for things like the stdlib. There are some notable gaps in this work right now: - The stdlib modules are exempted from this behavior; we always generate IR for stdlib functions in any user module that calls them. This is just a workaround for the fact that the stdlib modules don't show up in the list of imported modules right now. - We don't currently have logic that does the "linking" step for global variables like we do for functions. We really need to look up the symbols with the same mangled name, and favor any one of them that has a definition (if there is one) - Similarly, the handling of witness tables is incomplete. During initial IR generation, we should probably be generating empty witness tables for any conformances that were declared in other modules (but are being used locally in this module), and then the "linking" step should favor non-empty witness tables over empty ones. Still, all the test cases pass with the code like this, and this seems like an important step in the right direction.	28 November 2017, 14:42:13 UTC
3199385	Tim Foley	28 November 2017, 00:33:28 UTC	Cleanups (#298) * Rename `lower.{h,cpp}` to `ast-legalize.{h,cpp}` This pass isn't really performing lowering akin to `lower-to-ir.{h,cpp}` so the file name is misleading. By renaming this pass we emphasize its role as an AST-related pass. Also update the comment at the top of `ast-legalize.h` to reflect the intended purpose of this pass in a world where we have the IR up and running. * Allow `import` as an alias for `__import` The use of double underscores to mark our new syntax has so far had two purposes: 1. It helps identify syntax that isn't meant to be exposed to users in its current form (e.g., `__generic` gets a double underscore because we want users to have a more pleasant surface syntax for generics that they write). This rationale doesn't apply to `__import`, which is a major language feature that users need to interact with. 2. It helps avoid the problem where the compiler treats something as a keyword that isn't supposed to be reserved in HLSL/GLSL and so causes existing user code to fail to parse (e.g., when the user tries to write a function called `import`). This no longer matters because we look up almost all of our keywords using the existing lexical scoping in the language (so the user can shadow almost any keyword with a local declaration). So, neither of the original two reasons applies to `__import`, and it makes sense to expose it as `import`. Doing so is a one-line change.	28 November 2017, 00:33:28 UTC
109ee8a	Yong He	24 November 2017, 16:55:54 UTC	Fix substitution mechanism to remove special cases for global params (#297) Add a new function: `substituteSubstitutions(Substitutions * substHead, Substitutions subst, int * ioDiff)` This function substitutes the type arguments referenced in a linked list of substitutions headed at `substHead` using the substitutions specified by `subst`. If the linked list `substHead` does not contain `GlobalGenericParamSubstitution` entries, they will be added to the bottom (outter most) of the linked list. Note that this function should be called when `substHead` is known to be the head of substitution linked list because the existance of `GlobalGenericPaaramSubstitution` is detected assuming the linked lists starts at `substHead`. If a substitution that is not the head of a substitution linked list is passed in, duplicate `GlobalGenericParamSubstitution`s could be appended to the linked list. This means that this function should not be called in places like `GenericSubstitution::SubstitutionImpl()` for its outer substitutions, because `outer` is obviously not the head of the linked list. Instead, use this function to substitution the substitution lists of `DeclRef` etc. instead of calling `declRef.substitutions->SubstituteImpl()` where the head to the linked list is known as a member of that class. With this function, IRSpecContext::maybeCloneType() is simplified down to `originalType->Substitute(subst)` Updates `DeclRefBase::SubstituteImpl` and `DeclRefType::SubstituteImpl` to call `substituteSubstitutions` instead of making direct `substitutions->SubstituteImpl` call. Providing actual implementation of `GlobalGenericParamSubstitution::SubstituteImpl` instead of just returning `this` to deal with potential situations where a true substitution is needed.	24 November 2017, 16:55:54 UTC
83d49ce	Yong He	22 November 2017, 22:32:15 UTC	Merge pull request #293 from csyonghe/generic-param-fix Fixup global generic parameters	22 November 2017, 22:32:15 UTC
581b30d	Tim Foley	22 November 2017, 21:58:16 UTC	Merge branch 'master' into generic-param-fix	22 November 2017, 21:58:16 UTC
56e49fe	Tim Foley	22 November 2017, 19:28:29 UTC	Fix emitting of loop attributes for HLSL pass-through (#296) Fixes #295. The code previously had a white list of attributes that it passed through, implemented in `emit.cpp` in an ad hoc fashion. The fix here is to just pass through whatever attributes the user wrote, and then let the downstream compiler diagnose if any of them are errorneous.	22 November 2017, 19:28:29 UTC
cd2d646	Yong He	22 November 2017, 00:55:23 UTC	Merge branch 'master' into generic-param-fix	22 November 2017, 00:55:23 UTC
43434bf	Yong He	22 November 2017, 00:26:56 UTC	update input layout definition of test case `global-type-param-in-entrypoint`	22 November 2017, 00:26:56 UTC
64108c8	Yong He	22 November 2017, 00:26:17 UTC	Add logic to propagate GlobalGenericParamSubstitution	22 November 2017, 00:26:17 UTC
37315c9	Tim Foley	20 November 2017, 21:45:10 UTC	IR: support global variable with initializers (#294) The big change here is that the ability to contain basic blocks with instructions in them has been hoisted from `IRFunc` into a new base type `IRGlobalValueWithCode` shared with `IRGlobalVar`. The basic blocks of a global variable define initialization logic for it; they can be looked at like a function that returns the initial value. Places in the IR that used to assume functions contain all the code need to be updated, but so far I only handled the cloning step. The emit logic currently handles an initializer for a global variable by outputting its logic as a separate function, and then having the variable call that function to initialize itself. This should be cleaned up over time so that we generate an ordinary expression whenever possible. I also made the emit logic correctly label any global variable without a layout (that is, any that don't represent a shader parameter) as `static` so that the downstream HLSL compiler sees them as variables rather than parameters.	20 November 2017, 21:45:10 UTC
ab49ac3	Yong He	20 November 2017, 10:49:57 UTC	fix warnings	20 November 2017, 10:49:57 UTC
d6130ba	Yong He	20 November 2017, 10:28:22 UTC	fixup global generic parameters 1. simplify RoundUpToAlignment() 2. add new a render-compute test case to cover the situation where the entry-point interface (parameter/return types of an entry-point function) is dependent on the global generic type. 3. initial fixes to get this test case to compile (but is not producing correct HLSL output yet)	20 November 2017, 10:28:22 UTC
3dff5a5	Tim Foley	18 November 2017, 03:38:26 UTC	IR: add lowering for initializer list expressions (#290) * IR: add lowering for initializer list expressions This is relatively straightforward in the easy cases, because the front-end will have already type-checked the elements of the initializer list, and attached an appropriate type to the overall expression. Notes: - We are assuming in this code that if the user provides a "flattened" initializer list when dealing with nested aggregates, then the front-end is responsible for grouping things up apprporiately (this is not actually implemented in the front-end today). - I have only handled arrays and `struct` types here, so uses of initializer lists for anything else will fail. - I have not tried to handle the common HLSL idiom of using `{0}` as a way to default-initialize things, even when their first field is not compatible with the expression `0` - I have not implemented support for default-initializing fields/elements beyond those for which explicit initializers were provided. This can be addressed as a follow-on change. This change is one clear place where the front-end lowering logic could potentially be made much cleaner using a "destination-driven" code generation strategy. For example, given the following code ```hlsl struct A { int a0; a1; }; struct B { A b0; A b1; }; struct C { B c0; B c1; }; // ... C c = { { { 0, 1 }, {2, 3}, }, /* ... / }; ``` Our current code generator will end up allocating local variables for 1 instance of `C`, two instances of `B`, and four instances of `C`, for over 3x the allocation that would be done by a good destination-driven code generator. Yes, later optimization passes should be able to clean up the waste, but avoiding the waste from the start should result in faster compiles and also easier debugging (since intermediate IR won't be as messy in general). Fixup: try to appease clang compiler	18 November 2017, 03:38:26 UTC
ba594d0	Tim Foley	18 November 2017, 03:01:58 UTC	IR: Add support for `out` and `inout` parameters (#289) These were already being handled a little bit, by lowering an `out T` or `inout T` function parameter in the AST to a function parameter with type `T*` in the IR, and then emiting explicit loads/stores. The HLSL emit logic, however, couldn't tell the difference between an `out` parameter, an `inout`, or a true pointer (if we ever needed to support them...). The intention (not fully implemented) was that we'd use a hierarchy of types rooted at `PtrTypeBase`: - `PtrTypeBase` - `Ptr`: "real" pointers in the C/C++ sense - `OutTypeBase`: pointers used to represent by-reference parameter passing - `OutType`: IR level type for an `out` parameter - `InOutType`: IR level type for an `inout` or `in out` parameter Actually implementing this involved: - Adding a bit more flexibility to the `Session::getPtrType` logic to allow for creating any of the concrete types above - Making the `lower-to-ir` logic create the right type for function parameters (instead of just using `PtrType`) - Making the HLSL emit logic check for the `OutType` and `InOutType` cases rather than just `PtrType` - Changing a bunch of small places in the code so that they use `PtrTypeBase` instead of `PtrType` when they should handle any of the above cases, and also make a few places check for `OutTypeBase` instead of `PtrType` or `PtrTypeBase`, when they are really trying to capture by-reference parameters - Add a test case that uses all of the different cases we care about (without these fixes, this test case generates errors from fxc because of variables being used before being initialized, becaues parameters get declared `out` that should be `inout`). A minor point here is that we are playing a bit fast and loose right now because the IR does not actually enforce any type checks. From the standpoint of the front end, `Ptr<T>`, `Out<T>`, and `InOut<T>` are all unrelated types (each is just a `struct` declared in `core.meta.slang`), but this doesn't really matter because none of these are types our current users are explicitly using. In the IR it makes perfect sense to allow `Out<T>` or `InOut<T>` as the operand of a `load` or `store` instruction (and ditto for `getFieldAddr`, etc.) - there instructions just apply to any `PtrTypeBase`. The place where this potentially gets tricky is whether an `Out<T>` can be used where a `Ptr<T>` is expected, or vice vers (e.g., can I just pass my local variable's pointer directly to an `Out<T>` function parameter? I'm going to ignore these issues for now, since the code currently works for our test case.	18 November 2017, 03:01:58 UTC
54bf54b	Yong He	18 November 2017, 02:26:21 UTC	Add support for global generic parameters (#285) * Add support for global generic parameters (In-progress work) This commit include: 1. Update Slang API to allow specification of generic type arguments in an `EntryPointRequest` 2. Add parsing of `__generic_param` construct, which becomes a GlobalGenericParamDecl, contains members of `GenericTypeConstraintDecl`. 3. Semantics checking will check whether the provided type arguments conform to the interfaces as defined by the generic parameter, and store SubtypeWitness values in the EntryPointRequest, which will be used by `specializeIRForEntryPoint` when generating final IR. 4. Add a new type of substitution - `GlobalGenericParamSubstitution` for subsittuting references to `__generic_param` decls or to its member `GenericTypeConsraintDecl` with the actual type argument or witness tables. 5. Update `IRSpecContext` to apply `GlobalGenericParamSubstitution` when specializing the IR for an EntryPointRequest. 6. Update `render-test` to take additional `type` inputs, which specifies the type arguments to substitute into the global `__generic_param` types. This commit does not include ProgramLayout specialization. * IR: pass through `[unroll]` attribute (#284) The initial lowering was adding an `IRLoopControlDecoration` to the instruction at the head of a loop, but this was getting dropped when the IR gets cloned for a particular entry point. The fix was simply to add a case for loop-control decorations to `cloneDecoration`. * fix warnings * IR: support `CompileTimeForStmt` (#286) This statement type is a bit of a hack, to support loops that must be unrolled. The AST-to-AST pass handles them by cloning the AST for the loop body N times, and it was easy enough to do the same thing for the IR: emit the instructions for the body N times. The only thing that requires a bit of care is that now we might see the same variable declarations multiple times, so we need to play it safe and overwrite existing entries in our map from declarations to their IR values. Of course a better answer long-term would be to do the actual unrolling in the IR. This is especially true because we might some day want to support compile-time/must-unroll loops in functions, where the loop counter comes in as a parameter (but must still be compile-time-constant at every call site). * Add support for global generic parameters (In-progress work) This commit include: 1. Update Slang API to allow specification of generic type arguments in an `EntryPointRequest` 2. Add parsing of `__generic_param` construct, which becomes a GlobalGenericParamDecl, contains members of `GenericTypeConstraintDecl`. 3. Semantics checking will check whether the provided type arguments conform to the interfaces as defined by the generic parameter, and store SubtypeWitness values in the EntryPointRequest, which will be used by `specializeIRForEntryPoint` when generating final IR. 4. Add a new type of substitution - `GlobalGenericParamSubstitution` for subsittuting references to `__generic_param` decls or to its member `GenericTypeConsraintDecl` with the actual type argument or witness tables. 5. Update `IRSpecContext` to apply `GlobalGenericParamSubstitution` when specializing the IR for an EntryPointRequest. 6. Update `render-test` to take additional `type` inputs, which specifies the type arguments to substitute into the global `__generic_param` types. progress on parameter binding * Add a more contrived test case for specializing parameter bindings * update render-test to align buffers to 256 bytes (to get rid of D3D complains on minimal buffer size). * adding one more test case for parameter binding specialization. * Cleanup according to @tfoleyNV 's suggestions. * fix a bug introduced in the cleanup	18 November 2017, 02:26:21 UTC
0298a04	Tim Foley	17 November 2017, 15:09:58 UTC	IR: support `CompileTimeForStmt` (#286) This statement type is a bit of a hack, to support loops that must be unrolled. The AST-to-AST pass handles them by cloning the AST for the loop body N times, and it was easy enough to do the same thing for the IR: emit the instructions for the body N times. The only thing that requires a bit of care is that now we might see the same variable declarations multiple times, so we need to play it safe and overwrite existing entries in our map from declarations to their IR values. Of course a better answer long-term would be to do the actual unrolling in the IR. This is especially true because we might some day want to support compile-time/must-unroll loops in functions, where the loop counter comes in as a parameter (but must still be compile-time-constant at every call site).	17 November 2017, 15:09:58 UTC
0e3d9ba	Tim Foley	17 November 2017, 03:31:57 UTC	IR: pass through `[unroll]` attribute (#284) The initial lowering was adding an `IRLoopControlDecoration` to the instruction at the head of a loop, but this was getting dropped when the IR gets cloned for a particular entry point. The fix was simply to add a case for loop-control decorations to `cloneDecoration`.	17 November 2017, 03:31:57 UTC
871fbee	Tim Foley	16 November 2017, 22:01:16 UTC	Revise type legalization so it can handle constant buffers (#282) * Revise type legalization so it can handle constant buffers The existing legalization approach with "tuples" can handle scalarizing a `struct` type with resource-type fields in it, but it had several big gaps. The most notable is that given a type that mixes uniform and resource fields, we can't just blindly scalarize things: ``` struct P { float4 a; float4 b; Texture2D t; }; cbuffer C { P gParam[8]; }; ``` The existing code was completely ignoring the declaration of `gParam` inside `C`, but even if we fixed that issue, we'd get something like: ``` cbuffer C { float4 gParam_a[8]; float4 gParam_b[8]; }; Texture2D gParam_t[8]; ``` In this case we've completely changed the layout of the uniform buffer, by switching from AOS to SOA. Even if we could get the type layout logic and the IR to agree on this, it would be a surprise to users, and "principle of least surprise" should be a big deal on a project with as many moving parts as ours. The right thing to do is to have legalization create a "stripped" version of the original type `P` and use that: ``` struct P_stripped { float4 a; float4 b; }; cbuffer C { P_stripped gParam[8]; }; Texture2D gParam_t[8]; ``` Then at a call site, this: ``` foo(gParam); ``` becomes: ``` foo(gParam, gParam_t); ``` This is exactly how the current AST-to-AST legalization handles mixed uniform and resource types, but the way it does it involves some annoying kludges: - That pass has a notion of a "tuple" similar to our legalization, but every tuple has an optional "primary" entry for all the uniform data, plus tuple elements for the resources, and a given field may be represented on one side, the other, or both. It makes the code for handling tuples very messy. - That pass does the "stripping" of types by actually marking up the AST declarations (this is okay because it is constructing a new AST as it goes), so that when they get emitted certain fields don't actually show up. That is, we fix the problem with type `P` by actually modifying the user's declaration of `P`. That seems out of bounds for the IR. This change fixes the problem in our IR type legalization while trying to avoid the problems of the AST-to-AST pass by using two new ideas: 1. We add a new case for `LegalType` (and `LegalVal`) that is a "pair" type, where a pair consists of both an "ordinary" type (for uniform data) and a "special" type (for resource data). E.g., after legalization, the type for `C` (which can be over-simplified to `ConstantBuffer<P>` for our purposes), will be a `LegalType::pair` where the ordinary side is `ConstantBuffer<P_stripped>` and the special side is a tuple containing the `Texture2D` field. 2. We add a new (and annoyingly hacky) AST-level type called `FilteredTupleType` which is semantically a sort of tuple type (it holds a list of elements, and the elements have their own types), but which remembers an "original type" that it was created from, and for each element remembers the field of the original type that it corresponds to. This is used to construct a type like `P_stripped` as an actual AST-level structural type. The core logic for legalizing an aggregate type had to get more complicated just because of the new pair case, so there is now a `TupleTypeBuilder` that asists with taking an aggregate type, processing its fields, and then picking the right `LegalType` representation for the result. Other smaller changes: - Made the legalization logic actually legalize `PtrType<T>`. E.g., if `T` legalizes to a tuple, we need to construct a tuple of pointer types. The same exact thing needs to be applied to arrays, and any other generic type that should "distribute over" pairs/tuples. - Made the legalization logic actually legalize `ConstantBuffer<T>` and similar. The basic idea there is if `T` maps to a pair, we wrap `ConstantBuffer<...>` around the ordinary side, and `implicitDeref` around the special side. - Removed a bunch of `#ifdef`ed-out code from the end of `ir-legalize-types.cpp`. That was code from my first attempt at legalization that failed miserably (trying to do it via local changes and a work list instead of a global rewrite pass), but it had some code I wanted to reference when writing the version that actually got checked in (should have deleted the code earlier, though). - Added a bunch of cases for `LegalType::none` (and the `LegalVal` equivalent) that helped simplify the logic fo the `pair` case by allowing me to always dispatch to both the "ordinary" and "special" sides, even if they might not actually be present. - Renamed `TupleType` and `TupleVal` over to `TuplePseudoType` and `TuplePseudoval` to recognize the fact that we might actually need/want real tuples in the type system, to go along with these fake ones (that need to be optimized away). The biggest doubt I have about this change is the whole `FilteredTupleType` thing; it seems like an obviously contrived type to add to the front-end type system that really only solves IR-level problems. A cleaner approach might have been to just add a plain old `TupleType` to the front-end type system (and initially I started with that), and then have yet another `LegalType`/`LegalVal` case that handles mapping from the fields of the original type to the numbered tuple elements. I expect we'll actually want to make that change in the future (especially if we ever add true tuples to the front-end), but for right now I let myself be swayed by the desire to have these stripped/filtered types get names that explain their provenance ("where they came from") to make our output code more debuggable. The way I've done it is probably overkill, though, and we need a much more complete effort on the readability and debuggability of our output before anything like that is worth worrying about. * Fixup: typo * Fixup: fix output of "non-mangled" names for test cases - Make sure to attach high-level decls to variables created as part of type legalization - Also, try to share more of the code between the different cases of variables - Fix up `parameter-blocks` test case that was passing `-no-mangle` but expecting mangled names in the output - Fix up `multiple-parameter-blocks` to not rely on `-no-mangle` for now, because it would lead to two global variables with the same name (need to fix that underlying issue eventually). - Also fix name generation logic so that we only use "original" names (from high-level decls) specifically when the `-no-mangle` flag is on, and otherwise use IR-level names. * Fix: handle constant buffers better in render-test - Don't request both CB and SRV usage for buffers, since that is illegal - Also, don't try to create an SRV when user requested a CB (since the required usage flag won't be there) - Record the input buffer type on the `D3DBinding` for a buffer, and use that to tell us when to bind a CB instead of SRV/UAV - Fix expected output for `cbuffer-legalize` test now that we are actually feeding it correct cbuffer dta.	16 November 2017, 22:01:16 UTC
1a0a7f6	Tim Foley	15 November 2017, 18:50:15 UTC	Various IR fixes for Falcor (#280) - Change function mangling so we use `p<parameterCount>p` instead of just `p<parameterCount>` to avoid the parameter count running into digits at the start of a mangled type name and tripping up the un-mangling logic. - We really need to step back at some point and define our mangling scheme a bit more carefully, especially if we are going to keep going down this road where un-mangling things is important for generating HLSL output. - Also allow the unmangling logic to unmangle a few more cases of generic parameters, so that it can skip over them to get to the parameter count of the underlying function. - Add a notion of an `unreachable` instruction to the IR, and emit it as the terminator (if needed) at the end of the last block for a function with a non-void return type. - This does not implement any logic to emit a diagnostic if the `unreachable` turns out to be potentially reachable - Fix a bug in IR specialization of generics where we can't create two different specializations of the same function, because both get registered in the same hash map With all these fixes, testing in Falcor modified to use the full Slang compiler and IR for all HLSL/Slang: - The UI and text rendering shaders yield HLSL that compiles without error; no idea if they actually work - The ModelViewer shaders yield HLSL, but there are some issues (looks like type legalization isn't applying to stuff inside constant buffers)	15 November 2017, 18:50:15 UTC
59a4c0c	Tim Foley	14 November 2017, 19:33:36 UTC	IR: add support for `switch` statements (#278) * IR: add support for `switch` statements Fixes #273 This is just something we hadn't gotten to yet on the IR. The actual design of the instruction is unsurprising (once you take into consideration the requirement for structured control flow). A `switch` instruction takes the form: switch <condition> <breakLabel> <defaultLabel> [<caseVal> <caseLabel>]* Where `condition` is the value to switch on, `breakLabel` is the "join point" after the original `switch` statement, `defaultLabel` is where to go if the value doesn't match any case, and each pair of `caseVal` and `caseLabel` is what to do on a particular value. It is required that `caseVal` be a literal, but this isn't currently being enforced in the IR (the front-end should be making a check and constant-folding the case labels). For structured control flow, we also make the assumption that the cases are in order: cases with the same label must be grouped together, and any case that falls through to another must come right before it. Given this representation, the emit logic can reconstruct a `switch` statement with relative ease, given the machinery we already have. It makes sure to group together case values with the same label (again, assuming they are contiguous), and will insert the `default:` label in with whatever group it belongs to. Actually emitting code for a `switch` statement seems superficially simple, until you realize that a complete implementation needs to handle stuff like "Duff's Device." The current implementation makes the assumption that all `case` and `default` statements are directly nested under a `switch`, and that there is no way for control flow to enter a case except by the `switch` itself, or fall-through. In order to facilitate the grouping of cases in the IR-to-HLSL emit logic, the AST-to-IR lowering logic tries to detect cases where there are multiple `case`s in a row, and emit only a single label for them. One big/annoying gotcha is that we don't properly handle the case where a `default:` case has a non-trivial fall-throguh to another case. That seems fine for now since HLSL doesn't support fall-through anyway, but it probably needs to get detected somewhere in the Slang compiler (e.g., maybe we should add a diagnostic pass over the IR that detects target-specific problems like that and emits errors). * IR: Add support for empty statements. - Add empty statement in `lower-to-ir.cpp` - Go ahead and eliminate the statement catch-all and explicitly enumerate the cases we don't support - Fix up parser for block statements so that it doesn't leave a null statement as the body of a `{}` - Add an empty statement to one of the cases for the `switch` test, to ensure we are testing empty statements	14 November 2017, 19:33:36 UTC
7c4ad87	Yong He	14 November 2017, 02:31:40 UTC	Merge pull request #277 from csyonghe/ir-legalization IR Legalization of function parameters	14 November 2017, 02:31:40 UTC
38f0113	Yong He	13 November 2017, 23:39:44 UTC	Merge branch 'ir-legalization' of https://github.com/csyonghe/slang	13 November 2017, 23:39:44 UTC
fd056b0	Yong He	13 November 2017, 23:22:03 UTC	Legalization of function parameter types. This commit fixes issue #275 This commit includes following changes: 1. legalize function parameter IRParam instructions 2. legalize function parameter types in IRFuncType 3. legalize call sites (IRCall) with proper arguments 4. legalize local vars that has a mixed resource type.	13 November 2017, 23:39:17 UTC
81f295f	Yong He	13 November 2017, 23:38:38 UTC	remove unfinished test case	13 November 2017, 23:38:38 UTC
3dc6c1b	Yong He	13 November 2017, 23:37:30 UTC	fix merge bug	13 November 2017, 23:37:30 UTC
891c2c6	Yong He	13 November 2017, 23:35:22 UTC	fix merge bug	13 November 2017, 23:35:22 UTC
bbf58ba	Yong He	13 November 2017, 23:26:10 UTC	Merge branch 'ir-legalization' of https://github.com/csyonghe/slang	13 November 2017, 23:26:10 UTC
fe22b6f	Yong He	13 November 2017, 23:22:03 UTC	Legalization of function parameter types. This commit addresses issue #275 This commit includes following changes: 1. legalize function parameter IRParam instructions 2. legalize function parameter types in IRFuncType 3. legalize call sites (IRCall) with proper arguments 4. legalize local vars that has a mixed resource type.	13 November 2017, 23:25:41 UTC
57f737d	Yong He	13 November 2017, 23:22:03 UTC	Legalization of function parameter types. This commit addresses issue #275 This commit includes following changes: 1. legalize function parameter IRParam instructions 2. legalize function parameter types in IRFuncType 3. legalize call sites (IRCall) with proper arguments 4. legalize local vars that has a mixed resource type.	13 November 2017, 23:22:34 UTC
c9d9424	Tim Foley	13 November 2017, 22:17:09 UTC	Parameter block work (#276) * Don't auto-enable IR use for compute tests The `COMPARE_COMPUTE` and `COMPARE_RENDER_COMPUTE` test fixtures were set up to always enable the `-use-ir` flag on Slang, which precludes having any tests that confirm functionality on the old non-IR path (which is still required by our main customer). This change adds the `-xslang -use-ir` flags explicitly to any compute test cases that left them out, and makes the fixture no longer add it by default. * Continue building out parameter block support The initial front-end logic for parameter blocks was already added, but they are still missing a bunch of functionality. This change addresses some of the known issues: - Bug fix: don't try to emit HLSL `register` bindings for variables that consume whole register spaces/sets - Overhaul type layout logic so that it can make decisions based on a given code generation target (currently passed in as a `TargetRequest`), which allows us to decide whether or not a parameter block should get its own register set on a per-target basis. - Always use a register space/set for Vulkan - Never use a register space/set for HLSL SM 5.0 and lower - By default, don't use register spaces/sets for HLSL output - Add a command-line flag and some "target flags" to enable register-space usage for D3D targets - Hackily add initial support for parameter blocks in the AST-to-AST path - This just blindly lowers `ParameterBlock<T>` to `T`, which shouldn't quite work - A more complete overhaul will probably need to wait until the AST-to-AST legalization is changed to use the `LegalType`s from the IR legalization pass. - Add a compute-based test case to actually run code using parameter blocks - This file runs test cases both with and without the IR	13 November 2017, 22:17:09 UTC
c9368fe	Tim Foley	09 November 2017, 17:18:16 UTC	IR: Add support for break and continue statements (#272) * IR: Add support for break and continue statements The front-end is already doing the work of connecting this statements to their "parent" statement, so we just needed to build a map from the `Stmt` to the corresponding `IRBlock`s to use for break/continue when outputting any loop statement, and then look up in the map for the branch target when outputting a break/continue. When we get around to adding `switch` statements, the same pattern should work just fine. I also added support for `do/while` statements in IR codegen, and made sure to exercise those in one of the test cases I added. There is also an unrelated IR codegen fix for when there is a "bound subscript" on the RHS of an assignment. * IR: fix handling of do/while and continue Thanks to @csyonghe for pointing out my mistake in the earlier commit. I implemented `continue` for `do/while` loops incorrectly, branching to the head of the loop instead of the loop test. I'll try to blame this mistake on the fact that I never use `do/while` loops because I think they are awful. :) The fix for that issue wasn't too bad (see `lower-to-ir.cpp`) but it surfaces a much more serious issue: I wasn't actually implementing `continue` correctly at all when it comes to generating HLSL/GLSL from the IR (I can't easily make an excuse for that one). The basic issue at the heart of this is that given an input statement like: ``` for(int ii = 0; ii < N; ii = doSomething(ii)) { ... } ``` The continue clause (`ii = doSomething(ii)`) could expand into many instructions (across multiple blocks, if we inline), and there is in general no guarantee when we are done that we can package up that code as an expression and spit out a new `for` loop (the same basic argument applies to a `do { ... } while(someComplexExpression())`. So, if we assume that in general we have to generate a full statement for the `continue` clause, what can we emit? - We could try to "outline" the continue code into its own function, so that we can call it from an expression. That could work, but has high implementation complexity. - We could introduce additional `bool` variables for control flow, outputting something like: ``` bool useContinueBlock = false; for(;;) { if(useContinueBlock) { <CONTINUE CODE>; } useContinueBlock = true; <LOOP TEST> <LOOP BODY> } ``` This works but user might balk at the extra variable we introduce. - We could duplicate the code at each continue site. That is, we emit the loop as: ``` for(;;) { <LOOP TEST> <LOOP BODY> <CONTINUE CODE> } ``` but then whenever we'd like to emit `continue;` we instead emit `{ <CONTINUE CODE>; continue; }`. This doesn't introduce any extra variables, but it causes code duplication (limited, if we don't have too many `continue` sites, and the continue clause is small - which are the common cases). When I was initially working on the IR codegen I picked that last option just because it is what `fxc` seems to do, but I neglected to actually implement the special-case codegen for a `continue` instruction. This change addresses that (see `emit.cpp`). Finally, once things were fixed the `continue` test case produced the results Yong told me to expect, but it also produced a warning from the downstream HLSL compiler ("hey, your loop doesn't ever actually loop!"), so I reworked the test back to one that actually loops (but still tests `continue`). As a final aside in this essay of a commit message: the current IR representation of control flow uses special-case instructions for various cases of unconditional branch (and two variations on `if`), but these are not strictly necessary, and a future change will hopefully clean it up. The biggest catch in doing that is that it will require the IR->source codegen to carefully track which blocks represent which kinds of branch targets in context (e.g., you can't assume that a `continue` that nees the special handling above will appear as a distinct kind of instruction).	09 November 2017, 17:18:16 UTC
9a26ba8	Yong He	08 November 2017, 18:35:11 UTC	Merge pull request #267 from csyonghe/work Don't update generated .h file if its not changed.	08 November 2017, 18:35:11 UTC
da58c70	Yong He	08 November 2017, 17:55:08 UTC	Merge branch 'master' into work	08 November 2017, 17:55:08 UTC
d4aaa6c	Yong He	08 November 2017, 17:54:52 UTC	Merge pull request #268 from csyonghe/work1 Cleanup of "support generic interface method".	08 November 2017, 17:54:52 UTC
f408165	Yong He	08 November 2017, 17:08:13 UTC	Cleanup substitution of DeclaredSubtypeWitness. Now using DeclaredSubtypeWitness::declRef to determine the proper argument index in a GenericSubstitution.	08 November 2017, 17:08:13 UTC