Attachment 349357 Details for Bug 188641 – Patch

[patch] Patch

bug-188641-20180910170133.patch (text/plain), 16.19 KB, created by Thomas Denney on 2018-09-10 17:01:34 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Thomas Denney

Created: 2018-09-10 17:01:34 PDT

Size: 16.19 KB

patch

obsolete

>Subversion Revision: 235753
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 7dfc1574c252f4ce48852bce58db7dfff9ee30f9..b30bcf6d22cff265cc5a7330d389857b1b9f1b05 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,41 @@
>+2018-09-10  Thomas Denney  <tdenney@apple.com>
>+
>+        [WHLSL] Inlining should be optional
>+        https://bugs.webkit.org/show_bug.cgi?id=188641
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Inlining functions is now optional, and disabled by default. This patch
>+        additionally resolves https://bugs.webkit.org/show_bug.cgi?id=189326 as
>+        the code for calling functions was completely rewritten.
>+
>+        * WebGPUShadingLanguageRI/All.js: Add LateCheckAndLayoutBuffers.js
>+        * WebGPUShadingLanguageRI/CallFunction.js:
>+        (callFunction): Remove dependency on inlining.
>+        * WebGPUShadingLanguageRI/Evaluator.js:
>+        (Evaluator.prototype._snapshot): Allow null srcPtr in the case of void
>+        returns from functions.
>+        (Evaluator.prototype._evaluateArguments): Abstract out argument
>+        evaluation, ensuring that the order is right-to-left for
>+        FunctionLikeBlocks (i.e. inlined functions) and regular calls.
>+        (Evaluator.prototype._evaluateFunction): Abstracted out function body
>+        evaluation for inlined and non-inlined functions.
>+        (Evaluator.prototype.visitFunctionLikeBlock): Ditto.
>+        (Evaluator.prototype.visitCallExpression): Ditto.
>+        * WebGPUShadingLanguageRI/Inline.js:
>+        (_inlineFunction): Moved logic that is needed regardless of inlining
>+        into LateCheckAndLayoutBuffers.js
>+        (resolveInlinedFunction): Deleted.
>+        * WebGPUShadingLanguageRI/LateCheckAndLayoutBuffers.js: Moved logic for
>+        late checking.
>+        * WebGPUShadingLanguageRI/Prepare.js: Make inlining optional and off by
>+        default.
>+        * WebGPUShadingLanguageRI/SPIRV.html: Update include.
>+        * WebGPUShadingLanguageRI/Test.html: Ditto.
>+        * WebGPUShadingLanguageRI/Test.js: Right-to-left argument evaluation,
>+        and inlining tests.
>+        * WebGPUShadingLanguageRI/index.html: Update include.
>+
> 2018-09-10  Don Olmstead  <don.olmstead@sony.com>
> 
>         CBOR coders should only be compiled if WebAuthN is enabled
>diff --git a/Tools/WebGPUShadingLanguageRI/All.js b/Tools/WebGPUShadingLanguageRI/All.js
>index 0a5885161a403d09aeaa1fe5e1043adfe612565f..3c2c48742001115a1713483a1f23ce28668bb9c6 100644
>--- a/Tools/WebGPUShadingLanguageRI/All.js
>+++ b/Tools/WebGPUShadingLanguageRI/All.js
>@@ -102,6 +102,7 @@ load("Inliner.js");
> load("IntLiteral.js");
> load("IntLiteralType.js");
> load("Intrinsics.js");
>+load("LateCheckAndLayoutBuffers.js");
> load("LateChecker.js");
> load("Lexer.js");
> load("LexerToken.js");
>diff --git a/Tools/WebGPUShadingLanguageRI/CallFunction.js b/Tools/WebGPUShadingLanguageRI/CallFunction.js
>index 7aaf0ba2249ff27ed817c8dc8e772c25da1bb5dc..72f674e7980fa85507492582aaed93ffa63018cc 100644
>--- a/Tools/WebGPUShadingLanguageRI/CallFunction.js
>+++ b/Tools/WebGPUShadingLanguageRI/CallFunction.js
>@@ -28,12 +28,11 @@
> function callFunction(program, name, argumentList)
> {
>     let argumentTypes = argumentList.map(argument => argument.type);
>-    let funcOrFailures = resolveInlinedFunction(program, name, argumentTypes, true);
>-    if (!(funcOrFailures instanceof Func)) {
>-        let failures = funcOrFailures;
>-        throw new WTypeError("<callFunction>", "Cannot resolve function call " + name + "(" + argumentList + ")" + (failures.length ? "; tried:\n" + failures.join("\n") : ""));
>-    }
>-    let func = funcOrFailures;
>+    let overload = program.globalNameContext.resolveFuncOverload(name, argumentTypes, null, true);
>+    if (!overload.func)
>+        throw new WTypeError("<callFunction>", "Cannot resolve function call " + name + "(" + argumentList + ")" + (overload.failures.length ? "; tried:\n" + overload.failures.join("\n") : ""));
>+
>+    const func = overload.func;
>     for (let i = 0; i < func.parameters.length; ++i) {
>         let type = argumentTypes[i];
>         type.visit(new StructLayoutBuilder());
>diff --git a/Tools/WebGPUShadingLanguageRI/Evaluator.js b/Tools/WebGPUShadingLanguageRI/Evaluator.js
>index 920987aeac494de5c399d020d461dcd9220b726f..3b8094268f824fab4d43b9eeaa5006781cc55956 100644
>--- a/Tools/WebGPUShadingLanguageRI/Evaluator.js
>+++ b/Tools/WebGPUShadingLanguageRI/Evaluator.js
>@@ -37,6 +37,8 @@ class Evaluator extends Visitor {
>     // possible for a pointer returned from a visit method in rvalue context to live across any effects.
>     _snapshot(type, dstPtr, srcPtr)
>     {
>+        if (!srcPtr)
>+            return null;
>         let size = type.size;
>         if (size == null)
>             throw new Error("Cannot get size of type: " + type + " (size = " + size + ", constructor = " + type.constructor.name + ")");
>@@ -71,16 +73,34 @@ class Evaluator extends Visitor {
>             throw e;
>         }
>     }
>-    
>-    visitFunctionLikeBlock(node)
>+
>+    _evaluateArguments(argumentList, parameterList)
>     {
>-        for (let i = 0; i < node.argumentList.length; ++i) {
>-            node.parameters[i].ePtr.copyFrom(
>-                node.argumentList[i].visit(this),
>-                node.parameters[i].type.size);
>+        const callArguments = [];
>+        for (let i = argumentList.length; i--;) {
>+            const argument = argumentList[i];
>+            const type = parameterList[i].type;
>+            if (!type || !argument)
>+                throw new Error("Cannot get type or argument; i = " + i + ", argument = " + argument + ", type = " + type);
>+            let argumentValue = argument.visit(this);
>+            if (!argumentValue)
>+                throw new Error("Null argument value, i = " + i + ", node = " + node);
>+            callArguments.unshift(EBuffer.allowAllocation(() => this._snapshot(type, null, argumentValue)));
>         }
>-        let result = this._runBody(node.returnType, node.returnEPtr, node.body);
>-        return result;
>+        return callArguments;
>+    }
>+
>+    _evaluateFunction(node, argumentList, parameterList, funcBody, returnType, returnEPtr)
>+    {
>+        const argumentValues = this._evaluateArguments(argumentList, parameterList);
>+        for (let i = 0; i < node.argumentList.length; ++i)
>+            parameterList[i].ePtr.copyFrom(argumentValues[i], parameterList[i].type.size);
>+        return EBuffer.allowAllocation(() => this._runBody(returnType, returnEPtr, funcBody));
>+    }
>+
>+    visitFunctionLikeBlock(node)
>+    {
>+        return this._evaluateFunction(node, node.argumentList, node.parameters, node.body, node.returnType, node.returnEPtr);
>     }
>     
>     visitReturn(node)
>@@ -310,21 +330,11 @@ class Evaluator extends Visitor {
>     
>     visitCallExpression(node)
>     {
>-        // We evaluate inlined ASTs, so this can only be a native call.
>-        let callArguments = [];
>-        for (let i = 0; i < node.argumentList.length; ++i) {
>-            let argument = node.argumentList[i];
>-            let type = node.func.parameterTypes[i];
>-            if (!type || !argument)
>-                throw new Error("Cannot get type or argument; i = " + i + ", argument = " + argument + ", type = " + type + "; in " + node);
>-            let argumentValue = argument.visit(this);
>-            if (!argumentValue)
>-                throw new Error("Null argument value, i = " + i + ", node = " + node);
>-            callArguments.push(EBuffer.allowAllocation(() => this._snapshot(type, null, argumentValue)));
>-        }
>-        let result = EBuffer.allowAllocation(() => node.func.implementation(callArguments, node));
>-        result = this._snapshot(node.func.returnType, node.resultEPtr, result);
>-        return result;
>+        if (node.func instanceof NativeFunc) {
>+            const callArguments = this._evaluateArguments(node.argumentList, node.func.parameters);
>+            return EBuffer.allowAllocation(() => this._snapshot(node.func.returnType, node.resultEPtr, node.func.implementation(callArguments, node)));
>+        } else
>+            return this._evaluateFunction(node, node.argumentList, node.func.parameters, node.func.body, node.func.returnType, node.resultEPtr);
>     }
> }
> 
>diff --git a/Tools/WebGPUShadingLanguageRI/Inline.js b/Tools/WebGPUShadingLanguageRI/Inline.js
>index 8ba9ad74e7c2a70367225214719a61fb7fbfa3c5..d35881c1bd03880193fa0fcc3201e0d888f2fddf 100644
>--- a/Tools/WebGPUShadingLanguageRI/Inline.js
>+++ b/Tools/WebGPUShadingLanguageRI/Inline.js
>@@ -36,26 +36,6 @@ function _inlineFunction(program, func, visiting)
> {
>     if (func.inlined || func.isNative)
>         return;
>-    
>-    func.visit(new LateChecker());
>-    
>-    // This is the precise time when we can build EBuffers in order to get them to be uniqued by
>-    // type instantiation but nothing else.
>-    func.visit(new StructLayoutBuilder());
>-    func.visit(new EBufferBuilder(program));
>-    
>     func.rewrite(new Inliner(program, func, visiting));
>-
>     func.inlined = true;
> }
>-
>-function resolveInlinedFunction(program, name, argumentTypes, allowEntryPoint = false)
>-{
>-    let overload = program.globalNameContext.resolveFuncOverload(name, argumentTypes, undefined, allowEntryPoint);
>-    if (!overload.func)
>-        return overload.failures;
>-
>-    let func = overload.func;
>-    _inlineFunction(program, func, new VisitingSet(func));
>-    return func;
>-}
>diff --git a/Tools/WebGPUShadingLanguageRI/LateCheckAndLayoutBuffers.js b/Tools/WebGPUShadingLanguageRI/LateCheckAndLayoutBuffers.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..f5b35bb80e14d16e26031a70af1c9c386b05c641
>--- /dev/null
>+++ b/Tools/WebGPUShadingLanguageRI/LateCheckAndLayoutBuffers.js
>@@ -0,0 +1,37 @@
>+/*
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+"use strict";
>+
>+function lateCheckAndLayoutBuffers(program)
>+{
>+    for (let funcList of program.functions.values()) {
>+        for (let func of funcList) {
>+            if (func.isNative)
>+                continue;
>+            func.visit(new LateChecker());
>+            func.visit(new EBufferBuilder());
>+        }
>+    }
>+}
>\ No newline at end of file
>diff --git a/Tools/WebGPUShadingLanguageRI/Prepare.js b/Tools/WebGPUShadingLanguageRI/Prepare.js
>index 834592d36f3457dfa86b5751bb7262472a3f0a2d..13ebde80bd91ccb65468681292526334dd3887e8 100644
>--- a/Tools/WebGPUShadingLanguageRI/Prepare.js
>+++ b/Tools/WebGPUShadingLanguageRI/Prepare.js
>@@ -26,7 +26,7 @@
> 
> let prepare = (() => {
>     let standardProgram;
>-    return function(origin, lineNumberOffset, text) {
>+    return function(origin, lineNumberOffset, text, shouldInline = false) {
>         if (!standardProgram) {
>             standardProgram = new Program();
>             let firstLineOfStandardLibrary = 28; // See StandardLibrary.js.
>@@ -68,8 +68,9 @@ let prepare = (() => {
>         checkTypesWithArguments(program);
>         findHighZombies(program);
>         program.visit(new StructLayoutBuilder());
>-        inline(program);
>-        
>+        lateCheckAndLayoutBuffers(program);
>+        if (shouldInline)
>+            inline(program);
>         return program;
>     };
> })();
>diff --git a/Tools/WebGPUShadingLanguageRI/SPIRV.html b/Tools/WebGPUShadingLanguageRI/SPIRV.html
>index 804cd9abda6d5c3bec03be2582aa3f23f007a188..0d2371005e710cdc60f033da5a511c49ae005c36 100644
>--- a/Tools/WebGPUShadingLanguageRI/SPIRV.html
>+++ b/Tools/WebGPUShadingLanguageRI/SPIRV.html
>@@ -85,6 +85,7 @@ td {
>     <script src="IntLiteral.js"></script>
>     <script src="IntLiteralType.js"></script>
>     <script src="Intrinsics.js"></script>
>+    <script src="LateCheckAndLayoutBuffers.js"></script>
>     <script src="LateChecker.js"></script>
>     <script src="Lexer.js"></script>
>     <script src="LexerToken.js"></script>
>diff --git a/Tools/WebGPUShadingLanguageRI/Test.html b/Tools/WebGPUShadingLanguageRI/Test.html
>index f5ea3d90198dcdb87131f92c250c743304b66ac0..c12e5d99dc74a6f1901c95df91d34a05ae855e1c 100644
>--- a/Tools/WebGPUShadingLanguageRI/Test.html
>+++ b/Tools/WebGPUShadingLanguageRI/Test.html
>@@ -80,6 +80,7 @@
> <script src="IntLiteralType.js"></script>
> <script src="Intrinsics.js"></script>
> <script src="LateChecker.js"></script>
>+<script src="LateCheckAndLayoutBuffers.js"></script>
> <script src="Lexer.js"></script>
> <script src="LexerToken.js"></script>
> <script src="LiteralTypeChecker.js"></script>
>diff --git a/Tools/WebGPUShadingLanguageRI/Test.js b/Tools/WebGPUShadingLanguageRI/Test.js
>index 1da9adfddff16f89b925690487d8771f9a28bb65..21593bba9ea2de7ba798cb50f7faee165b8dc7a8 100644
>--- a/Tools/WebGPUShadingLanguageRI/Test.js
>+++ b/Tools/WebGPUShadingLanguageRI/Test.js
>@@ -7643,6 +7643,82 @@ tests.callArgumentsAreCopiedImmediatelyAfterEvaluation = () => {
>     checkInt(program, callFunction(program, "foo", []), 12);
> };
> 
>+tests.evaluationOrderForArguments = () => {
>+    const program = doPrep(`
>+        test int foo() { return *bar(10) + *bar(20); }
>+
>+        thread int* bar(int value)
>+        {
>+            int x = value;
>+            return &x;
>+        }
>+
>+        test int baz() { return plus(bar(10), bar(20)); }
>+
>+        int plus(thread int* x, thread int* y)
>+        {
>+            return *x + *y;
>+        }
>+    `);
>+    checkInt(program, callFunction(program, "foo", []), 30);
>+    checkInt(program, callFunction(program, "baz", []), 20);
>+}
>+
>+tests.cannotCallAnotherEntryPoint = () => {
>+    checkFail(() => doPrep(`
>+        struct Foo { int x; }
>+        vertex Foo foo() { return bar(); }
>+        vertex Foo bar() { return Foo(); }
>+    `), e => e instanceof WTypeError && e.message.indexOf("it cannot be called from within an existing shader") !== -1);
>+}
>+
>+tests.inliningDoesNotAffectNestedCalls = () => {
>+    const program = prepare("/internal/test", 0, `
>+        test int foo()
>+        {
>+            return plus(bar(), baz());
>+        }
>+
>+        int plus(int a, int b)
>+        {
>+            return a + b;
>+        }
>+
>+        int bar()
>+        {
>+            return 17;
>+        }
>+
>+        int baz()
>+        {
>+            return 3;
>+        }
>+    `, true);
>+    checkInt(program, callFunction(program, "foo", []), 20)
>+}
>+
>+tests.inliningDoesntProduceAliasingIssues = () => {
>+    const program = prepare("/internal/test", 0, `
>+        test int foo()
>+        {
>+            return bar(2);
>+        }
>+
>+        int bar(int x)
>+        {
>+            int y = x * x;
>+            return baz(y);
>+        }
>+
>+        int baz(int x)
>+        {
>+            int y = 4 * x;
>+            return x + y;
>+        }
>+    `, true);
>+    checkInt(program, callFunction(program, "foo", []), 20);
>+}
>+
> okToTest = true;
> 
> let testFilter = /.*/; // run everything by default
>diff --git a/Tools/WebGPUShadingLanguageRI/index.html b/Tools/WebGPUShadingLanguageRI/index.html
>index dfc90c4c2394395458865c4a8d65486870c26234..24415f33a1709b2e0e6d5bece67aa895306b89be 100644
>--- a/Tools/WebGPUShadingLanguageRI/index.html
>+++ b/Tools/WebGPUShadingLanguageRI/index.html
>@@ -79,6 +79,7 @@
> <script src="IntLiteral.js"></script>
> <script src="IntLiteralType.js"></script>
> <script src="Intrinsics.js"></script>
>+<script src="LateCheckAndLayoutBuffers.js"></script>
> <script src="LateChecker.js"></script>
> <script src="Lexer.js"></script>
> <script src="LexerToken.js"></script>

Actions: View | Formatted Diff | Diff

Attachments on bug 188641: 349039 | 349352 | 349357