Changeset 203211 in webkit

Timestamp:

Jul 13, 2016, 8:45:13 PM (10 years ago)

Author:

benjamin@webkit.org

Message:

[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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• cssjit/SelectorCompiler.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/cssjit/SelectorCompiler.cpp

@@ -1067 +1067 @@
 // 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)
@@ -1094 +1094 @@
     }
 
-#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.