Attachment 349046 Details for Bug 189326 – WIP

[patch] WIP

bug-189326-20180906112406.patch (text/plain), 4.83 KB, created by Thomas Denney on 2018-09-06 11:24:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Thomas Denney

Created: 2018-09-06 11:24:10 PDT

Size: 4.83 KB

patch

obsolete

>Subversion Revision: 235739
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 1a1e216449451bb436d1d73300f2df9e1161676a..69916acfbdedd2817f4476ead05f1658fe46fac0 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,20 @@
>+2018-09-05  Thomas Denney  <tdenney@apple.com>
>+
>+        [WHLSL] The interpreter should evaluate arguments right-to-left
>+        https://bugs.webkit.org/show_bug.cgi?id=189326
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The behavior of the interpreter now matches the specification with
>+        respect to evaluation order.
>+
>+        * WebGPUShadingLanguageRI/Evaluator.js: Correct the evaluation order.
>+        (Evaluator.prototype.visitFunctionLikeBlock): Note that the agruments
>+        must all be evaluated before they can be copied in.
>+        (Evaluator.prototype.visitCallExpression):
>+        (Evaluator):
>+        * WebGPUShadingLanguageRI/Test.js: Add test for evaluation order.
>+
> 2018-09-06  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         [WHLSL] The parser is too slow
>diff --git a/Tools/WebGPUShadingLanguageRI/Evaluator.js b/Tools/WebGPUShadingLanguageRI/Evaluator.js
>index aee0a2312fed72c0c87e73f18ac348ea1ac3c7ba..6e23eb8e9321d8391cdf73f0d86a7ded6a793db5 100644
>--- a/Tools/WebGPUShadingLanguageRI/Evaluator.js
>+++ b/Tools/WebGPUShadingLanguageRI/Evaluator.js
>@@ -74,7 +74,7 @@ class Evaluator extends Visitor {
>     
>     visitFunctionLikeBlock(node)
>     {
>-        for (let i = 0; i < node.argumentList.length; ++i) {
>+        for (let i = node.argumentList.length; i--;) {
>             node.parameters[i].ePtr.copyFrom(
>                 node.argumentList[i].visit(this),
>                 node.parameters[i].type.size);
>@@ -312,7 +312,7 @@ class Evaluator extends Visitor {
>     {
>         // We evaluate inlined ASTs, so this can only be a native call.
>         let callArguments = [];
>-        for (let i = 0; i < node.argumentList.length; ++i) {
>+        for (let i = node.argumentList.length; i--;) {
>             let argument = node.argumentList[i];
>             let type = node.func.parameterTypes[i];
>             if (!type || !argument)
>@@ -320,17 +320,9 @@ class Evaluator extends Visitor {
>             let argumentValue = argument.visit(this);
>             if (!argumentValue)
>                 throw new Error("Null argument value, i = " + i + ", node = " + node);
>-            callArguments.push(() => {
>-                let result = this._snapshot(type, null, argumentValue);
>-                return result;
>-            });
>+            callArguments.unshift(EBuffer.allowAllocation(() => this._snapshot(type, null, argumentValue)));
>         }
>-        
>-        // For simplicity, we allow intrinsics to just allocate new buffers, and we allocate new
>-        // buffers when snapshotting their arguments. This is not observable to the user, so it's OK.
>-        let result = EBuffer.allowAllocation(
>-            () => node.func.implementation(callArguments.map(thunk => thunk()), node));
>-        
>+        let result = EBuffer.allowAllocation(() => node.func.implementation(callArguments, node));
>         result = this._snapshot(node.func.returnType, node.resultEPtr, result);
>         return result;
>     }
>diff --git a/Tools/WebGPUShadingLanguageRI/Test.js b/Tools/WebGPUShadingLanguageRI/Test.js
>index 556cb2fb8bb5108790c5b4da8fa67e4c54cac666..9310d01967634b68068a1b7d4a3968053743e651 100644
>--- a/Tools/WebGPUShadingLanguageRI/Test.js
>+++ b/Tools/WebGPUShadingLanguageRI/Test.js
>@@ -7626,6 +7626,59 @@ tests.commentParsing = function() {
>         (e) => e instanceof WLexicalError);
> }
> 
>+tests.callArgumentsAreCopiedImmediatelyAfterEvaluation = () => {
>+    let program = doPrep(`
>+        int foo()
>+        {
>+            return *bar(5) + *bar(7);
>+        }
>+
>+        thread int* bar(int value)
>+        {
>+            int x = value;
>+            return &x;
>+        }
>+    `);
>+
>+    checkInt(program, callFunction(program, "foo", []), 12);
>+}
>+
>+tests.evaluationOrderForArguments = () => {
>+    const program = doPrep(`
>+        int evaluationOrderUserFunction()
>+        {
>+            return plus(bar(5), bar(7));
>+        }
>+
>+        thread int* bar(int value)
>+        {
>+            int x = value;
>+            return &x;
>+        }
>+
>+        int plus(thread int* x, thread int* y)
>+        {
>+            return *x + *y;
>+        }
>+
>+        int evaluationOrderNativeFunction()
>+        {
>+            int x;
>+            int y = functionWithSideEffect(&x, 2) + functionWithSideEffect(&y, 3);
>+            return x;
>+        }
>+
>+        int functionWithSideEffect(thread int* x, int newValue)
>+        {
>+            *x = newValue;
>+            return 1;
>+        }
>+    `);
>+
>+    checkInt(program, callFunction(program, "evaluationOrderUserFunction", []), 10);
>+    checkInt(program, callFunction(program, "evaluationOrderNativeFunction", []), 2);
>+}
>+
> okToTest = true;
> 
> let testFilter = /.*/; // run everything by default

Actions: View | Formatted Diff | Diff

Attachments on bug 189326: 348978 | 348982 | 349046