[CSS][ARMv7] :nth-child() do not reserve enough registers if it is in backtracking...
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jul 2016 03:45:13 +0000 (03:45 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jul 2016 03:45:13 +0000 (03:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=159746
rdar://problem/26156169

Reviewed by Andreas Kling.

The generator generateElementIsNthChild() requires 6 registers in style resolution
to mark previous siblings with generateAddStyleRelationIfResolvingStyle() in the loop.

We were only reserving 5, which is a problem is the sixth is taken by the backtracking
register. x86_64 was already requiring 6 for unrelated reasons and ARM64 has so many registers
that you cannot possibly run out of them in CSS JIT.

I generalized the x86_64 path to all architectures.
I did not limit this case to style resolution because the extra register is irrelevant
in most cases. The only difference is one extra push/pop on ARMv7 if you use querySelector
with :nth-child in a backtracking chain.

This problem is covered by the existing test fast/selectors/nth-child-with-backtracking.html

* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::minimumRegisterRequirements): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@203211 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/cssjit/SelectorCompiler.cpp

index d365544..ca58b89 100644 (file)
@@ -1,3 +1,28 @@
+2016-07-13  Benjamin Poulain  <benjamin@webkit.org>
+
+        [CSS][ARMv7] :nth-child() do not reserve enough registers if it is in backtracking chain
+        https://bugs.webkit.org/show_bug.cgi?id=159746
+        rdar://problem/26156169
+
+        Reviewed by Andreas Kling.
+
+        The generator generateElementIsNthChild() requires 6 registers in style resolution
+        to mark previous siblings with generateAddStyleRelationIfResolvingStyle() in the loop.
+
+        We were only reserving 5, which is a problem is the sixth is taken by the backtracking
+        register. x86_64 was already requiring 6 for unrelated reasons and ARM64 has so many registers
+        that you cannot possibly run out of them in CSS JIT.
+
+        I generalized the x86_64 path to all architectures.
+        I did not limit this case to style resolution because the extra register is irrelevant
+        in most cases. The only difference is one extra push/pop on ARMv7 if you use querySelector
+        with :nth-child in a backtracking chain.
+
+        This problem is covered by the existing test fast/selectors/nth-child-with-backtracking.html
+
+        * cssjit/SelectorCompiler.cpp:
+        (WebCore::SelectorCompiler::minimumRegisterRequirements): Deleted.
+
 2016-07-13  Chris Dumez  <cdumez@apple.com>
 
         Drop unnecessary check from ContainerNode::removeChild()
index 63985d4..dbd29e9 100644 (file)
@@ -1066,10 +1066,10 @@ static inline bool attributeValueTestingRequiresExtraRegister(const AttributeMat
 static const unsigned minimumRequiredRegisterCount = 5;
 // Element + ElementData + scratchRegister + attributeArrayPointer + expectedLocalName + (qualifiedNameImpl && expectedValue).
 static const unsigned minimumRequiredRegisterCountForAttributeFilter = 6;
-#if CPU(X86_64)
-// Element + SiblingCounter + SiblingCounterCopy + divisor + dividend + remainder.
+// On x86, we always need 6 registers: Element + SiblingCounter + SiblingCounterCopy + divisor + dividend + remainder.
+// On other architectures, we need 6 registers for style resolution:
+//     Element + elementCounter + previousSibling + checkingContext + lastRelation + nextSiblingElement.
 static const unsigned minimumRequiredRegisterCountForNthChildFilter = 6;
-#endif
 
 static unsigned minimumRegisterRequirements(const SelectorFragment& selectorFragment)
 {
@@ -1093,10 +1093,8 @@ static unsigned minimumRegisterRequirements(const SelectorFragment& selectorFrag
         minimum = std::max(minimum, attributeMinimum);
     }
 
-#if CPU(X86_64)
     if (!selectorFragment.nthChildFilters.isEmpty() || !selectorFragment.nthChildOfFilters.isEmpty() || !selectorFragment.nthLastChildFilters.isEmpty() || !selectorFragment.nthLastChildOfFilters.isEmpty())
         minimum = std::max(minimum, minimumRequiredRegisterCountForNthChildFilter);
-#endif
 
     // :any pseudo class filters cause some register pressure.
     for (const auto& subFragments : selectorFragment.anyFilters) {