summaryrefslogtreecommitdiffstats
path: root/tests/compute
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2018-10-11 09:20:10 -0700
committerGitHub <noreply@github.com>2018-10-11 09:20:10 -0700
commitdcd9f78b972a4b7b88e9c3403bd1e887d628b40a (patch)
treeb476d07018b85a21f04f973d1d93f4c21fe30ce3 /tests/compute
parent879ec1b385d290a4375682ec613a9e7a1967fc7d (diff)
Add basic support for [mutating] methods (#667)
By default, when writing a "method" (aka "member function") in Slang, the `this` parameter is implicitly an `in` parameter. So this: ```hlsl struct Foo { int state; int getState() { return state; } void setState(int s) { state = s; } }; ``` is desugared into something like this: ```hlsl struct Foo { int state }; int Foo_getState(Foo this) { return this.state; } // BAD: void Foo_setState(Foo this, int s) { this.state = s; } ``` That "setter" doesn't really do what was intended. It modifies a local copy of type `Foo`, because `in` parameters in HLSL represent by-value copy-in semantics, and are mutable in the body the function. Slang was updated to give a static error on the original code to catch this kind of mistake (so that `this` parameters are unlike ordinary function parameters, and no longer mutable). Of course, sometimes users *want* a mutable `this` parameter. Rather than make a mutable `this` the default (there are arguments both for and against this), this change adds a new attribute `[mutating]` that can be put on a method (member function) to indicate that its `this` parameter should be an `in out` parameter: ```hlsl [mutating] void setState(int s) { state = s; } ``` The above will translate to, more or less: ```hlsl void Foo_setState(inout Foo this, int s) { this.state = s; } ``` One added detail is that `[mutating]` can also be used on interface requirements, with the same semantics. A `[mutating]` requirement can be satisfied with a `[mutating]` or non-`[mutating]` method, while a non-`[mutating]` requirement can't be satisfied with a `[mutating]` method (the call sites would not expect mutation to happen). The design of `[mutating]` here is heavily influenced by the equivalent `mutating` keyword in Swift. Notes on the implementation: * Adding the new attribute was straightforward using the existing support, but I had to change around where attributes get checked in the overall sequencing of static checks, because attributes were being checked *after* function bodies, but with this change I need to look at semantically-checked attributes to determine the mutability of `this` * The check to restrict it so that `[mutating]` methods cannot satisfy non-`[mutating]` requirements was easy to add, but it points out the fact that there is a huge TODO comment where the actual checking of method *signatures* is supposed to happen. That is a bug waiting to bite users and needs to be fixed! * While we had special-case logic to detect attempts to modify state accessed through an immutable `this` (e.g., `this.state = s`), that logic didn't trigger when the mutation happened through a function/operator call (e.g., `this.state += s`), so this change factors out the validation logic for that case and calls through to it from both the assignment and `out` argument cases. * The error message for the special-case check was updated to note that the user could apply `[mutating]` to their function declaration to get rid of the error. * The semantic checking logic for an explicit `this` expression was already walking up through the scopes (created during parsing) and looking for a scope that represents an outer type declaration that `this` might be referring to. We simply extend it to note when it passes through the scope for a function or similar declaration (`FunctionDeclBase`) and check for the `[mutating]` attribute. If the attribute is seen, it returns a mutable `this` expression, and otherwise leaves it immutable. * The IR lowering logic then needed to be updated so that when adding an IR-level parameter to represent `this`, it gives it the appropriate "direction" based on the attributes of the function declaration being lowered. The rest of the IR logic works as-is, because it will treat `this` just like an other parameter (whether it is `in` or `inout`). * This biggest chunk of work was the "implicit `this`" case, because ordinary name lookup may resolve an expression like `state` into `this.state`, so that the `this` expression comes out of "thin air." To handle this case, I extended the structure of the "breadcrumbs" that come along with a lookup result (the breadcrumbs are used for any case where a single identifier like `state` needs to be embellished to a more complex expression as a result of lookup), so that it can identify whether a `Breadcrumb::Kind::This` node comes from a `[mutating]` context or not. Similar to the logic for an explicit `this`, we handle this by noting when we pass through a `FunctionDeclBase` when moving up through scopes, and look for the `[mutating]` attribute on it. The rest of the work was just plumbing the additional state through.
Diffstat (limited to 'tests/compute')
-rw-r--r--tests/compute/mutating-methods.slang51
-rw-r--r--tests/compute/mutating-methods.slang.expected.txt4
2 files changed, 55 insertions, 0 deletions
diff --git a/tests/compute/mutating-methods.slang b/tests/compute/mutating-methods.slang
new file mode 100644
index 000000000..736d4f7e0
--- /dev/null
+++ b/tests/compute/mutating-methods.slang
@@ -0,0 +1,51 @@
+// mutating-methods.slang
+//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute
+//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12
+//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute
+
+interface IAccumulator
+{
+ [mutating] void accumulate(int v);
+}
+
+struct Accumulator : IAccumulator
+{
+ int state;
+
+ [mutating] void accumulate(int v)
+ {
+ state += v;
+ }
+
+ [mutating] void accumulateMore(int v)
+ {
+ this.state += v;
+ }
+}
+
+void doStuff<A : IAccumulator>(inout A a)
+{
+ a.accumulate(16);
+}
+
+int test(int x)
+{
+ Accumulator a;
+ a.state = 0;
+
+ a.accumulate(x);
+ a.accumulateMore(x);
+ doStuff(a);
+
+ return a.state;
+}
+
+//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out
+RWStructuredBuffer<int> outputBuffer;
+
+[numthreads(4, 1, 1)]
+void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
+{
+ int tid = dispatchThreadID.x;
+ outputBuffer[tid] = test(tid);
+}
diff --git a/tests/compute/mutating-methods.slang.expected.txt b/tests/compute/mutating-methods.slang.expected.txt
new file mode 100644
index 000000000..128492da7
--- /dev/null
+++ b/tests/compute/mutating-methods.slang.expected.txt
@@ -0,0 +1,4 @@
+10
+12
+14
+16