Skip to content

Commit 69f7d28

Browse files
author
Tim Foley
authored
Add a basc inlining facility for use in the stdlib (shader-slang#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.
1 parent 935768c commit 69f7d28

16 files changed

+611
-14
lines changed

source/slang/core.meta.slang

+8-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,12 @@ interface __FlagsEnumType : __EnumType
113113
// argument. The left-to-right evaluation order guaranteed by Slang then ensures that
114114
// `left` is evaluated before `right`.
115115
//
116-
__generic<T,U> __intrinsic_op($(kCompoundIntrinsicOp_Sequence)) U operator,(T left, U right);
116+
__generic<T,U>
117+
[__unsafeForceInlineEarly]
118+
U operator,(T left, U right)
119+
{
120+
return right;
121+
}
117122

118123
// The ternary `?:` operator does not short-circuit in HLSL, and Slang continues to
119124
// follow that definition, so that this operator is effectively just an ordinary
@@ -1773,3 +1778,5 @@ attribute_syntax [allow(diagnostic: String)] : AllowAttribute;
17731778
__attributeTarget(Decl)
17741779
attribute_syntax [__extern] : ExternAttribute;
17751780

1781+
__attributeTarget(FunctionDeclBase)
1782+
attribute_syntax [__unsafeForceInlineEarly] : UnsafeForceInlineEarlyAttribute;

source/slang/slang-compound-intrinsics.h

-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
M(PreDec) \
3030
M(PostInc) \
3131
M(PostDec) \
32-
M(Sequence) \
3332
M(AddAssign) \
3433
M(SubAssign) \
3534
M(MulAssign) \

source/slang/slang-ir-clone.cpp

+20-1
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,26 @@ IRInst* cloneInst(
228228
SLANG_ASSERT(builder);
229229
SLANG_ASSERT(oldInst);
230230

231-
auto newInst = cloneInstAndOperands(
231+
IRInst* newInst = nullptr;
232+
if( env->mapOldValToNew.TryGetValue(oldInst, newInst) )
233+
{
234+
// In this case, somebody is trying to clone an
235+
// instruction that already had been cloned
236+
// (e.g., trying to clone a `param` in a function
237+
// body that had already been mapped to a specialization)
238+
// so we will make the operation safer and more
239+
// convenient by just returning the registered value.
240+
//
241+
// TODO: There might be cases where the client doesn't
242+
// want this convenience feature (because it could
243+
// accidentally mask a bug), so we should consider
244+
// having two versions of `cloneInst()` with one
245+
// explicitly not including this feature.
246+
//
247+
return newInst;
248+
}
249+
250+
newInst = cloneInstAndOperands(
232251
env, builder, oldInst);
233252

234253
env->mapOldValToNew.Add(oldInst, newInst);

0 commit comments

Comments
 (0)