Attachment 373438 Details for Bug 199474 – Patch

[patch] Patch

bug-199474-20190703165717.patch (text/plain), 19.00 KB, created by Robin Morisset on 2019-07-03 16:57:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Robin Morisset

Created: 2019-07-03 16:57:17 PDT

Size: 19.00 KB

patch

obsolete

>Subversion Revision: 247115
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e5314278b1683f86214441f57ae760daf61d378f..a127bd5bec25c38011488dc6044c938e289aeb94 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2019-07-03  Robin Morisset  <rmorisset@apple.com>
>+
>+        [WHLSL] Remove the phase resolveCallsInFunctions
>+        https://bugs.webkit.org/show_bug.cgi?id=199474
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This pass only stores into each property access and call expression vectors of all the functions it might be calling, for use by the Checker afterwards.
>+        But the checker is perfectly able to compute a pointer to these vectors by itself.
>+        So by removing this pass, we gain the following:
>+        - One less pass over the AST
>+        - No need to copy these vectors (which can be large for heavily overloaded functions, of which there are quite a few in the stdlib)
>+        - No need to have these vectors in the expressions, saving 24 bytes per CallExpression and 72 bytes per PropertyAccessExpression
>+        - No need to allocate and then destroy these vectors.
>+
>+        No new tests as there is no intended functional change.
>+
>+        * Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h:
>+        (WebCore::WHLSL::AST::CallExpression::castReturnType):
>+        * Modules/webgpu/WHLSL/AST/WHLSLPropertyAccessExpression.h:
>+        * Modules/webgpu/WHLSL/WHLSLAutoInitializeVariables.cpp:
>+        (WebCore::WHLSL::AutoInitialize::visit):
>+        * Modules/webgpu/WHLSL/WHLSLChecker.cpp:
>+        (WebCore::WHLSL::resolveFunction):
>+        (WebCore::WHLSL::Checker::finishVisiting):
>+        (WebCore::WHLSL::Checker::visit):
>+        * Modules/webgpu/WHLSL/WHLSLNameResolver.cpp:
>+        (WebCore::WHLSL::NameResolver::NameResolver):
>+        (WebCore::WHLSL::NameResolver::visit):
>+        * Modules/webgpu/WHLSL/WHLSLNameResolver.h:
>+        * Modules/webgpu/WHLSL/WHLSLPrepare.cpp:
>+        (WebCore::WHLSL::prepareShared):
>+
> 2019-07-02  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         [WHLSL] Standard library is too big to directly include in WebCore
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h b/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h
>index 17b94cdca071ce9a042970df7986399494855755..206b02a264694517fea12ccf67607b2bb72aeeec 100644
>--- a/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h
>@@ -67,13 +67,6 @@ public:
> 
>     bool isCast() { return m_castReturnType; }
>     NamedType* castReturnType() { return m_castReturnType; }
>-    bool hasOverloads() const { return static_cast<bool>(m_overloads); }
>-    Optional<Vector<std::reference_wrapper<FunctionDeclaration>, 1>>& overloads() { return m_overloads; }
>-    void setOverloads(const Vector<std::reference_wrapper<FunctionDeclaration>, 1>& overloads)
>-    {
>-        ASSERT(!hasOverloads());
>-        m_overloads = overloads;
>-    }
> 
>     FunctionDeclaration& function()
>     {
>@@ -90,7 +83,6 @@ public:
> private:
>     String m_name;
>     Vector<UniqueRef<Expression>> m_arguments;
>-    Optional<Vector<std::reference_wrapper<FunctionDeclaration>, 1>> m_overloads;
>     FunctionDeclaration* m_function { nullptr };
>     NamedType* m_castReturnType { nullptr };
> };
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLPropertyAccessExpression.h b/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLPropertyAccessExpression.h
>index bb0c3012eeb203903a430020328c8b94829b684d..34d12a1e9dd9b6c36211d662af4b971b9b77e9a3 100644
>--- a/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLPropertyAccessExpression.h
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLPropertyAccessExpression.h
>@@ -57,27 +57,11 @@ public:
>     virtual String setterFunctionName() const = 0;
>     virtual String anderFunctionName() const = 0;
> 
>-    Vector<std::reference_wrapper<FunctionDeclaration>, 1>& possibleGetterOverloads() { return m_possibleGetterOverloads; }
>-    Vector<std::reference_wrapper<FunctionDeclaration>, 1>& possibleSetterOverloads() { return m_possibleSetterOverloads; }
>-    Vector<std::reference_wrapper<FunctionDeclaration>, 1>& possibleAnderOverloads() { return m_possibleAnderOverloads; }
>     FunctionDeclaration* getterFunction() { return m_getterFunction; }
>     FunctionDeclaration* anderFunction() { return m_anderFunction; }
>     FunctionDeclaration* threadAnderFunction() { return m_threadAnderFunction; }
>     FunctionDeclaration* setterFunction() { return m_setterFunction; }
> 
>-    void setPossibleGetterOverloads(const Vector<std::reference_wrapper<FunctionDeclaration>, 1>& overloads)
>-    {
>-        m_possibleGetterOverloads = overloads;
>-    }
>-    void setPossibleAnderOverloads(const Vector<std::reference_wrapper<FunctionDeclaration>, 1>& overloads)
>-    {
>-        m_possibleAnderOverloads = overloads;
>-    }
>-    void setPossibleSetterOverloads(const Vector<std::reference_wrapper<FunctionDeclaration>, 1>& overloads)
>-    {
>-        m_possibleSetterOverloads = overloads;
>-    }
>-
>     void setGetterFunction(FunctionDeclaration* getterFunction)
>     {
>         m_getterFunction = getterFunction;
>@@ -104,9 +88,6 @@ public:
> 
> private:
>     UniqueRef<Expression> m_base;
>-    Vector<std::reference_wrapper<FunctionDeclaration>, 1> m_possibleGetterOverloads;
>-    Vector<std::reference_wrapper<FunctionDeclaration>, 1> m_possibleSetterOverloads;
>-    Vector<std::reference_wrapper<FunctionDeclaration>, 1> m_possibleAnderOverloads;
>     FunctionDeclaration* m_getterFunction { nullptr };
>     FunctionDeclaration* m_anderFunction { nullptr };
>     FunctionDeclaration* m_threadAnderFunction { nullptr };
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLAutoInitializeVariables.cpp b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLAutoInitializeVariables.cpp
>index d11b907af3dd3dcf699844db3a27bbffc79c4483..81647093567ecef78cb57254a99b16752bda3986 100644
>--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLAutoInitializeVariables.cpp
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLAutoInitializeVariables.cpp
>@@ -72,9 +72,8 @@ private:
>         auto callExpression = std::make_unique<AST::CallExpression>(variableDeclaration.origin(), WTFMove(functionName), Vector<UniqueRef<AST::Expression>>());
>         callExpression->setType(type->clone());
>         callExpression->setTypeAnnotation(AST::RightValue());
>-        callExpression->setOverloads(m_castFunctions);
>         Vector<std::reference_wrapper<ResolvingType>> argumentTypes;
>-        auto* function = resolveFunctionOverload(*callExpression->overloads(), argumentTypes, type);
>+        auto* function = resolveFunctionOverload(m_castFunctions, argumentTypes, type);
>         if (!function) {
>             setError();
>             return;
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp
>index 63985d929918e5e2f7798cd91a79d76c37bc6d02..0e50869a3cf7cf6bcf0884917affb61543b83e9c 100644
>--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp
>@@ -46,6 +46,7 @@
> #include "WHLSLLogicalNotExpression.h"
> #include "WHLSLMakeArrayReferenceExpression.h"
> #include "WHLSLMakePointerExpression.h"
>+#include "WHLSLNameContext.h"
> #include "WHLSLPointerType.h"
> #include "WHLSLProgram.h"
> #include "WHLSLReadModifyWriteExpression.h"
>@@ -216,10 +217,12 @@ static Optional<AST::NativeFunctionDeclaration> resolveByInstantiation(const Str
>     return WTF::nullopt;
> }
> 
>-static AST::FunctionDeclaration* resolveFunction(Program& program, Vector<std::reference_wrapper<AST::FunctionDeclaration>, 1>& possibleOverloads, Vector<std::reference_wrapper<ResolvingType>>& types, const String& name, Lexer::Token origin, const Intrinsics& intrinsics, AST::NamedType* castReturnType = nullptr)
>+static AST::FunctionDeclaration* resolveFunction(Program& program, Vector<std::reference_wrapper<AST::FunctionDeclaration>, 1>* possibleOverloads, Vector<std::reference_wrapper<ResolvingType>>& types, const String& name, Lexer::Token origin, const Intrinsics& intrinsics, AST::NamedType* castReturnType = nullptr)
> {
>-    if (AST::FunctionDeclaration* function = resolveFunctionOverload(possibleOverloads, types, castReturnType))
>-        return function;
>+    if (possibleOverloads) {
>+        if (AST::FunctionDeclaration* function = resolveFunctionOverload(*possibleOverloads, types, castReturnType))
>+            return function;
>+    }
> 
>     if (auto newFunction = resolveByInstantiation(name, origin, types, intrinsics)) {
>         program.append(WTFMove(*newFunction));
>@@ -1020,7 +1023,10 @@ void Checker::finishVisiting(AST::PropertyAccessExpression& propertyAccessExpres
>         Vector<std::reference_wrapper<ResolvingType>> getterArgumentTypes { baseInfo->resolvingType };
>         if (additionalArgumentType)
>             getterArgumentTypes.append(*additionalArgumentType);
>-        if ((getterFunction = resolveFunction(m_program, propertyAccessExpression.possibleGetterOverloads(), getterArgumentTypes, propertyAccessExpression.getterFunctionName(), propertyAccessExpression.origin(), m_intrinsics)))
>+        auto getterName = propertyAccessExpression.getterFunctionName();
>+        auto* getterFunctions = m_program.nameContext().getFunctions(getterName);
>+        getterFunction = resolveFunction(m_program, getterFunctions, getterArgumentTypes, getterName, propertyAccessExpression.origin(), m_intrinsics);
>+        if (getterFunction)
>             getterReturnType = &getterFunction->type();
>     }
> 
>@@ -1033,7 +1039,10 @@ void Checker::finishVisiting(AST::PropertyAccessExpression& propertyAccessExpres
>             Vector<std::reference_wrapper<ResolvingType>> anderArgumentTypes { argumentType };
>             if (additionalArgumentType)
>                 anderArgumentTypes.append(*additionalArgumentType);
>-            if ((anderFunction = resolveFunction(m_program, propertyAccessExpression.possibleAnderOverloads(), anderArgumentTypes, propertyAccessExpression.anderFunctionName(), propertyAccessExpression.origin(), m_intrinsics)))
>+            auto anderName = propertyAccessExpression.anderFunctionName();
>+            auto* anderFunctions = m_program.nameContext().getFunctions(anderName);
>+            anderFunction = resolveFunction(m_program, anderFunctions, anderArgumentTypes, anderName, propertyAccessExpression.origin(), m_intrinsics);
>+            if (anderFunction)
>                 anderReturnType = &downcast<AST::PointerType>(anderFunction->type()).elementType(); // FIXME: https://bugs.webkit.org/show_bug.cgi?id=198164 Enforce the return of anders will always be a pointer
>         }
>     }
>@@ -1045,7 +1054,10 @@ void Checker::finishVisiting(AST::PropertyAccessExpression& propertyAccessExpres
>         Vector<std::reference_wrapper<ResolvingType>> threadAnderArgumentTypes { argumentType };
>         if (additionalArgumentType)
>             threadAnderArgumentTypes.append(*additionalArgumentType);
>-        if ((threadAnderFunction = resolveFunction(m_program, propertyAccessExpression.possibleAnderOverloads(), threadAnderArgumentTypes, propertyAccessExpression.anderFunctionName(), propertyAccessExpression.origin(), m_intrinsics)))
>+        auto anderName = propertyAccessExpression.anderFunctionName();
>+        auto* anderFunctions = m_program.nameContext().getFunctions(anderName);
>+        threadAnderFunction = resolveFunction(m_program, anderFunctions, threadAnderArgumentTypes, anderName, propertyAccessExpression.origin(), m_intrinsics);
>+        if (threadAnderFunction)
>             threadAnderReturnType = &downcast<AST::PointerType>(threadAnderFunction->type()).elementType(); // FIXME: https://bugs.webkit.org/show_bug.cgi?id=198164 Enforce the return of anders will always be a pointer
>     }
> 
>@@ -1089,7 +1101,9 @@ void Checker::finishVisiting(AST::PropertyAccessExpression& propertyAccessExpres
>         if (additionalArgumentType)
>             setterArgumentTypes.append(*additionalArgumentType);
>         setterArgumentTypes.append(fieldResolvingType);
>-        setterFunction = resolveFunction(m_program, propertyAccessExpression.possibleSetterOverloads(), setterArgumentTypes, propertyAccessExpression.setterFunctionName(), propertyAccessExpression.origin(), m_intrinsics);
>+        auto setterName = propertyAccessExpression.setterFunctionName();
>+        auto* setterFunctions = m_program.nameContext().getFunctions(setterName);
>+        setterFunction = resolveFunction(m_program, setterFunctions, setterArgumentTypes, setterName, propertyAccessExpression.origin(), m_intrinsics);
>         if (setterFunction)
>             setterReturnType = &setterFunction->type();
>     }
>@@ -1469,8 +1483,22 @@ void Checker::visit(AST::CallExpression& callExpression)
>     // Don't recurse on the castReturnType, because it's guaranteed to be a NamedType, which will get visited later.
>     // We don't want to recurse to the same node twice.
> 
>-    ASSERT(callExpression.hasOverloads());
>-    auto* function = resolveFunction(m_program, *callExpression.overloads(), types, callExpression.name(), callExpression.origin(), m_intrinsics, callExpression.castReturnType());
>+    NameContext& nameContext = m_program.nameContext();
>+    auto* functions = nameContext.getFunctions(callExpression.name());
>+    if (!functions) {
>+        if (auto* types = nameContext.getTypes(callExpression.name())) {
>+            if (types->size() == 1) {
>+                if ((functions = nameContext.getFunctions("operator cast"_str)))
>+                    callExpression.setCastData((*types)[0].get());
>+            }
>+        }
>+    }
>+    if (!functions) {
>+        setError();
>+        return;
>+    }
>+
>+    auto* function = resolveFunction(m_program, functions, types, callExpression.name(), callExpression.origin(), m_intrinsics, callExpression.castReturnType());
>     if (!function) {
>         setError();
>         return;
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.cpp b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.cpp
>index 072f5c94664b43dec89df2be8dfdcd2f2c72f1ff..2c3a2d742e8277d5e3b2bfcfc7ab2ad70435342e 100644
>--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.cpp
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.cpp
>@@ -61,7 +61,6 @@ NameResolver::NameResolver(NameResolver& parentResolver, NameContext& nameContex
>     : m_nameContext(nameContext)
>     , m_parentNameResolver(&parentResolver)
> {
>-    m_isResolvingCalls = parentResolver.m_isResolvingCalls;
>     setCurrentFunctionDefinition(parentResolver.m_currentFunction);
> }
> 
>@@ -73,9 +72,6 @@ NameResolver::~NameResolver()
> 
> void NameResolver::visit(AST::TypeReference& typeReference)
> {
>-    if (m_isResolvingCalls)
>-        return;
>-
>     ScopedSetAdder<AST::TypeReference*> adder(m_typeReferences, &typeReference);
>     if (!adder.isNewEntry()) {
>         setError();
>@@ -203,14 +199,6 @@ void NameResolver::visit(AST::Return& returnStatement)
> 
> void NameResolver::visit(AST::PropertyAccessExpression& propertyAccessExpression)
> {
>-    if (m_isResolvingCalls) {
>-        if (auto* getterFunctions = m_nameContext.getFunctions(propertyAccessExpression.getterFunctionName()))
>-            propertyAccessExpression.setPossibleGetterOverloads(*getterFunctions);
>-        if (auto* setterFunctions = m_nameContext.getFunctions(propertyAccessExpression.setterFunctionName()))
>-            propertyAccessExpression.setPossibleSetterOverloads(*setterFunctions);
>-        if (auto* anderFunctions = m_nameContext.getFunctions(propertyAccessExpression.anderFunctionName()))
>-            propertyAccessExpression.setPossibleAnderOverloads(*anderFunctions);
>-    }
>     Visitor::visit(propertyAccessExpression);
> }
> 
>@@ -241,26 +229,6 @@ void NameResolver::visit(AST::DotExpression& dotExpression)
> 
> void NameResolver::visit(AST::CallExpression& callExpression)
> {
>-    if (m_isResolvingCalls) {
>-        if (!callExpression.hasOverloads()) {
>-            if (auto* functions = m_nameContext.getFunctions(callExpression.name()))
>-                callExpression.setOverloads(*functions);
>-            else {
>-                if (auto* types = m_nameContext.getTypes(callExpression.name())) {
>-                    if (types->size() == 1) {
>-                        if (auto* functions = m_nameContext.getFunctions("operator cast"_str)) {
>-                            callExpression.setCastData((*types)[0].get());
>-                            callExpression.setOverloads(*functions);
>-                        }
>-                    }
>-                }
>-            }
>-        }
>-        if (!callExpression.hasOverloads()) {
>-            setError();
>-            return;
>-        }
>-    }
>     Visitor::visit(callExpression);
> }
> 
>@@ -335,20 +303,6 @@ bool resolveTypeNamesInFunctions(Program& program, NameResolver& nameResolver)
>     return true;
> }
> 
>-bool resolveCallsInFunctions(Program& program, NameResolver& nameResolver)
>-{
>-    nameResolver.setIsResolvingCalls(true);
>-    for (auto& functionDefinition : program.functionDefinitions()) {
>-        nameResolver.setCurrentFunctionDefinition(&functionDefinition);
>-        nameResolver.checkErrorAndVisit(functionDefinition);
>-        if (nameResolver.error())
>-            return false;
>-    }
>-    nameResolver.setCurrentFunctionDefinition(nullptr);
>-    nameResolver.setIsResolvingCalls(false);
>-    return true;
>-}
>-
> } // namespace WHLSL
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.h b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.h
>index 91faacccc0c7e9b8ae1b62312d7a894a2bb5476a..13f210bfc5bbd4a980a769fbd3efc817b97231e1 100644
>--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.h
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.h
>@@ -51,8 +51,6 @@ public:
>         m_currentFunction = functionDefinition;
>     }
> 
>-    void setIsResolvingCalls(bool isResolvingCalls) { m_isResolvingCalls = isResolvingCalls; }
>-
> private:
>     void visit(AST::NativeFunctionDeclaration&) override;
>     void visit(AST::TypeReference&) override;
>@@ -73,12 +71,10 @@ private:
>     HashSet<AST::TypeReference*> m_typeReferences;
>     AST::FunctionDefinition* m_currentFunction { nullptr };
>     NameResolver* m_parentNameResolver { nullptr };
>-    bool m_isResolvingCalls { false };
> };
> 
> bool resolveNamesInTypes(Program&, NameResolver&);
> bool resolveTypeNamesInFunctions(Program&, NameResolver&);
>-bool resolveCallsInFunctions(Program&, NameResolver&);
> 
> } // namespace WHLSL
> 
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp
>index 228a48fc593a7793913f899f16d0d92fa5fb8f28..7c55ceaaccc5b8142645907c853f9ea78b7a5717 100644
>--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp
>@@ -54,6 +54,8 @@
> #include "WHLSLSynthesizeStructureAccessors.h"
> #include <wtf/Optional.h>
> 
>+#include <wtf/TimingScope.h>
>+
> namespace WebCore {
> 
> namespace WHLSL {
>@@ -128,7 +130,6 @@ static Optional<Program> prepareShared(String& whlslSource)
>     CHECK_PASS(synthesizeArrayOperatorLength, program);
>     CHECK_PASS(resolveTypeNamesInFunctions, program, nameResolver);
>     CHECK_PASS(synthesizeConstructors, program);
>-    CHECK_PASS(resolveCallsInFunctions, program, nameResolver);
>     CHECK_PASS(checkDuplicateFunctions, program);
> 
>     CHECK_PASS(check, program);

Actions: View | Formatted Diff | Diff

Attachments on bug 199474: 373438 | 373439 | 373519 | 373520