Title: [164972] trunk/Source/WebCore
Revision
164972
Author
[email protected]
Date
2014-03-02 23:10:57 -0800 (Sun, 02 Mar 2014)

Log Message

Add a fallback path for compiling the remaining attribute checkers
https://bugs.webkit.org/show_bug.cgi?id=129580

Reviewed by Darin Adler.

The remaining attribute checkers appear to be less common than the simple value match.
This patch adds them to SelectorCompiler for completeness but no attempt is made at optimizing them,
they all default to function calls.

If the assumption that those selectors are not common turn out to be incorrect, we should see
the function calls in profiles and optimize them as needed.

* css/SelectorChecker.cpp:
(WebCore::attributeValueMatches):
If we get anything but attribute match here, something has gone horribly wrong. Update the code
to fail if that were to happen.

* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::SelectorCodeGenerator::SelectorCodeGenerator):
Add the missing match type to the selector fragment.

Begin, End, Contain cannot match an empty value per specification. We can test that at compile time
and fail immediately. See http://www.w3.org/TR/css3-selectors/#attribute-substrings

List has the extra requirement that a value containing a space does not match anything. It also cannot
match with an empty string. See http://www.w3.org/TR/css3-selectors/#attribute-representation

(WebCore::SelectorCompiler::attributeValueBeginsWith):
(WebCore::SelectorCompiler::attributeValueContains):
(WebCore::SelectorCompiler::attributeValueEndsWith):
(WebCore::SelectorCompiler::attributeValueMatchHyphenRule):
(WebCore::SelectorCompiler::attributeValueSpaceSeparetedListContains):
The slow fallbacks.

(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementAttributeValueMatching):
(WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementAttributeFunctionCallValueMatching):
A generic code generator making function call to match an attribute value.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (164971 => 164972)


--- trunk/Source/WebCore/ChangeLog	2014-03-03 06:01:44 UTC (rev 164971)
+++ trunk/Source/WebCore/ChangeLog	2014-03-03 07:10:57 UTC (rev 164972)
@@ -1,3 +1,43 @@
+2014-03-02  Benjamin Poulain  <[email protected]>
+
+        Add a fallback path for compiling the remaining attribute checkers
+        https://bugs.webkit.org/show_bug.cgi?id=129580
+
+        Reviewed by Darin Adler.
+
+        The remaining attribute checkers appear to be less common than the simple value match.
+        This patch adds them to SelectorCompiler for completeness but no attempt is made at optimizing them,
+        they all default to function calls.
+
+        If the assumption that those selectors are not common turn out to be incorrect, we should see
+        the function calls in profiles and optimize them as needed.
+
+        * css/SelectorChecker.cpp:
+        (WebCore::attributeValueMatches):
+        If we get anything but attribute match here, something has gone horribly wrong. Update the code
+        to fail if that were to happen.
+
+        * cssjit/SelectorCompiler.cpp:
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::SelectorCodeGenerator):
+        Add the missing match type to the selector fragment.
+
+        Begin, End, Contain cannot match an empty value per specification. We can test that at compile time
+        and fail immediately. See http://www.w3.org/TR/css3-selectors/#attribute-substrings
+
+        List has the extra requirement that a value containing a space does not match anything. It also cannot
+        match with an empty string. See http://www.w3.org/TR/css3-selectors/#attribute-representation
+
+        (WebCore::SelectorCompiler::attributeValueBeginsWith):
+        (WebCore::SelectorCompiler::attributeValueContains):
+        (WebCore::SelectorCompiler::attributeValueEndsWith):
+        (WebCore::SelectorCompiler::attributeValueMatchHyphenRule):
+        (WebCore::SelectorCompiler::attributeValueSpaceSeparetedListContains):
+        The slow fallbacks.
+
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementAttributeValueMatching):
+        (WebCore::SelectorCompiler::SelectorCodeGenerator::generateElementAttributeFunctionCallValueMatching):
+        A generic code generator making function call to match an attribute value.
+
 2014-02-25  Andreas Kling  <[email protected]>
 
         JSDOMWindow::commonVM() should return a reference.

Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (164971 => 164972)


--- trunk/Source/WebCore/css/SelectorChecker.cpp	2014-03-03 06:01:44 UTC (rev 164971)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp	2014-03-03 07:10:57 UTC (rev 164972)
@@ -280,6 +280,8 @@
     ASSERT(!value.isNull());
 
     switch (match) {
+    case CSSSelector::Set:
+        break;
     case CSSSelector::Exact:
         if (caseSensitive ? selectorValue != value : !equalIgnoringCase(selectorValue, value))
             return false;
@@ -327,10 +329,9 @@
         if (value.length() != selectorValue.length() && value[selectorValue.length()] != '-')
             return false;
         break;
-    case CSSSelector::PseudoClass:
-    case CSSSelector::PseudoElement:
     default:
-        break;
+        ASSERT_NOT_REACHED();
+        return false;
     }
 
     return true;

Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (164971 => 164972)


--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2014-03-03 06:01:44 UTC (rev 164971)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2014-03-03 07:10:57 UTC (rev 164972)
@@ -175,6 +175,7 @@
     void generateElementAttributeMatching(Assembler::JumpList& failureCases, Assembler::RegisterID currentAttributeAddress, Assembler::RegisterID decIndexRegister, const AttributeMatchingInfo& attributeInfo);
     void generateElementAttributeValueMatching(Assembler::JumpList& failureCases, Assembler::RegisterID currentAttributeAddress, const AttributeMatchingInfo& attributeInfo);
     void generateElementAttributeValueExactMatching(Assembler::JumpList& failureCases, Assembler::RegisterID currentAttributeAddress, const AtomicString& expectedValue, bool caseSensitive);
+    void generateElementAttributeFunctionCallValueMatching(Assembler::JumpList& failureCases, Assembler::RegisterID currentAttributeAddress, const AtomicString& expectedValue, bool caseSensitive, JSC::FunctionPtr caseSensitiveTest, JSC::FunctionPtr caseInsensitiveTest);
     void generateElementHasTagName(Assembler::JumpList& failureCases, const QualifiedName& nameToMatch);
     void generateElementHasId(Assembler::JumpList& failureCases, const LocalRegister& elementDataAddress, const AtomicString& idToMatch);
     void generateElementHasClasses(Assembler::JumpList& failureCases, const LocalRegister& elementDataAddress, const Vector<const AtomicStringImpl*>& classNames);
@@ -348,19 +349,29 @@
             if (m_functionType == FunctionType::CannotCompile || m_functionType == FunctionType::CannotMatchAnything)
                 return;
             break;
+        case CSSSelector::List:
+            if (selector->value().contains(' ')) {
+                m_functionType = FunctionType::CannotMatchAnything;
+                return;
+            }
+            FALLTHROUGH;
+        case CSSSelector::Begin:
+        case CSSSelector::End:
+        case CSSSelector::Contain:
+            if (selector->value().isEmpty()) {
+                m_functionType = FunctionType::CannotMatchAnything;
+                return;
+            }
+            FALLTHROUGH;
         case CSSSelector::Exact:
+        case CSSSelector::Hyphen:
             fragment.attributes.append(AttributeMatchingInfo(selector, HTMLDocument::isCaseSensitiveAttribute(selector->attribute())));
             break;
         case CSSSelector::Set:
             fragment.attributes.append(AttributeMatchingInfo(selector, true));
             break;
         case CSSSelector::Unknown:
-        case CSSSelector::List:
-        case CSSSelector::Hyphen:
         case CSSSelector::PseudoElement:
-        case CSSSelector::Contain:
-        case CSSSelector::Begin:
-        case CSSSelector::End:
         case CSSSelector::PagePseudoClass:
             goto CannotHandleSelector;
         }
@@ -1173,25 +1184,120 @@
 
     successCases.link(&m_assembler);
 
-    // We make the assumption that name matching fails in most cases and we keep value matching outside
-    // of the loop. We re-enter the loop if needed.
-    // FIXME: exact case sensitive value matching is so simple that it should be done in the loop.
-    Assembler::JumpList localFailureCases;
-    generateElementAttributeValueMatching(localFailureCases, currentAttributeAddress, attributeInfo);
-    localFailureCases.linkTo(loopReEntry, &m_assembler);
+    if (attributeSelector.m_match != CSSSelector::Set) {
+        // We make the assumption that name matching fails in most cases and we keep value matching outside
+        // of the loop. We re-enter the loop if needed.
+        // FIXME: exact case sensitive value matching is so simple that it should be done in the loop.
+        Assembler::JumpList localFailureCases;
+        generateElementAttributeValueMatching(localFailureCases, currentAttributeAddress, attributeInfo);
+        localFailureCases.linkTo(loopReEntry, &m_assembler);
+    }
 }
 
+enum CaseSensitivity {
+    CaseSensitive,
+    CaseInsensitive
+};
+
+template<CaseSensitivity caseSensitivity>
+static bool attributeValueBeginsWith(const Attribute* attribute, AtomicStringImpl* expectedString)
+{
+    AtomicStringImpl& valueImpl = *attribute->value().impl();
+    if (caseSensitivity == CaseSensitive)
+        return valueImpl.startsWith(expectedString);
+    return valueImpl.startsWith(expectedString, false);
+}
+
+template<CaseSensitivity caseSensitivity>
+static bool attributeValueContains(const Attribute* attribute, AtomicStringImpl* expectedString)
+{
+    AtomicStringImpl& valueImpl = *attribute->value().impl();
+    if (caseSensitivity == CaseSensitive)
+        return valueImpl.find(expectedString) != notFound;
+    return valueImpl.findIgnoringCase(expectedString) != notFound;
+}
+
+template<CaseSensitivity caseSensitivity>
+static bool attributeValueEndsWith(const Attribute* attribute, AtomicStringImpl* expectedString)
+{
+    AtomicStringImpl& valueImpl = *attribute->value().impl();
+    if (caseSensitivity == CaseSensitive)
+        return valueImpl.endsWith(expectedString);
+    return valueImpl.endsWith(expectedString, false);
+}
+
+template<CaseSensitivity caseSensitivity>
+static bool attributeValueMatchHyphenRule(const Attribute* attribute, AtomicStringImpl* expectedString)
+{
+    AtomicStringImpl& valueImpl = *attribute->value().impl();
+    if (valueImpl.length() < expectedString->length())
+        return false;
+
+    bool valueStartsWithExpectedString;
+    if (caseSensitivity == CaseSensitive)
+        valueStartsWithExpectedString = valueImpl.startsWith(expectedString);
+    else
+        valueStartsWithExpectedString = valueImpl.startsWith(expectedString, false);
+
+    if (!valueStartsWithExpectedString)
+        return false;
+
+    return valueImpl.length() == expectedString->length() || valueImpl[expectedString->length()] == '-';
+}
+
+template<CaseSensitivity caseSensitivity>
+static bool attributeValueSpaceSeparetedListContains(const Attribute* attribute, AtomicStringImpl* expectedString)
+{
+    AtomicStringImpl& value = *attribute->value().impl();
+
+    unsigned startSearchAt = 0;
+    while (true) {
+        size_t foundPos;
+        if (caseSensitivity == CaseSensitive)
+            foundPos = value.find(expectedString, startSearchAt);
+        else
+            foundPos = value.findIgnoringCase(expectedString, startSearchAt);
+        if (foundPos == notFound)
+            return false;
+        if (!foundPos || value[foundPos - 1] == ' ') {
+            unsigned endStr = foundPos + expectedString->length();
+            if (endStr == value.length() || value[endStr] == ' ')
+                return true;
+        }
+        startSearchAt = foundPos + 1;
+    }
+    return false;
+}
+
 void SelectorCodeGenerator::generateElementAttributeValueMatching(Assembler::JumpList& failureCases, Assembler::RegisterID currentAttributeAddress, const AttributeMatchingInfo& attributeInfo)
 {
     const CSSSelector& attributeSelector = attributeInfo.selector();
-    if (attributeSelector.m_match == CSSSelector::Set)
-        return;
-
     const AtomicString& expectedValue = attributeSelector.value();
     ASSERT(!expectedValue.isNull());
+    bool defaultToCaseSensitiveValueMatch = attributeInfo.canDefaultToCaseSensitiveValueMatch();
 
-    RELEASE_ASSERT(attributeSelector.m_match == CSSSelector::Exact);
-    generateElementAttributeValueExactMatching(failureCases, currentAttributeAddress, expectedValue, attributeInfo.canDefaultToCaseSensitiveValueMatch());
+    switch (attributeSelector.m_match) {
+    case CSSSelector::Begin:
+        generateElementAttributeFunctionCallValueMatching(failureCases, currentAttributeAddress, expectedValue, defaultToCaseSensitiveValueMatch, attributeValueBeginsWith<CaseSensitive>, attributeValueBeginsWith<CaseInsensitive>);
+        break;
+    case CSSSelector::Contain:
+        generateElementAttributeFunctionCallValueMatching(failureCases, currentAttributeAddress, expectedValue, defaultToCaseSensitiveValueMatch, attributeValueContains<CaseSensitive>, attributeValueContains<CaseInsensitive>);
+        break;
+    case CSSSelector::End:
+        generateElementAttributeFunctionCallValueMatching(failureCases, currentAttributeAddress, expectedValue, defaultToCaseSensitiveValueMatch, attributeValueEndsWith<CaseSensitive>, attributeValueEndsWith<CaseInsensitive>);
+        break;
+    case CSSSelector::Exact:
+        generateElementAttributeValueExactMatching(failureCases, currentAttributeAddress, expectedValue, defaultToCaseSensitiveValueMatch);
+        break;
+    case CSSSelector::Hyphen:
+        generateElementAttributeFunctionCallValueMatching(failureCases, currentAttributeAddress, expectedValue, defaultToCaseSensitiveValueMatch, attributeValueMatchHyphenRule<CaseSensitive>, attributeValueMatchHyphenRule<CaseInsensitive>);
+        break;
+    case CSSSelector::List:
+        generateElementAttributeFunctionCallValueMatching(failureCases, currentAttributeAddress, expectedValue, defaultToCaseSensitiveValueMatch, attributeValueSpaceSeparetedListContains<CaseSensitive>, attributeValueSpaceSeparetedListContains<CaseInsensitive>);
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
 }
 
 static inline Assembler::Jump testIsHTMLClassOnDocument(Assembler::ResultCondition condition, Assembler& assembler, Assembler::RegisterID documentAddress)
@@ -1235,6 +1341,49 @@
     }
 }
 
+void SelectorCodeGenerator::generateElementAttributeFunctionCallValueMatching(Assembler::JumpList& failureCases, Assembler::RegisterID currentAttributeAddress, const AtomicString& expectedValue, bool canDefaultToCaseSensitiveValueMatch, JSC::FunctionPtr caseSensitiveTest, JSC::FunctionPtr caseInsensitiveTest)
+{
+    LocalRegister expectedValueRegister(m_registerAllocator);
+    m_assembler.move(Assembler::TrustedImmPtr(expectedValue.impl()), expectedValueRegister);
+
+    if (canDefaultToCaseSensitiveValueMatch) {
+        FunctionCall functionCall(m_assembler, m_registerAllocator, m_stackAllocator, m_functionCalls);
+        functionCall.setFunctionAddress(caseSensitiveTest);
+        functionCall.setTwoArguments(currentAttributeAddress, expectedValueRegister);
+        failureCases.append(functionCall.callAndBranchOnCondition(Assembler::Zero));
+    } else {
+        Assembler::JumpList shouldUseCaseSensitiveComparison;
+        shouldUseCaseSensitiveComparison.append(testIsHTMLFlagOnNode(Assembler::Zero, m_assembler, elementAddressRegister));
+        {
+            LocalRegister scratchRegister(m_registerAllocator);
+            // scratchRegister = pointer to treeScope.
+            m_assembler.loadPtr(Assembler::Address(elementAddressRegister, Node::treeScopeMemoryOffset()), scratchRegister);
+            // scratchRegister = pointer to document.
+            m_assembler.loadPtr(Assembler::Address(scratchRegister, TreeScope::documentScopeMemoryOffset()), scratchRegister);
+            shouldUseCaseSensitiveComparison.append(testIsHTMLClassOnDocument(Assembler::Zero, m_assembler, scratchRegister));
+        }
+
+        {
+            FunctionCall functionCall(m_assembler, m_registerAllocator, m_stackAllocator, m_functionCalls);
+            functionCall.setFunctionAddress(caseInsensitiveTest);
+            functionCall.setTwoArguments(currentAttributeAddress, expectedValueRegister);
+            failureCases.append(functionCall.callAndBranchOnCondition(Assembler::Zero));
+        }
+
+        Assembler::Jump skipCaseSensitiveCase = m_assembler.jump();
+
+        {
+            shouldUseCaseSensitiveComparison.link(&m_assembler);
+            FunctionCall functionCall(m_assembler, m_registerAllocator, m_stackAllocator, m_functionCalls);
+            functionCall.setFunctionAddress(caseSensitiveTest);
+            functionCall.setTwoArguments(currentAttributeAddress, expectedValueRegister);
+            failureCases.append(functionCall.callAndBranchOnCondition(Assembler::Zero));
+        }
+
+        skipCaseSensitiveCase.link(&m_assembler);
+    }
+}
+
 void SelectorCodeGenerator::generateElementFunctionCallTest(Assembler::JumpList& failureCases, JSC::FunctionPtr testFunction)
 {
     Assembler::RegisterID elementAddress = elementAddressRegister;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to