Attachment 372312 Details for Bug 198580 – patch for landing

[patch] patch for landing

c-backup.diff (text/plain), 40.32 KB, created by Saam Barati on 2019-06-17 18:57:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2019-06-17 18:57:52 PDT

Size: 40.32 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 246530)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,94 @@
>+2019-06-17  Saam Barati  <sbarati@apple.com>
>+
>+        [WHLSL] Do not generate duplicate constructors/copy constructors in synthesizeConstructors
>+        https://bugs.webkit.org/show_bug.cgi?id=198580
>+
>+        Reviewed by Robin Morisset.
>+
>+        Prior to this patch, we were generating duplicate constructors 
>+        for unnamed types. This is bad for two reasons:
>+        1. It's inefficient, since we'd generate a constructor for every place in
>+        the AST where we'd visit this unnamed type.
>+        2. It made it impossible to resolve function overloads to call
>+        the default constructor. This made it so that the autoInitializeVariables
>+        pass would crash if we ever generated more than one of these functions
>+        for the same type.
>+        
>+        To make this work, this patch splits up what used to be the resolveNamesInFunctions
>+        pass. Previously, this pass would both resolve calls and resolve type names.
>+        Synthesize constructors would run before this, since resolving calls meant we
>+        may resolve a call to one of these synthesized constructors. However, synthesize
>+        constructors now needs to test for the equality unnamed types, so it now requires
>+        running the type resolution part of resolveNamesInFunctions before it runs.
>+        
>+        This patch splits resolveNamesInFunctions into two parts:
>+        resolveTypeNamesInFunctions and resolveCallsInFunctions.
>+        
>+        So we used to run:
>+        synthesizeConstructors
>+        resolveNamesInFunctions
>+        
>+        And now we run:
>+        resolveTypeNamesInFunctions 
>+        synthesizeConstructors
>+        resolveCallsInFunctions
>+
>+        Test: webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors.html
>+
>+        * Modules/webgpu/WHLSL/AST/WHLSLArrayReferenceType.h:
>+        * Modules/webgpu/WHLSL/AST/WHLSLArrayType.h:
>+        * Modules/webgpu/WHLSL/AST/WHLSLBooleanLiteral.h:
>+        (WebCore::WHLSL::AST::BooleanLiteral::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLEnumerationMemberLiteral.h:
>+        (WebCore::WHLSL::AST::EnumerationMemberLiteral::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLExpression.h:
>+        (WebCore::WHLSL::AST::Expression::copyTypeTo const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLFloatLiteral.h:
>+        (WebCore::WHLSL::AST::FloatLiteral::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLFloatLiteralType.cpp:
>+        (WebCore::WHLSL::AST::FloatLiteralType::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLFloatLiteralType.h:
>+        * Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteral.h:
>+        (WebCore::WHLSL::AST::IntegerLiteral::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteralType.cpp:
>+        (WebCore::WHLSL::AST::IntegerLiteralType::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteralType.h:
>+        * Modules/webgpu/WHLSL/AST/WHLSLNullLiteral.h:
>+        (WebCore::WHLSL::AST::NullLiteral::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLPointerType.h:
>+        * Modules/webgpu/WHLSL/AST/WHLSLTypeReference.h:
>+        * Modules/webgpu/WHLSL/AST/WHLSLUnnamedType.h:
>+        * Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteral.h:
>+        (WebCore::WHLSL::AST::UnsignedIntegerLiteral::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteralType.cpp:
>+        (WebCore::WHLSL::AST::UnsignedIntegerLiteralType::clone const):
>+        * Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteralType.h:
>+        * Modules/webgpu/WHLSL/WHLSLASTDumper.h:
>+        * Modules/webgpu/WHLSL/WHLSLInferTypes.cpp:
>+        (WebCore::WHLSL::matches):
>+        * Modules/webgpu/WHLSL/WHLSLNameResolver.cpp:
>+        (WebCore::WHLSL::NameResolver::visit):
>+        (WebCore::WHLSL::resolveTypeNamesInFunctions):
>+        (WebCore::WHLSL::resolveCallsInFunctions):
>+        (WebCore::WHLSL::resolveNamesInFunctions): Deleted.
>+        * Modules/webgpu/WHLSL/WHLSLNameResolver.h:
>+        (WebCore::WHLSL::NameResolver::setIsResolvingCalls):
>+        * Modules/webgpu/WHLSL/WHLSLPrepare.cpp:
>+        (WebCore::WHLSL::prepareShared):
>+        * Modules/webgpu/WHLSL/WHLSLSynthesizeConstructors.cpp:
>+        (WebCore::WHLSL::UnnamedTypeKey::UnnamedTypeKey):
>+        (WebCore::WHLSL::UnnamedTypeKey::isEmptyValue const):
>+        (WebCore::WHLSL::UnnamedTypeKey::isHashTableDeletedValue const):
>+        (WebCore::WHLSL::UnnamedTypeKey::hash const):
>+        (WebCore::WHLSL::UnnamedTypeKey::operator== const):
>+        (WebCore::WHLSL::UnnamedTypeKey::unnamedType const):
>+        (WebCore::WHLSL::UnnamedTypeKey::Hash::hash):
>+        (WebCore::WHLSL::UnnamedTypeKey::Hash::equal):
>+        (WebCore::WHLSL::UnnamedTypeKey::Traits::isEmptyValue):
>+        (WebCore::WHLSL::FindAllTypes::takeUnnamedTypes):
>+        (WebCore::WHLSL::FindAllTypes::appendNamedType):
>+        (WebCore::WHLSL::synthesizeConstructors):
>+
> 2019-06-17  Sihui Liu  <sihui_liu@apple.com>
> 
>         -[WKWebsiteDataStore removeDataOfTypes:modifiedSince:completionHandler:] doesn't delete _WKWebsiteDataTypeCredentials
>Index: Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/WHLSLASTDumper.h	(working copy)
>@@ -142,6 +142,7 @@ MAKE_PRINT_ADAPTOR(StatementDumper, AST:
> MAKE_PRINT_ADAPTOR(ProgramDumper, Program&, dumpASTNode);
> MAKE_PRINT_ADAPTOR(StructureDefinitionDumper, AST::StructureDefinition&, dumpASTNode);
> MAKE_PRINT_ADAPTOR(FunctionDefinitionDumper, AST::FunctionDefinition&, dumpASTNode);
>+MAKE_PRINT_ADAPTOR(NativeFunctionDeclarationDumper, AST::NativeFunctionDeclaration&, dumpASTNode);
> MAKE_PRINT_ADAPTOR(TypeDumper, AST::UnnamedType&, dumpASTNode);
> 
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/WHLSLInferTypes.cpp
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/WHLSLInferTypes.cpp	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/WHLSLInferTypes.cpp	(working copy)
>@@ -61,28 +61,7 @@ static bool matches(const AST::Type& uni
>         auto& unnamedThis = downcast<AST::UnnamedType>(unifyThis);
>         auto& unnamedOther = downcast<AST::UnnamedType>(unifyOther);
>         ASSERT(!is<AST::TypeReference>(unnamedThis) && !is<AST::TypeReference>(unnamedOther));
>-        if (is<AST::PointerType>(unnamedThis) && is<AST::PointerType>(unnamedOther)) {
>-            auto& pointerThis = downcast<AST::PointerType>(unnamedThis);
>-            auto& pointerOther = downcast<AST::PointerType>(unnamedOther);
>-            if (pointerThis.addressSpace() != pointerOther.addressSpace())
>-                return false;
>-            return matches(pointerThis.elementType(), pointerOther.elementType());
>-        }
>-        if (is<AST::ArrayReferenceType>(unnamedThis) && is<AST::ArrayReferenceType>(unnamedOther)) {
>-            auto& arrayReferenceThis = downcast<AST::ArrayReferenceType>(unnamedThis);
>-            auto& arrayReferenceOther = downcast<AST::ArrayReferenceType>(unnamedOther);
>-            if (arrayReferenceThis.addressSpace() != arrayReferenceOther.addressSpace())
>-                return false;
>-            return matches(arrayReferenceThis.elementType(), arrayReferenceOther.elementType());
>-        }
>-        if (is<AST::ArrayType>(unnamedThis) && is<AST::ArrayType>(unnamedOther)) {
>-            auto& arrayThis = downcast<AST::ArrayType>(unnamedThis);
>-            auto& arrayOther = downcast<AST::ArrayType>(unnamedOther);
>-            if (arrayThis.numElements() != arrayOther.numElements())
>-                return false;
>-            return matches(arrayThis.type(), arrayOther.type());
>-        }
>-        return false;
>+        return unnamedThis == unnamedOther;
>     }
>     return false;
> }
>Index: Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.cpp
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.cpp	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.cpp	(working copy)
>@@ -58,6 +58,9 @@ NameResolver::NameResolver(NameContext& 
> 
> void NameResolver::visit(AST::TypeReference& typeReference)
> {
>+    if (m_isResolvingCalls)
>+        return;
>+
>     ScopedSetAdder<AST::TypeReference*> adder(m_typeReferences, &typeReference);
>     if (!adder.isNewEntry()) {
>         setError();
>@@ -88,6 +91,7 @@ void NameResolver::visit(AST::FunctionDe
>     NameContext newNameContext(&m_nameContext);
>     NameResolver newNameResolver(newNameContext);
>     newNameResolver.setCurrentFunctionDefinition(m_currentFunction);
>+    newNameResolver.setIsResolvingCalls(m_isResolvingCalls);
>     checkErrorAndVisit(functionDefinition.type());
>     for (auto& parameter : functionDefinition.parameters())
>         newNameResolver.checkErrorAndVisit(parameter);
>@@ -99,6 +103,7 @@ void NameResolver::visit(AST::Block& blo
>     NameContext nameContext(&m_nameContext);
>     NameResolver newNameResolver(nameContext);
>     newNameResolver.setCurrentFunctionDefinition(m_currentFunction);
>+    newNameResolver.setIsResolvingCalls(m_isResolvingCalls);
>     newNameResolver.Visitor::visit(block);
> }
> 
>@@ -108,11 +113,13 @@ void NameResolver::visit(AST::IfStatemen
>     NameContext nameContext(&m_nameContext);
>     NameResolver newNameResolver(nameContext);
>     newNameResolver.setCurrentFunctionDefinition(m_currentFunction);
>+    newNameResolver.setIsResolvingCalls(m_isResolvingCalls);
>     newNameResolver.checkErrorAndVisit(ifStatement.body());
>     if (ifStatement.elseBody()) {
>         NameContext nameContext(&m_nameContext);
>         NameResolver newNameResolver(nameContext);
>         newNameResolver.setCurrentFunctionDefinition(m_currentFunction);
>+        newNameResolver.setIsResolvingCalls(m_isResolvingCalls);
>         newNameResolver.checkErrorAndVisit(*ifStatement.elseBody());
>     }
> }
>@@ -123,6 +130,7 @@ void NameResolver::visit(AST::WhileLoop&
>     NameContext nameContext(&m_nameContext);
>     NameResolver newNameResolver(nameContext);
>     newNameResolver.setCurrentFunctionDefinition(m_currentFunction);
>+    newNameResolver.setIsResolvingCalls(m_isResolvingCalls);
>     newNameResolver.checkErrorAndVisit(whileLoop.body());
> }
> 
>@@ -131,6 +139,7 @@ void NameResolver::visit(AST::DoWhileLoo
>     NameContext nameContext(&m_nameContext);
>     NameResolver newNameResolver(nameContext);
>     newNameResolver.setCurrentFunctionDefinition(m_currentFunction);
>+    newNameResolver.setIsResolvingCalls(m_isResolvingCalls);
>     newNameResolver.checkErrorAndVisit(whileLoop.body());
>     checkErrorAndVisit(whileLoop.conditional());
> }
>@@ -140,6 +149,7 @@ void NameResolver::visit(AST::ForLoop& f
>     NameContext nameContext(&m_nameContext);
>     NameResolver newNameResolver(nameContext);
>     newNameResolver.setCurrentFunctionDefinition(m_currentFunction);
>+    newNameResolver.setIsResolvingCalls(m_isResolvingCalls);
>     newNameResolver.Visitor::visit(forLoop);
> }
> 
>@@ -174,12 +184,14 @@ void NameResolver::visit(AST::Return& re
> 
> void NameResolver::visit(AST::PropertyAccessExpression& propertyAccessExpression)
> {
>-    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);
>+    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);
> }
> 
>@@ -210,23 +222,25 @@ void NameResolver::visit(AST::DotExpress
> 
> void NameResolver::visit(AST::CallExpression& callExpression)
> {
>-    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 (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;
>+        if (!callExpression.hasOverloads()) {
>+            setError();
>+            return;
>+        }
>     }
>     Visitor::visit(callExpression);
> }
>@@ -278,7 +292,7 @@ bool resolveNamesInTypes(Program& progra
>     return true;
> }
> 
>-bool resolveNamesInFunctions(Program& program, NameResolver& nameResolver)
>+bool resolveTypeNamesInFunctions(Program& program, NameResolver& nameResolver)
> {
>     for (auto& functionDefinition : program.functionDefinitions()) {
>         nameResolver.setCurrentFunctionDefinition(&functionDefinition);
>@@ -295,6 +309,20 @@ bool resolveNamesInFunctions(Program& pr
>     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
>Index: Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.h	(working copy)
>@@ -50,6 +50,8 @@ public:
>         m_currentFunction = functionDefinition;
>     }
> 
>+    void setIsResolvingCalls(bool isResolvingCalls) { m_isResolvingCalls = isResolvingCalls; }
>+
> private:
>     void visit(AST::TypeReference&) override;
>     void visit(AST::Block&) override;
>@@ -68,10 +70,12 @@ private:
>     NameContext& m_nameContext;
>     HashSet<AST::TypeReference*> m_typeReferences;
>     AST::FunctionDefinition* m_currentFunction { nullptr };
>+    bool m_isResolvingCalls { false };
> };
> 
> bool resolveNamesInTypes(Program&, NameResolver&);
>-bool resolveNamesInFunctions(Program&, NameResolver&);
>+bool resolveTypeNamesInFunctions(Program&, NameResolver&);
>+bool resolveCallsInFunctions(Program&, NameResolver&);
> 
> } // namespace WHLSL
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp	(working copy)
>@@ -125,8 +125,9 @@ static Optional<Program> prepareShared(S
>     RUN_PASS(synthesizeStructureAccessors, program);
>     RUN_PASS(synthesizeEnumerationFunctions, program);
>     RUN_PASS(synthesizeArrayOperatorLength, program);
>+    RUN_PASS(resolveTypeNamesInFunctions, program, nameResolver);
>     RUN_PASS(synthesizeConstructors, program);
>-    RUN_PASS(resolveNamesInFunctions, program, nameResolver);
>+    RUN_PASS(resolveCallsInFunctions, program, nameResolver);
>     RUN_PASS(checkDuplicateFunctions, program);
> 
>     RUN_PASS(check, program);
>Index: Source/WebCore/Modules/webgpu/WHLSL/WHLSLSynthesizeConstructors.cpp
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/WHLSLSynthesizeConstructors.cpp	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/WHLSLSynthesizeConstructors.cpp	(working copy)
>@@ -45,47 +45,82 @@ namespace WebCore {
> 
> namespace WHLSL {
> 
>+class UnnamedTypeKey {
>+public:
>+    UnnamedTypeKey() = default;
>+    UnnamedTypeKey(WTF::HashTableDeletedValueType)
>+    {
>+        m_type = bitwise_cast<AST::UnnamedType*>(static_cast<uintptr_t>(1));
>+    }
>+
>+    UnnamedTypeKey(AST::UnnamedType& type)
>+        : m_type(&type)
>+    { }
>+
>+    bool isEmptyValue() const { return !m_type; }
>+    bool isHashTableDeletedValue() const { return m_type == bitwise_cast<AST::UnnamedType*>(static_cast<uintptr_t>(1)); }
>+
>+    unsigned hash() const { return m_type->hash(); }
>+    bool operator==(const UnnamedTypeKey& other) const { return *m_type == *other.m_type; }
>+    AST::UnnamedType& unnamedType() const { return *m_type; }
>+
>+    struct Hash {
>+        static unsigned hash(const UnnamedTypeKey& key) { return key.hash(); }
>+        static bool equal(const UnnamedTypeKey& a, const UnnamedTypeKey& b) { return a == b; }
>+        static const bool safeToCompareToEmptyOrDeleted = false;
>+        static const bool emptyValueIsZero = true;
>+    };
>+
>+    struct Traits : public WTF::SimpleClassHashTraits<UnnamedTypeKey> {
>+        static const bool hasIsEmptyValueFunction = true;
>+        static bool isEmptyValue(const UnnamedTypeKey& key) { return key.isEmptyValue(); }
>+    };
>+
>+private:
>+    AST::UnnamedType* m_type { nullptr };
>+};
>+
> class FindAllTypes : public Visitor {
> public:
>     ~FindAllTypes() = default;
> 
>     void visit(AST::PointerType& pointerType) override
>     {
>-        m_unnamedTypes.append(pointerType);
>+        m_unnamedTypes.add(UnnamedTypeKey { pointerType });
>         Visitor::visit(pointerType);
>     }
> 
>     void visit(AST::ArrayReferenceType& arrayReferenceType) override
>     {
>-        m_unnamedTypes.append(arrayReferenceType);
>+        m_unnamedTypes.add(UnnamedTypeKey { arrayReferenceType });
>         Visitor::visit(arrayReferenceType);
>     }
> 
>     void visit(AST::ArrayType& arrayType) override
>     {
>-        m_unnamedTypes.append(arrayType);
>+        m_unnamedTypes.add(UnnamedTypeKey { arrayType });
>         Visitor::visit(arrayType);
>     }
> 
>     void visit(AST::EnumerationDefinition& enumerationDefinition) override
>     {
>-        m_namedTypes.append(enumerationDefinition);
>+        appendNamedType(enumerationDefinition);
>         Visitor::visit(enumerationDefinition);
>     }
> 
>     void visit(AST::StructureDefinition& structureDefinition) override
>     {
>-        m_namedTypes.append(structureDefinition);
>+        appendNamedType(structureDefinition);
>         Visitor::visit(structureDefinition);
>     }
> 
>     void visit(AST::NativeTypeDeclaration& nativeTypeDeclaration) override
>     {
>-        m_namedTypes.append(nativeTypeDeclaration);
>+        appendNamedType(nativeTypeDeclaration);
>         Visitor::visit(nativeTypeDeclaration);
>     }
> 
>-    Vector<std::reference_wrapper<AST::UnnamedType>> takeUnnamedTypes()
>+    HashSet<UnnamedTypeKey, UnnamedTypeKey::Hash, UnnamedTypeKey::Traits> takeUnnamedTypes()
>     {
>         return WTFMove(m_unnamedTypes);
>     }
>@@ -96,7 +131,17 @@ public:
>     }
> 
> private:
>-    Vector<std::reference_wrapper<AST::UnnamedType>> m_unnamedTypes;
>+    void appendNamedType(AST::NamedType& type)
>+    {
>+        // The way we walk the AST ensures we should never visit a named type twice.
>+#if !ASSERT_DISABLED
>+        for (auto& entry : m_namedTypes)
>+            ASSERT(&entry.get().unifyNode() != &type.unifyNode());
>+#endif
>+        m_namedTypes.append(type);
>+    }
>+
>+    HashSet<UnnamedTypeKey, UnnamedTypeKey::Hash, UnnamedTypeKey::Traits> m_unnamedTypes;
>     Vector<std::reference_wrapper<AST::NamedType>> m_namedTypes;
> };
> 
>@@ -109,14 +154,16 @@ bool synthesizeConstructors(Program& pro
> 
>     bool isOperator = true;
> 
>-    for (auto& unnamedType : unnamedTypes) {
>-        auto variableDeclaration = makeUniqueRef<AST::VariableDeclaration>(Lexer::Token(unnamedType.get().origin()), AST::Qualifiers(), unnamedType.get().clone(), String(), WTF::nullopt, WTF::nullopt);
>+    for (auto& unnamedTypeKey : unnamedTypes) {
>+        auto& unnamedType = unnamedTypeKey.unnamedType();
>+
>+        auto variableDeclaration = makeUniqueRef<AST::VariableDeclaration>(Lexer::Token(unnamedType.origin()), AST::Qualifiers(), unnamedType.clone(), String(), WTF::nullopt, WTF::nullopt);
>         AST::VariableDeclarations parameters;
>         parameters.append(WTFMove(variableDeclaration));
>-        AST::NativeFunctionDeclaration copyConstructor(AST::FunctionDeclaration(Lexer::Token(unnamedType.get().origin()), AST::AttributeBlock(), WTF::nullopt, unnamedType.get().clone(), "operator cast"_str, WTFMove(parameters), WTF::nullopt, isOperator));
>+        AST::NativeFunctionDeclaration copyConstructor(AST::FunctionDeclaration(Lexer::Token(unnamedType.origin()), AST::AttributeBlock(), WTF::nullopt, unnamedType.clone(), "operator cast"_str, WTFMove(parameters), WTF::nullopt, isOperator));
>         program.append(WTFMove(copyConstructor));
> 
>-        AST::NativeFunctionDeclaration defaultConstructor(AST::FunctionDeclaration(Lexer::Token(unnamedType.get().origin()), AST::AttributeBlock(), WTF::nullopt, unnamedType.get().clone(), "operator cast"_str, AST::VariableDeclarations(), WTF::nullopt, isOperator));
>+        AST::NativeFunctionDeclaration defaultConstructor(AST::FunctionDeclaration(Lexer::Token(unnamedType.origin()), AST::AttributeBlock(), WTF::nullopt, unnamedType.clone(), "operator cast"_str, AST::VariableDeclarations(), WTF::nullopt, isOperator));
>         if (!program.append(WTFMove(defaultConstructor)))
>             return false;
>     }
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayReferenceType.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayReferenceType.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayReferenceType.h	(working copy)
>@@ -63,6 +63,15 @@ public:
>         return this->Base::hash() ^ StringHasher::computeLiteralHash("array");
>     }
> 
>+    bool operator==(const UnnamedType& other) const override
>+    {
>+        if (!is<ArrayReferenceType>(other))
>+            return false;
>+
>+        return addressSpace() == downcast<ArrayReferenceType>(other).addressSpace()
>+            && elementType() == downcast<ArrayReferenceType>(other).elementType();
>+    }
>+
> private:
> };
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayType.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayType.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLArrayType.h	(working copy)
>@@ -69,6 +69,15 @@ public:
>         return WTF::IntHash<unsigned>::hash(m_numElements) ^ m_elementType->hash();
>     }
> 
>+    bool operator==(const UnnamedType& other) const override
>+    {
>+        if (!is<ArrayType>(other))
>+            return false;
>+
>+        return numElements() == downcast<ArrayType>(other).numElements()
>+            && type() == downcast<ArrayType>(other).type();
>+    }
>+
> private:
>     UniqueRef<UnnamedType> m_elementType;
>     unsigned m_numElements;
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLBooleanLiteral.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLBooleanLiteral.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLBooleanLiteral.h	(working copy)
>@@ -58,7 +58,9 @@ public:
> 
>     BooleanLiteral clone() const
>     {
>-        return BooleanLiteral(Lexer::Token(origin()), m_value);
>+        BooleanLiteral result(Lexer::Token(origin()), m_value);
>+        copyTypeTo(result);
>+        return result;
>     }
> 
> private:
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLEnumerationMemberLiteral.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLEnumerationMemberLiteral.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLEnumerationMemberLiteral.h	(working copy)
>@@ -73,6 +73,7 @@ public:
>     {
>         auto result = EnumerationMemberLiteral(Lexer::Token(origin()), String(m_left), String(m_right));
>         result.m_enumerationMember = m_enumerationMember;
>+        copyTypeTo(result);
>         return result;
>     }
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLExpression.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLExpression.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLExpression.h	(working copy)
>@@ -84,6 +84,14 @@ public:
>         m_typeAnnotation = WTFMove(typeAnnotation);
>     }
> 
>+    void copyTypeTo(Expression& other) const
>+    {
>+        if (auto* resolvedType = const_cast<Expression*>(this)->maybeResolvedType())
>+            other.setType(resolvedType->clone());
>+        if (auto* typeAnnotation = maybeTypeAnnotation())
>+            other.setTypeAnnotation(TypeAnnotation(*typeAnnotation));
>+    }
>+
>     virtual bool isAssignmentExpression() const { return false; }
>     virtual bool isBooleanLiteral() const { return false; }
>     virtual bool isCallExpression() const { return false; }
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteral.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteral.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteral.h	(working copy)
>@@ -62,8 +62,10 @@ public:
>     FloatLiteral clone() const
>     {
>         FloatLiteral result(Lexer::Token(origin()), m_value);
>+        result.m_type = m_type.clone();
>         if (auto* resolvedType = m_type.maybeResolvedType())
>             result.m_type.resolve(resolvedType->clone());
>+        copyTypeTo(result);
>         return result;
>     }
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteralType.cpp
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteralType.cpp	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteralType.cpp	(working copy)
>@@ -72,6 +72,16 @@ unsigned FloatLiteralType::conversionCos
>     return 1;
> }
> 
>+FloatLiteralType FloatLiteralType::clone() const
>+{
>+    FloatLiteralType result(Lexer::Token(m_preferredType->origin()), m_value);
>+    if (auto* type = maybeResolvedType())
>+        result.resolve(type->clone());
>+    result.m_preferredType = m_preferredType->cloneTypeReference();
>+    return result;
>+}
>+
>+
> } // namespace AST
> 
> }
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteralType.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteralType.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLFloatLiteralType.h	(working copy)
>@@ -60,6 +60,8 @@ public:
>     bool canResolve(const Type&) const override;
>     unsigned conversionCost(const UnnamedType&) const override;
> 
>+    FloatLiteralType clone() const;
>+
> private:
>     float m_value;
>     // This is a unique_ptr to resolve a circular dependency between
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteral.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteral.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteral.h	(working copy)
>@@ -62,8 +62,10 @@ public:
>     IntegerLiteral clone() const
>     {
>         IntegerLiteral result(Lexer::Token(origin()), m_value);
>+        result.m_type = m_type.clone();
>         if (auto* resolvedType = m_type.maybeResolvedType())
>             result.m_type.resolve(resolvedType->clone());
>+        copyTypeTo(result);
>         return result;
>     }
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteralType.cpp
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteralType.cpp	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteralType.cpp	(working copy)
>@@ -73,6 +73,15 @@ unsigned IntegerLiteralType::conversionC
>     return 1;
> }
> 
>+IntegerLiteralType IntegerLiteralType::clone() const
>+{
>+    IntegerLiteralType result(Lexer::Token(m_preferredType->origin()), m_value);
>+    if (auto* type = maybeResolvedType())
>+        result.resolve(type->clone());
>+    result.m_preferredType = m_preferredType->cloneTypeReference();
>+    return result;
>+}
>+
> } // namespace AST
> 
> }
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteralType.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteralType.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLIntegerLiteralType.h	(working copy)
>@@ -60,6 +60,8 @@ public:
>     bool canResolve(const Type&) const override;
>     unsigned conversionCost(const UnnamedType&) const override;
> 
>+    IntegerLiteralType clone() const;
>+
> private:
>     int m_value;
>     // This is a unique_ptr to resolve a circular dependency between
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLNullLiteral.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLNullLiteral.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLNullLiteral.h	(working copy)
>@@ -61,6 +61,7 @@ public:
>         auto result = NullLiteral(Lexer::Token(origin()));
>         if (auto* resolvedType = m_type.maybeResolvedType())
>             result.m_type.resolve(resolvedType->clone());
>+        copyTypeTo(result);
>         return result;
>     }
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLPointerType.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLPointerType.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLPointerType.h	(working copy)
>@@ -63,6 +63,15 @@ public:
>         return this->Base::hash() ^ StringHasher::computeLiteralHash("pointer");
>     }
> 
>+    bool operator==(const UnnamedType& other) const override
>+    {
>+        if (!is<PointerType>(other))
>+            return false;
>+
>+        return addressSpace() == downcast<PointerType>(other).addressSpace()
>+            && elementType() == downcast<PointerType>(other).elementType();
>+    }
>+
> private:
> };
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLTypeReference.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLTypeReference.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLTypeReference.h	(working copy)
>@@ -107,6 +107,15 @@ public:
>         return WTF::PtrHash<const Type*>::hash(&unifyNode());
>     }
> 
>+    bool operator==(const UnnamedType& other) const override
>+    {
>+        ASSERT(m_resolvedType);
>+        if (!is<TypeReference>(other))
>+            return false;
>+
>+        return &unifyNode() == &downcast<TypeReference>(other).unifyNode();
>+    }
>+
> private:
>     String m_name;
>     TypeArguments m_typeArguments;
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnnamedType.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnnamedType.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnnamedType.h	(working copy)
>@@ -64,6 +64,7 @@ public:
>     virtual UniqueRef<UnnamedType> clone() const = 0;
> 
>     virtual unsigned hash() const = 0;
>+    virtual bool operator==(const UnnamedType&) const = 0;
> 
>     const Lexer::Token& origin() const { return m_origin; }
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteral.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteral.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteral.h	(working copy)
>@@ -62,8 +62,10 @@ public:
>     UnsignedIntegerLiteral clone() const
>     {
>         UnsignedIntegerLiteral result(Lexer::Token(origin()), m_value);
>+        result.m_type = m_type.clone();
>         if (auto* resolvedType = m_type.maybeResolvedType())
>             result.m_type.resolve(resolvedType->clone());
>+        copyTypeTo(result);
>         return result;
>     }
> 
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteralType.cpp
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteralType.cpp	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteralType.cpp	(working copy)
>@@ -76,6 +76,15 @@ unsigned UnsignedIntegerLiteralType::con
>     return 1;
> }
> 
>+UnsignedIntegerLiteralType UnsignedIntegerLiteralType::clone() const
>+{
>+    UnsignedIntegerLiteralType result(Lexer::Token(m_preferredType->origin()), m_value);
>+    if (auto* type = maybeResolvedType())
>+        result.resolve(type->clone());
>+    result.m_preferredType = m_preferredType->cloneTypeReference();
>+    return result;
>+}
>+
> } // namespace AST
> 
> }
>Index: Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteralType.h
>===================================================================
>--- Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteralType.h	(revision 246515)
>+++ Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnsignedIntegerLiteralType.h	(working copy)
>@@ -60,6 +60,8 @@ public:
>     bool canResolve(const Type&) const override;
>     unsigned conversionCost(const UnnamedType&) const override;
> 
>+    UnsignedIntegerLiteralType clone() const;
>+
> private:
>     unsigned m_value;
>     // This is a unique_ptr to resolve a circular dependency between
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 246515)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2019-06-17  Saam Barati  <sbarati@apple.com>
>+
>+        [WHLSL] Do not generate duplicate constructors/copy constructors in synthesizeConstructors
>+        https://bugs.webkit.org/show_bug.cgi?id=198580
>+
>+        Reviewed by Robin Morisset.
>+
>+        * webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors-expected.txt: Added.
>+        * webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors.html: Added.
>+
> 2019-06-17  Saam Barati  <sbarati@apple.com>
> 
>         [WHLSL] Make .length work
>Index: LayoutTests/webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors-expected.txt
>===================================================================
>--- LayoutTests/webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors-expected.txt	(nonexistent)
>+++ LayoutTests/webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors-expected.txt	(working copy)
>@@ -0,0 +1,12 @@
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+PASS resultsFloat32Array[0] is 2
>+PASS resultsFloat32Array[1] is 4
>+PASS resultsFloat32Array[2] is 6
>+PASS resultsFloat32Array[3] is 8
>+PASS resultsFloat32Array[4] is 5
>+PASS resultsFloat32Array[5] is 6
>+PASS resultsFloat32Array[6] is 7
>+PASS resultsFloat32Array[7] is 8
>+
>Index: LayoutTests/webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors.html
>===================================================================
>--- LayoutTests/webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors.html	(nonexistent)
>+++ LayoutTests/webgpu/whlsl-duplicate-types-should-not-produce-duplicate-ctors.html	(working copy)
>@@ -0,0 +1,111 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../resources/js-test-pre.js"></script>
>+</head>
>+<body>
>+<script>
>+const shaderSource = `
>+struct XYZ {
>+    int x;
>+    int y;
>+    int z;
>+}
>+
>+typedef XYZ1 = XYZ;
>+typedef XYZ2 = XYZ;
>+typedef XYZ3 = XYZ;
>+
>+[numthreads(2, 1, 1)]
>+compute void computeShader(device float[] buffer : register(u0), float3 threadID : SV_DispatchThreadID) {
>+    thread int* x;
>+    thread int* y;
>+    thread float* f1;
>+    thread float* f2;
>+    XYZ1 a;
>+    XYZ2 b;
>+    XYZ3 c;
>+    thread int[] intArrayRef1;
>+    thread int[] intArrayRef2;
>+    if (*x == *y && x == y && x == null && f1 == null && *f1 == 0) {
>+        buffer[uint(threadID.x)] = buffer[uint(threadID.x)] * 2.0;
>+    }
>+}
>+`;
>+let resultsFloat32Array;
>+async function start() {
>+    const adapter = await navigator.gpu.requestAdapter();
>+    const device = await adapter.requestDevice();
>+
>+    const shaderModule = device.createShaderModule({code: shaderSource, isWHLSL: true});
>+    const computeStage = {module: shaderModule, entryPoint: "computeShader"};
>+
>+    const bindGroupLayoutDescriptor = {bindings: [{binding: 0, visibility: 7, type: "storage-buffer"}]};
>+    const bindGroupLayout = device.createBindGroupLayout(bindGroupLayoutDescriptor);
>+    const pipelineLayoutDescriptor = {bindGroupLayouts: [bindGroupLayout]};
>+    const pipelineLayout = device.createPipelineLayout(pipelineLayoutDescriptor);
>+
>+    const computePipelineDescriptor = {computeStage, layout: pipelineLayout};
>+    const computePipeline = device.createComputePipeline(computePipelineDescriptor);
>+
>+    const size = Float32Array.BYTES_PER_ELEMENT * 8;
>+
>+    const bufferDescriptor = {size, usage: GPUBufferUsage.MAP_WRITE | GPUBufferUsage.TRANSFER_SRC};
>+    const buffer = device.createBuffer(bufferDescriptor);
>+    const bufferArrayBuffer = await buffer.mapWriteAsync();
>+    const bufferFloat32Array = new Float32Array(bufferArrayBuffer);
>+    bufferFloat32Array[0] = 1;
>+    bufferFloat32Array[1] = 2;
>+    bufferFloat32Array[2] = 3;
>+    bufferFloat32Array[3] = 4;
>+    bufferFloat32Array[4] = 5;
>+    bufferFloat32Array[5] = 6;
>+    bufferFloat32Array[6] = 7;
>+    bufferFloat32Array[7] = 8;
>+    buffer.unmap();
>+
>+    const resultsBufferDescriptor = {size, usage: GPUBufferUsage.STORAGE | GPUBufferUsage.TRANSFER_DST | GPUBufferUsage.MAP_READ};
>+    const resultsBuffer = device.createBuffer(resultsBufferDescriptor);
>+
>+    const bufferBinding = {buffer: resultsBuffer, size};
>+    const bindGroupBinding = {binding: 0, resource: bufferBinding};
>+    const bindGroupDescriptor = {layout: bindGroupLayout, bindings: [bindGroupBinding]};
>+    const bindGroup = device.createBindGroup(bindGroupDescriptor);
>+
>+    const commandEncoder = device.createCommandEncoder(); // {}
>+    commandEncoder.copyBufferToBuffer(buffer, 0, resultsBuffer, 0, Float32Array.BYTES_PER_ELEMENT * 8);
>+    const computePassEncoder = commandEncoder.beginComputePass();
>+    computePassEncoder.setPipeline(computePipeline);
>+    computePassEncoder.setBindGroup(0, bindGroup);
>+    computePassEncoder.dispatch(2, 1, 1);
>+    computePassEncoder.endPass();
>+    const commandBuffer = commandEncoder.finish();
>+    device.getQueue().submit([commandBuffer]);
>+
>+    const resultsArrayBuffer = await resultsBuffer.mapReadAsync();
>+    resultsFloat32Array = new Float32Array(resultsArrayBuffer);
>+    shouldBe("resultsFloat32Array[0]", "2");
>+    shouldBe("resultsFloat32Array[1]", "4");
>+    shouldBe("resultsFloat32Array[2]", "6");
>+    shouldBe("resultsFloat32Array[3]", "8");
>+    shouldBe("resultsFloat32Array[4]", "5");
>+    shouldBe("resultsFloat32Array[5]", "6");
>+    shouldBe("resultsFloat32Array[6]", "7");
>+    shouldBe("resultsFloat32Array[7]", "8");
>+    resultsBuffer.unmap();
>+}
>+if (window.testRunner)
>+    testRunner.waitUntilDone();
>+window.addEventListener("load", function() {
>+    start().then(function() {
>+        if (window.testRunner)
>+            testRunner.notifyDone();
>+    }, function() {
>+        if (window.testRunner)
>+            testRunner.notifyDone();
>+    });
>+});
>+</script>
>+<script src="../resources/js-test-post.js"></script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 198580: 372083 | 372131 | 372134 | 372312 | 372348