diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2020-03-11 12:53:09 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-03-11 12:53:09 -0700 |
| commit | 69f7d288313eb238bfb42943694dfcd9bb911d3e (patch) | |
| tree | 3cc81aab5bddd2c7d09f742471cc6d49fcb648b2 /source/slang/slang-ir-clone.cpp | |
| parent | 935768c6a00c258bf5122a2d04b84064a1eee67d (diff) | |
Add a basc inlining facility for use in the stdlib (#1271)
The main feature visible to the stdlib here is the `[__unsafeForceInlineEarly]` attribute, which can be attached to a function definition and forces calls to that function to be inlined immediately after initial IR lowering.
The pass is implemented in `slang-ir-inline.{h,cpp}` and currently only handles the completely trivial case of a function with no control flow that ends with a single `return`. The lack of support for any other cases motivates the `__unsafe` prefix on the attribute.
In order to test that the pass works, I modified the "comma operator" in the standard library to be defined directly (rather than relying on special-case handling in IR lowering), and then added a test that uses that operator to make sure it generates code as expected. The compute version of the test confirms that we generate semantically correct code for the operator, while the SPIR-V cross-compilation test confirms that our output matches GLSL where the comma operator has been inlined, rather than turned into a subroutine.
Notes for the future:
* With this change it should be possible (in principle) to redefine all the compound operators in the stdlib to instead be ordinary functions with the new attribute, removing the need for `slang-compound-intrinsics.h`.
* Once the compound intrinsics are defined in the stdlib, it should be easier/possible to start making built-in operators like `+` be ordinary functions from the standpoint of the IR
* The attribute and pass here could be extended to include an alternative inlining attribute that happens later in compilation (after linking) but otherwise works the same. This could in theory be used for functions where we don't want to inline the definition into generated IR, but still want to inline things berfore generating final HlSL/GLSL/whatever.
* The inlining pass itself could be generalized to work for less trivial functions pretty easily; for the most part it would just mean "splitting" the block with the call site and then inserting clones of the blocks in the callee. Any `return` instructions in the clone would become unconditional branches (with arguments) to the block after the call (which would get a parameter to represent the returned value).
* The "hard" part for such an inlining pass would be handling cases where the control flow that results from inlining can't be handled by our later restructuring passes. The long-term fix there is to implement something like the "relooper" algorithm to restructure control flow as required for specific targets.
Diffstat (limited to 'source/slang/slang-ir-clone.cpp')
| -rw-r--r-- | source/slang/slang-ir-clone.cpp | 21 |
1 files changed, 20 insertions, 1 deletions
diff --git a/source/slang/slang-ir-clone.cpp b/source/slang/slang-ir-clone.cpp index df0555f9b..9f9349f99 100644 --- a/source/slang/slang-ir-clone.cpp +++ b/source/slang/slang-ir-clone.cpp @@ -228,7 +228,26 @@ IRInst* cloneInst( SLANG_ASSERT(builder); SLANG_ASSERT(oldInst); - auto newInst = cloneInstAndOperands( + IRInst* newInst = nullptr; + if( env->mapOldValToNew.TryGetValue(oldInst, newInst) ) + { + // In this case, somebody is trying to clone an + // instruction that already had been cloned + // (e.g., trying to clone a `param` in a function + // body that had already been mapped to a specialization) + // so we will make the operation safer and more + // convenient by just returning the registered value. + // + // TODO: There might be cases where the client doesn't + // want this convenience feature (because it could + // accidentally mask a bug), so we should consider + // having two versions of `cloneInst()` with one + // explicitly not including this feature. + // + return newInst; + } + + newInst = cloneInstAndOperands( env, builder, oldInst); env->mapOldValToNew.Add(oldInst, newInst); |
