WebKit Bugzilla
Attachment 373520 Details for
Bug 199474
: [WHLSL] Remove the phase resolveCallsInFunctions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-199474-20190705120211.patch (text/plain), 20.71 KB, created by
Robin Morisset
on 2019-07-05 12:02:11 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Robin Morisset
Created:
2019-07-05 12:02:11 PDT
Size:
20.71 KB
patch
obsolete
>Subversion Revision: 247164 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 6c9d172d64913e8625b82cedd0c621c4b4717364..50d7c72408aaea3381fc8497df0e9b735bdc7e02 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 Myles Maxfield. >+ >+ 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-05 Ryan Haddad <ryanhaddad@apple.com> > > Unreviewed, rolling out r247115. >diff --git a/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h b/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h >index 1a0d0274e4d3c72ed2f6c0c8ab1de194d5fe544c..cbcdbf9d08ea67d1a3dacf791ed8f43f66605fda 100644 >--- a/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h >+++ b/Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLCallExpression.h >@@ -67,14 +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() { return m_function; } > > void setFunction(FunctionDeclaration& functionDeclaration) >@@ -86,7 +78,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 eb1fa983bdd6daf0378ce9a29cb7da865417f338..cc63884e5e0e7de794b9d3104e10f90b22753dfc 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" >@@ -215,10 +216,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)); >@@ -1019,7 +1022,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(); > } > >@@ -1032,7 +1038,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 > } > } >@@ -1044,7 +1053,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 > } > >@@ -1088,7 +1100,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 00674788fdaa14b183185684b454de62d8a8f5c4..069b93cc10d538f8ee270da676d34b0c402c5626 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(); >@@ -199,14 +195,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); > } > >@@ -237,26 +225,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); > } > >@@ -331,20 +299,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 ed9930b6993b64a528806897fc9b821ba4a7ad95..9d4f33b2c1274ba0d0072184df8d2f5004122379 100644 >--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp >+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp >@@ -133,7 +133,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); >diff --git a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPropertyResolver.cpp b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPropertyResolver.cpp >index dd7b929f205c1eeac047bb00e8ac94412a5f06d8..8abf0f674b2daf285321a5d8ea48e91cefdc90b7 100644 >--- a/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPropertyResolver.cpp >+++ b/Source/WebCore/Modules/webgpu/WHLSL/WHLSLPropertyResolver.cpp >@@ -759,9 +759,16 @@ void PropertyResolver::simplifyRightValue(AST::PropertyAccessExpression& propert > auto& getterFunction = *propertyAccessExpression.getterFunction(); > Vector<UniqueRef<AST::Expression>> arguments; > arguments.append(propertyAccessExpression.takeBase()); >- if (is<AST::IndexExpression>(propertyAccessExpression)) >- arguments.append(downcast<AST::IndexExpression>(propertyAccessExpression).takeIndex()); >- auto* callExpression = AST::replaceWith<AST::CallExpression>(propertyAccessExpression, WTFMove(origin), String(getterFunction.name()), WTFMove(arguments)); >+ AST::CallExpression* callExpression; >+ if (is<AST::IndexExpression>(propertyAccessExpression)) { >+ auto& indexExpression = downcast<AST::IndexExpression>(propertyAccessExpression); >+ arguments.append(indexExpression.takeIndex()); >+ callExpression = AST::replaceWith<AST::CallExpression>(indexExpression, WTFMove(origin), String(getterFunction.name()), WTFMove(arguments)); >+ } else { >+ ASSERT(is<AST::DotExpression>(propertyAccessExpression)); >+ auto& dotExpression = downcast<AST::DotExpression>(propertyAccessExpression); >+ callExpression = AST::replaceWith<AST::CallExpression>(dotExpression, WTFMove(origin), String(getterFunction.name()), WTFMove(arguments)); >+ } > callExpression->setFunction(getterFunction); > callExpression->setType(getterFunction.type().clone()); > callExpression->setTypeAnnotation(AST::RightValue());
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=373520">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 199474
:
373438
|
373439
|
373519
| 373520