Attachment 349045 Details for Bug 189360 – Patch

[patch] Patch

bug-189360-20180906110015.patch (text/plain), 3.18 KB, created by Thomas Denney on 2018-09-06 11:00:18 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Thomas Denney

Created: 2018-09-06 11:00:18 PDT

Size: 3.18 KB

patch

obsolete

>Subversion Revision: 235739
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 1a1e216449451bb436d1d73300f2df9e1161676a..45fca89dc3a2c5edc1d20f169a8a8c304bb17f53 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-09-06  Thomas Denney  <tdenney@apple.com>
>+
>+        [WHLSL] Call arguments should be copied as soon as they are evaluated
>+        https://bugs.webkit.org/show_bug.cgi?id=189360
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Previously all call arguments were evaluated and then their results were
>+        copied into new buffers for the call. However, the results are not
>+        necessarily independent, so the result should be copied immediately
>+        after evaluation.
>+
>+        * WebGPUShadingLanguageRI/Evaluator.js:
>+        (Evaluator.prototype.visitCallExpression): Move location of copy.
>+        (Evaluator):
>+        * WebGPUShadingLanguageRI/Test.js: Add new test to verify correct
>+        behavior.
>+
> 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..920987aeac494de5c399d020d461dcd9220b726f 100644
>--- a/Tools/WebGPUShadingLanguageRI/Evaluator.js
>+++ b/Tools/WebGPUShadingLanguageRI/Evaluator.js
>@@ -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.push(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..bd2b036adae1abb782a7578a6bca3b519636e1df 100644
>--- a/Tools/WebGPUShadingLanguageRI/Test.js
>+++ b/Tools/WebGPUShadingLanguageRI/Test.js
>@@ -7626,6 +7626,23 @@ 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);
>+};
>+
> okToTest = true;
> 
> let testFilter = /.*/; // run everything by default

Actions: View | Formatted Diff | Diff

Attachments on bug 189360: 349045