Title: [151889] branches/dfgFourthTier
Revision
151889
Author
[email protected]
Date
2013-06-23 18:41:11 -0700 (Sun, 23 Jun 2013)

Log Message

Merge trunk r147965.

Source/_javascript_Core: 

    2013-04-08  Filip Pizlo  <[email protected]>
    
    DFG should be able to inline string equality comparisons
    https://bugs.webkit.org/show_bug.cgi?id=114224
    
    Reviewed by Oliver Hunt.
            
    Inline 8-bit string equality, go to slow path for 16-bit strings. 2x speed-up for string equality
    comparisons on 8-bit strings. 20-50% speed-up on JSRegress/HashMap tests. 30% speed-up on
    string-fasta. 2% speed-up on SunSpider overall. Some small speed-ups elsewhere.
    
    This is a gnarly change but we have loads of test coverage already between the HashMap tests and
    preexisting DFG string equality tests (which appear to have been designed to test OSR exits, but
    also give us good overall coverage on string equality behavior).
    
    * dfg/DFGFixupPhase.cpp:
    (JSC::DFG::FixupPhase::fixupNode):
    * dfg/DFGOperations.cpp:
    * dfg/DFGOperations.h:
    * dfg/DFGSpeculativeJIT.cpp:
    (JSC::DFG::SpeculativeJIT::compilePeepHoleBranch):
    (JSC::DFG::SpeculativeJIT::compare):
    (JSC::DFG::SpeculativeJIT::compileStrictEq):
    (JSC::DFG::SpeculativeJIT::compileStringEquality):
    (DFG):
    * dfg/DFGSpeculativeJIT.h:
    (SpeculativeJIT):

LayoutTests: 

    2013-04-08  Filip Pizlo  <[email protected]>
    
    DFG should be able to inline string equality comparisons
    https://bugs.webkit.org/show_bug.cgi?id=114224
    
    Reviewed by Oliver Hunt.
    
    * fast/js/regress/script-tests/string-equality.js: Added.
    (foo):
    * fast/js/regress/string-equality-expected.txt: Added.
    * fast/js/regress/string-equality.html: Added.

Modified Paths

Added Paths

Diff

Modified: branches/dfgFourthTier/LayoutTests/ChangeLog (151888 => 151889)


--- branches/dfgFourthTier/LayoutTests/ChangeLog	2013-06-24 00:40:06 UTC (rev 151888)
+++ branches/dfgFourthTier/LayoutTests/ChangeLog	2013-06-24 01:41:11 UTC (rev 151889)
@@ -1,3 +1,19 @@
+2013-06-23  Filip Pizlo  <[email protected]>
+
+        Merge trunk r147965.
+
+    2013-04-08  Filip Pizlo  <[email protected]>
+    
+            DFG should be able to inline string equality comparisons
+            https://bugs.webkit.org/show_bug.cgi?id=114224
+    
+            Reviewed by Oliver Hunt.
+    
+            * fast/js/regress/script-tests/string-equality.js: Added.
+            (foo):
+            * fast/js/regress/string-equality-expected.txt: Added.
+            * fast/js/regress/string-equality.html: Added.
+    
 2013-06-21  Filip Pizlo  <[email protected]>
 
         fourthTier: DFG shouldn't exit just because a String GetByVal went out-of-bounds

Added: branches/dfgFourthTier/LayoutTests/fast/js/regress/script-tests/string-equality.js (0 => 151889)


--- branches/dfgFourthTier/LayoutTests/fast/js/regress/script-tests/string-equality.js	                        (rev 0)
+++ branches/dfgFourthTier/LayoutTests/fast/js/regress/script-tests/string-equality.js	2013-06-24 01:41:11 UTC (rev 151889)
@@ -0,0 +1,16 @@
+var array = [ "a", "b", "c", "d" ];
+
+function foo(array, s) {
+    for (var i = 0; i < array.length; ++i) {
+        if (array[i] == s)
+            return true;
+    }
+    return false;
+}
+
+var result = 0;
+for (var i = 0; i < 1000000; ++i)
+    result += foo(array, "d");
+
+if (result != 1000000)
+    throw "Bad result: " + result;

Added: branches/dfgFourthTier/LayoutTests/fast/js/regress/string-equality-expected.txt (0 => 151889)


--- branches/dfgFourthTier/LayoutTests/fast/js/regress/string-equality-expected.txt	                        (rev 0)
+++ branches/dfgFourthTier/LayoutTests/fast/js/regress/string-equality-expected.txt	2013-06-24 01:41:11 UTC (rev 151889)
@@ -0,0 +1,10 @@
+JSRegress/string-equality
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no exception thrown
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: branches/dfgFourthTier/LayoutTests/fast/js/regress/string-equality.html (0 => 151889)


--- branches/dfgFourthTier/LayoutTests/fast/js/regress/string-equality.html	                        (rev 0)
+++ branches/dfgFourthTier/LayoutTests/fast/js/regress/string-equality.html	2013-06-24 01:41:11 UTC (rev 151889)
@@ -0,0 +1,12 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
+</body>
+</html>

Modified: branches/dfgFourthTier/Source/_javascript_Core/ChangeLog (151888 => 151889)


--- branches/dfgFourthTier/Source/_javascript_Core/ChangeLog	2013-06-24 00:40:06 UTC (rev 151888)
+++ branches/dfgFourthTier/Source/_javascript_Core/ChangeLog	2013-06-24 01:41:11 UTC (rev 151889)
@@ -1,3 +1,35 @@
+2013-06-23  Filip Pizlo  <[email protected]>
+
+        Merge trunk r147965.
+
+    2013-04-08  Filip Pizlo  <[email protected]>
+    
+            DFG should be able to inline string equality comparisons
+            https://bugs.webkit.org/show_bug.cgi?id=114224
+    
+            Reviewed by Oliver Hunt.
+            
+            Inline 8-bit string equality, go to slow path for 16-bit strings. 2x speed-up for string equality
+            comparisons on 8-bit strings. 20-50% speed-up on JSRegress/HashMap tests. 30% speed-up on
+            string-fasta. 2% speed-up on SunSpider overall. Some small speed-ups elsewhere.
+    
+            This is a gnarly change but we have loads of test coverage already between the HashMap tests and
+            preexisting DFG string equality tests (which appear to have been designed to test OSR exits, but
+            also give us good overall coverage on string equality behavior).
+    
+            * dfg/DFGFixupPhase.cpp:
+            (JSC::DFG::FixupPhase::fixupNode):
+            * dfg/DFGOperations.cpp:
+            * dfg/DFGOperations.h:
+            * dfg/DFGSpeculativeJIT.cpp:
+            (JSC::DFG::SpeculativeJIT::compilePeepHoleBranch):
+            (JSC::DFG::SpeculativeJIT::compare):
+            (JSC::DFG::SpeculativeJIT::compileStrictEq):
+            (JSC::DFG::SpeculativeJIT::compileStringEquality):
+            (DFG):
+            * dfg/DFGSpeculativeJIT.h:
+            (SpeculativeJIT):
+    
 2013-06-22  Filip Pizlo  <[email protected]>
 
         fourthTier: DFG shouldn't exit just because a String GetByVal went out-of-bounds

Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (151888 => 151889)


--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2013-06-24 00:40:06 UTC (rev 151888)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2013-06-24 01:41:11 UTC (rev 151889)
@@ -283,14 +283,15 @@
             break;
         }
             
+        case CompareEqConstant: {
+            break;
+        }
+
         case CompareEq:
-        case CompareEqConstant:
         case CompareLess:
         case CompareLessEq:
         case CompareGreater:
         case CompareGreaterEq: {
-            if (node->op() == CompareEqConstant)
-                break;
             if (Node::shouldSpeculateInteger(node->child1().node(), node->child2().node())) {
                 setUseKindAndUnboxIfProfitable<Int32Use>(node->child1());
                 setUseKindAndUnboxIfProfitable<Int32Use>(node->child2());
@@ -303,8 +304,11 @@
             }
             if (node->op() != CompareEq)
                 break;
-            if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString())
+            if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString() && GPRInfo::numberOfRegisters >= 7) {
+                setUseKindAndUnboxIfProfitable<StringUse>(node->child1());
+                setUseKindAndUnboxIfProfitable<StringUse>(node->child2());
                 break;
+            }
             if (node->child1()->shouldSpeculateObject() && node->child2()->shouldSpeculateObject()) {
                 setUseKindAndUnboxIfProfitable<ObjectUse>(node->child1());
                 setUseKindAndUnboxIfProfitable<ObjectUse>(node->child2());
@@ -323,10 +327,11 @@
             break;
         }
             
-        case CompareStrictEq:
         case CompareStrictEqConstant: {
-            if (node->op() == CompareStrictEqConstant)
-                break;
+            break;
+        }
+            
+        case CompareStrictEq: {
             if (Node::shouldSpeculateInteger(node->child1().node(), node->child2().node())) {
                 setUseKindAndUnboxIfProfitable<Int32Use>(node->child1());
                 setUseKindAndUnboxIfProfitable<Int32Use>(node->child2());
@@ -337,8 +342,11 @@
                 fixDoubleEdge<NumberUse>(node->child2());
                 break;
             }
-            if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString())
+            if (node->child1()->shouldSpeculateString() && node->child2()->shouldSpeculateString() && GPRInfo::numberOfRegisters >= 7) {
+                setUseKindAndUnboxIfProfitable<StringUse>(node->child1());
+                setUseKindAndUnboxIfProfitable<StringUse>(node->child2());
                 break;
+            }
             if (node->child1()->shouldSpeculateObject() && node->child2()->shouldSpeculateObject()) {
                 setUseKindAndUnboxIfProfitable<ObjectUse>(node->child1());
                 setUseKindAndUnboxIfProfitable<ObjectUse>(node->child2());

Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGOperations.cpp (151888 => 151889)


--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGOperations.cpp	2013-06-24 00:40:06 UTC (rev 151888)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGOperations.cpp	2013-06-24 01:41:11 UTC (rev 151889)
@@ -1048,6 +1048,23 @@
     return JSValue::equalSlowCaseInline(exec, JSValue::decode(encodedOp1), JSValue::decode(encodedOp2));
 }
 
+#if USE(JSVALUE64)
+EncodedJSValue DFG_OPERATION operationCompareStringEq(ExecState* exec, JSCell* left, JSCell* right)
+#else
+size_t DFG_OPERATION operationCompareStringEq(ExecState* exec, JSCell* left, JSCell* right)
+#endif
+{
+    VM* vm = &exec->vm();
+    NativeCallFrameTracer tracer(vm, exec);
+    
+    bool result = asString(left)->value(exec) == asString(right)->value(exec);
+#if USE(JSVALUE64)
+    return JSValue::encode(jsBoolean(result));
+#else
+    return result;
+#endif
+}
+
 size_t DFG_OPERATION operationCompareStrictEqCell(ExecState* exec, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2)
 {
     VM* vm = &exec->vm();

Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGOperations.h (151888 => 151889)


--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGOperations.h	2013-06-24 00:40:06 UTC (rev 151888)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGOperations.h	2013-06-24 01:41:11 UTC (rev 151889)
@@ -180,6 +180,11 @@
 size_t DFG_OPERATION operationCompareGreater(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
 size_t DFG_OPERATION operationCompareGreaterEq(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
 size_t DFG_OPERATION operationCompareEq(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
+#if USE(JSVALUE64)
+EncodedJSValue DFG_OPERATION operationCompareStringEq(ExecState*, JSCell* left, JSCell* right) WTF_INTERNAL;
+#else
+size_t DFG_OPERATION operationCompareStringEq(ExecState*, JSCell* left, JSCell* right) WTF_INTERNAL;
+#endif
 size_t DFG_OPERATION operationCompareStrictEqCell(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
 size_t DFG_OPERATION operationCompareStrictEq(ExecState*, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2) WTF_INTERNAL;
 char* DFG_OPERATION operationVirtualCall(ExecState*) WTF_INTERNAL;

Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (151888 => 151889)


--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2013-06-24 00:40:06 UTC (rev 151888)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2013-06-24 01:41:11 UTC (rev 151889)
@@ -1515,8 +1515,8 @@
             compilePeepHoleDoubleBranch(node, branchNode, doubleCondition);
         else if (node->op() == CompareEq) {
             if (node->isBinaryUseKind(StringUse)) {
-                nonSpeculativePeepholeBranch(node, branchNode, condition, operation);
-                return true;
+                // Use non-peephole comparison, for now.
+                return false;
             }
             if (node->isBinaryUseKind(ObjectUse))
                 compilePeepHoleObjectEquality(node, branchNode);
@@ -3623,7 +3623,7 @@
     
     if (node->op() == CompareEq) {
         if (node->isBinaryUseKind(StringUse)) {
-            nonSpeculativeNonPeepholeCompare(node, condition, operation);
+            compileStringEquality(node);
             return false;
         }
         
@@ -3757,7 +3757,8 @@
     }
         
     case StringUse: {
-        return nonSpeculativeStrictEq(node);
+        compileStringEquality(node);
+        return false;
     }
         
     case ObjectUse: {
@@ -3785,6 +3786,111 @@
     }
 }
 
+void SpeculativeJIT::compileStringEquality(Node* node)
+{
+    SpeculateCellOperand left(this, node->child1());
+    SpeculateCellOperand right(this, node->child2());
+    GPRTemporary length(this);
+    GPRTemporary leftTemp(this);
+    GPRTemporary rightTemp(this);
+    GPRTemporary leftTemp2(this, left);
+    GPRTemporary rightTemp2(this, right);
+    
+    GPRReg leftGPR = left.gpr();
+    GPRReg rightGPR = right.gpr();
+    GPRReg lengthGPR = length.gpr();
+    GPRReg leftTempGPR = leftTemp.gpr();
+    GPRReg rightTempGPR = rightTemp.gpr();
+    GPRReg leftTemp2GPR = leftTemp2.gpr();
+    GPRReg rightTemp2GPR = rightTemp2.gpr();
+    
+    JITCompiler::JumpList trueCase;
+    JITCompiler::JumpList falseCase;
+    JITCompiler::JumpList slowCase;
+    
+    DFG_TYPE_CHECK(
+        JSValueSource::unboxedCell(leftGPR), node->child1(), SpecString, m_jit.branchPtr(
+            MacroAssembler::NotEqual,
+            MacroAssembler::Address(leftGPR, JSCell::structureOffset()),
+            MacroAssembler::TrustedImmPtr(m_jit.vm()->stringStructure.get())));
+    
+    // It's safe to branch around the type check below, since proving that the values are
+    // equal does indeed prove that the right value is a string.
+    trueCase.append(m_jit.branchPtr(MacroAssembler::Equal, leftGPR, rightGPR));
+    
+    DFG_TYPE_CHECK(
+        JSValueSource::unboxedCell(rightGPR), node->child2(), SpecString, m_jit.branchPtr(
+            MacroAssembler::NotEqual,
+            MacroAssembler::Address(rightGPR, JSCell::structureOffset()),
+            MacroAssembler::TrustedImmPtr(m_jit.vm()->stringStructure.get())));
+
+    m_jit.load32(MacroAssembler::Address(leftGPR, JSString::offsetOfLength()), lengthGPR);
+    
+    falseCase.append(m_jit.branch32(
+        MacroAssembler::NotEqual,
+        MacroAssembler::Address(rightGPR, JSString::offsetOfLength()),
+        lengthGPR));
+    
+    trueCase.append(m_jit.branchTest32(MacroAssembler::Zero, lengthGPR));
+    
+    m_jit.loadPtr(MacroAssembler::Address(leftGPR, JSString::offsetOfValue()), leftTempGPR);
+    m_jit.loadPtr(MacroAssembler::Address(rightGPR, JSString::offsetOfValue()), rightTempGPR);
+    
+    slowCase.append(m_jit.branchTestPtr(MacroAssembler::Zero, leftTempGPR));
+    slowCase.append(m_jit.branchTestPtr(MacroAssembler::Zero, rightTempGPR));
+    
+    slowCase.append(m_jit.branchTest32(
+        MacroAssembler::Zero,
+        MacroAssembler::Address(leftTempGPR, StringImpl::flagsOffset()),
+        TrustedImm32(StringImpl::flagIs8Bit())));
+    slowCase.append(m_jit.branchTest32(
+        MacroAssembler::Zero,
+        MacroAssembler::Address(rightTempGPR, StringImpl::flagsOffset()),
+        TrustedImm32(StringImpl::flagIs8Bit())));
+    
+    m_jit.loadPtr(MacroAssembler::Address(leftTempGPR, StringImpl::dataOffset()), leftTempGPR);
+    m_jit.loadPtr(MacroAssembler::Address(rightTempGPR, StringImpl::dataOffset()), rightTempGPR);
+    
+    MacroAssembler::Label loop = m_jit.label();
+    
+    m_jit.sub32(TrustedImm32(1), lengthGPR);
+
+    // This isn't going to generate the best code on x86. But that's OK, it's still better
+    // than not inlining.
+    m_jit.load8(MacroAssembler::BaseIndex(leftTempGPR, lengthGPR, MacroAssembler::TimesOne), leftTemp2GPR);
+    m_jit.load8(MacroAssembler::BaseIndex(rightTempGPR, lengthGPR, MacroAssembler::TimesOne), rightTemp2GPR);
+    falseCase.append(m_jit.branch32(MacroAssembler::NotEqual, leftTemp2GPR, rightTemp2GPR));
+    
+    m_jit.branchTest32(MacroAssembler::NonZero, lengthGPR).linkTo(loop, &m_jit);
+    
+    trueCase.link(&m_jit);
+#if USE(JSVALUE64)
+    m_jit.move(TrustedImm64(ValueTrue), leftTempGPR);
+#else
+    m_jit.move(TrustedImm32(true), leftTempGPR);
+#endif
+    
+    JITCompiler::Jump done = m_jit.jump();
+
+    falseCase.link(&m_jit);
+#if USE(JSVALUE64)
+    m_jit.move(TrustedImm64(ValueFalse), leftTempGPR);
+#else
+    m_jit.move(TrustedImm32(false), leftTempGPR);
+#endif
+    
+    done.link(&m_jit);
+    addSlowPathGenerator(
+        slowPathCall(
+            slowCase, this, operationCompareStringEq, leftTempGPR, leftGPR, rightGPR));
+    
+#if USE(JSVALUE64)
+    jsValueResult(leftTempGPR, node, DataFormatJSBoolean);
+#else
+    booleanResult(leftTempGPR, node);
+#endif
+}
+
 void SpeculativeJIT::compileGetIndexedPropertyStorage(Node* node)
 {
     SpeculateCellOperand base(this, node->child1());

Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (151888 => 151889)


--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2013-06-24 00:40:06 UTC (rev 151888)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h	2013-06-24 01:41:11 UTC (rev 151889)
@@ -1858,6 +1858,7 @@
     void compileValueAdd(Node*);
     void compileObjectOrOtherLogicalNot(Edge value);
     void compileLogicalNot(Node*);
+    void compileStringEquality(Node*);
     void emitObjectOrOtherBranch(Edge value, BlockIndex taken, BlockIndex notTaken);
     void emitBranch(Node*);
     void emitSwitchIntJump(SwitchData*, GPRReg value, GPRReg scratch);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to