Title: [164961] trunk/Source/WebCore
Revision
164961
Author
[email protected]
Date
2014-03-02 17:48:20 -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 (164960 => 164961)


--- trunk/Source/WebCore/ChangeLog	2014-03-03 01:37:46 UTC (rev 164960)
+++ trunk/Source/WebCore/ChangeLog	2014-03-03 01:48:20 UTC (rev 164961)
@@ -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-03-02  Darin Adler  <[email protected]>
 
         Fix build for case-sensitive file systems.

Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (164960 => 164961)


--- trunk/Source/WebCore/css/SelectorChecker.cpp	2014-03-03 01:37:46 UTC (rev 164960)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp	2014-03-03 01:48:20 UTC (rev 164961)
@@ -280,10 +280,12 @@
     ASSERT(!value.isNull());
 
     switch (match) {
+    case CSSSelector::Set:
+        return true;
     case CSSSelector::Exact:
         if (caseSensitive ? selectorValue != value : !equalIgnoringCase(selectorValue, value))
             return false;
-        break;
+        return true;
     case CSSSelector::List:
         {
             // Ignore empty selectors or selectors containing spaces
@@ -304,20 +306,20 @@
                 // No match. Keep looking.
                 startSearchAt = foundPos + 1;
             }
-            break;
+            return true;
         }
     case CSSSelector::Contain:
         if (!value.contains(selectorValue, caseSensitive) || selectorValue.isEmpty())
             return false;
-        break;
+        return true;
     case CSSSelector::Begin:
         if (!value.startsWith(selectorValue, caseSensitive) || selectorValue.isEmpty())
             return false;
-        break;
+        return true;
     case CSSSelector::End:
         if (!value.endsWith(selectorValue, caseSensitive) || selectorValue.isEmpty())
             return false;
-        break;
+        return true;
     case CSSSelector::Hyphen:
         if (value.length() < selectorValue.length())
             return false;
@@ -326,14 +328,11 @@
         // It they start the same, check for exact match or following '-':
         if (value.length() != selectorValue.length() && value[selectorValue.length()] != '-')
             return false;
-        break;
-    case CSSSelector::PseudoClass:
-    case CSSSelector::PseudoElement:
+        return true;
     default:
-        break;
+        ASSERT_NOT_REACHED();
+        return false;
     }
-
-    return true;
 }
 
 static bool anyAttributeMatches(Element* element, const CSSSelector* selector, const QualifiedName& selectorAttr, bool caseSensitive)

Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (164960 => 164961)


--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2014-03-03 01:37:46 UTC (rev 164960)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2014-03-03 01:48:20 UTC (rev 164961)
@@ -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;
         }
@@ -1181,17 +1192,112 @@
     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 expectedStringPosition;
+        if (caseSensitivity == CaseSensitive)
+            expectedStringPosition = value.find(expectedString, startSearchAt);
+        else
+            expectedStringPosition = value.findIgnoringCase(expectedString, startSearchAt);
+        if (expectedStringPosition == notFound)
+            return false;
+        if (!expectedStringPosition || value[expectedStringPosition - 1] == ' ') {
+            unsigned positionAfterExpectedString = expectedStringPosition + expectedString->length();
+            if (positionAfterExpectedString == value.length() || value[positionAfterExpectedString] == ' ')
+                return true;
+        }
+        startSearchAt = expectedStringPosition + 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;
+    case CSSSelector::Set:
+        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