Title: [185002] trunk
Revision
185002
Author
fpi...@apple.com
Date
2015-05-29 13:26:37 -0700 (Fri, 29 May 2015)

Log Message

Non-speculative Branch should be fast in the FTL
https://bugs.webkit.org/show_bug.cgi?id=145452

Reviewed by Andreas Kling.
Source/_javascript_Core:

        
Inlines the code for convertJSValueToBoolean into the FTL. This also includes some other
clean-ups that I found along the way.
        
I found this by looking at the hottest functions in DeltaBlue. Despite having so many
Branch specializations, apparently there was still a hot one that we missed that was going
down the untyped path. It was either Int32 or Other. Maybe we could specialize for that
combo, but it makes so much sense to just make all of this nonsense fast.

* dfg/DFGWatchpointCollectionPhase.cpp:
(JSC::DFG::WatchpointCollectionPhase::handle): Need to watch the masquerades watchpoint on UntypedUse: forms of Branch now.
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::boolify): The actual fix.
(JSC::FTL::LowerDFGToLLVM::int52ToStrictInt52):
(JSC::FTL::LowerDFGToLLVM::isInt32):
(JSC::FTL::LowerDFGToLLVM::isNotInt32):
(JSC::FTL::LowerDFGToLLVM::unboxInt32):
* runtime/JSCellInlines.h:
(JSC::JSCell::toBoolean): Symbol is always true.
(JSC::JSCell::pureToBoolean): Symbol is always true.
* runtime/JSString.cpp:
(JSC::JSString::getPrimitiveNumber):
(JSC::JSString::toNumber):
(JSC::JSString::toBoolean): Deleted. This is a tiny method. It doesn't need to be out-of-line.
* runtime/JSString.h:
(JSC::JSString::length):
(JSC::JSString::toBoolean): This method shouldbe inline.
* runtime/Symbol.cpp:
(JSC::Symbol::toPrimitive):
(JSC::Symbol::getPrimitiveNumber):
(JSC::Symbol::toBoolean): Deleted. A Symbol is always true, so we don't need a method for this.
* runtime/Symbol.h:

LayoutTests:


* js/regress/logical-not-weird-types-expected.txt: Added.
* js/regress/logical-not-weird-types.html: Added.
* js/regress/script-tests/logical-not-weird-types.js: Added.
(foo):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (185001 => 185002)


--- trunk/LayoutTests/ChangeLog	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/LayoutTests/ChangeLog	2015-05-29 20:26:37 UTC (rev 185002)
@@ -1,3 +1,15 @@
+2015-05-28  Filip Pizlo  <fpi...@apple.com>
+
+        Non-speculative Branch should be fast in the FTL
+        https://bugs.webkit.org/show_bug.cgi?id=145452
+
+        Reviewed by Andreas Kling.
+
+        * js/regress/logical-not-weird-types-expected.txt: Added.
+        * js/regress/logical-not-weird-types.html: Added.
+        * js/regress/script-tests/logical-not-weird-types.js: Added.
+        (foo):
+
 2015-05-29  Filip Pizlo  <fpi...@apple.com>
 
         Land some .html/-expected.txt files for some tests that were added without them.

Added: trunk/LayoutTests/js/regress/logical-not-weird-types-expected.txt (0 => 185002)


--- trunk/LayoutTests/js/regress/logical-not-weird-types-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/js/regress/logical-not-weird-types-expected.txt	2015-05-29 20:26:37 UTC (rev 185002)
@@ -0,0 +1,10 @@
+JSRegress/logical-not-weird-types
+
+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: trunk/LayoutTests/js/regress/logical-not-weird-types.html (0 => 185002)


--- trunk/LayoutTests/js/regress/logical-not-weird-types.html	                        (rev 0)
+++ trunk/LayoutTests/js/regress/logical-not-weird-types.html	2015-05-29 20:26:37 UTC (rev 185002)
@@ -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>

Added: trunk/LayoutTests/js/regress/script-tests/logical-not-weird-types.js (0 => 185002)


--- trunk/LayoutTests/js/regress/script-tests/logical-not-weird-types.js	                        (rev 0)
+++ trunk/LayoutTests/js/regress/script-tests/logical-not-weird-types.js	2015-05-29 20:26:37 UTC (rev 185002)
@@ -0,0 +1,32 @@
+function foo(value) {
+    return !!value;
+}
+
+noInline(foo);
+
+var tests = [
+    [0, false],
+    [1, true],
+    [0/0, false],
+    [0/-1, false],
+    [0.0, false],
+    ["", false],
+    ["f", true],
+    ["hello", true],
+    [{}, true],
+    [[], true],
+    [null, false],
+    [void 0, false],
+    [false, false],
+    [true, true]
+];
+
+for (var i = 0; i < 10000; ++i) {
+    for (var j = 0; j < tests.length; ++j) {
+        var input = tests[j][0];
+        var expected = tests[j][1];
+        var result = foo(input);
+        if (result !== expected)
+            throw "Error: bad result for " + input + ": " + result;
+    }
+}

Modified: trunk/Source/_javascript_Core/ChangeLog (185001 => 185002)


--- trunk/Source/_javascript_Core/ChangeLog	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-05-29 20:26:37 UTC (rev 185002)
@@ -1,3 +1,42 @@
+2015-05-28  Filip Pizlo  <fpi...@apple.com>
+
+        Non-speculative Branch should be fast in the FTL
+        https://bugs.webkit.org/show_bug.cgi?id=145452
+
+        Reviewed by Andreas Kling.
+        
+        Inlines the code for convertJSValueToBoolean into the FTL. This also includes some other
+        clean-ups that I found along the way.
+        
+        I found this by looking at the hottest functions in DeltaBlue. Despite having so many
+        Branch specializations, apparently there was still a hot one that we missed that was going
+        down the untyped path. It was either Int32 or Other. Maybe we could specialize for that
+        combo, but it makes so much sense to just make all of this nonsense fast.
+
+        * dfg/DFGWatchpointCollectionPhase.cpp:
+        (JSC::DFG::WatchpointCollectionPhase::handle): Need to watch the masquerades watchpoint on UntypedUse: forms of Branch now.
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::boolify): The actual fix.
+        (JSC::FTL::LowerDFGToLLVM::int52ToStrictInt52):
+        (JSC::FTL::LowerDFGToLLVM::isInt32):
+        (JSC::FTL::LowerDFGToLLVM::isNotInt32):
+        (JSC::FTL::LowerDFGToLLVM::unboxInt32):
+        * runtime/JSCellInlines.h:
+        (JSC::JSCell::toBoolean): Symbol is always true.
+        (JSC::JSCell::pureToBoolean): Symbol is always true.
+        * runtime/JSString.cpp:
+        (JSC::JSString::getPrimitiveNumber):
+        (JSC::JSString::toNumber):
+        (JSC::JSString::toBoolean): Deleted. This is a tiny method. It doesn't need to be out-of-line.
+        * runtime/JSString.h:
+        (JSC::JSString::length):
+        (JSC::JSString::toBoolean): This method shouldbe inline.
+        * runtime/Symbol.cpp:
+        (JSC::Symbol::toPrimitive):
+        (JSC::Symbol::getPrimitiveNumber):
+        (JSC::Symbol::toBoolean): Deleted. A Symbol is always true, so we don't need a method for this.
+        * runtime/Symbol.h:
+
 2015-05-29  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, rolling out r184860.

Modified: trunk/Source/_javascript_Core/dfg/DFGWatchpointCollectionPhase.cpp (185001 => 185002)


--- trunk/Source/_javascript_Core/dfg/DFGWatchpointCollectionPhase.cpp	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/Source/_javascript_Core/dfg/DFGWatchpointCollectionPhase.cpp	2015-05-29 20:26:37 UTC (rev 185002)
@@ -87,8 +87,14 @@
             
         case LogicalNot:
         case Branch:
-            if (m_node->child1().useKind() == ObjectOrOtherUse)
+            switch (m_node->child1().useKind()) {
+            case ObjectOrOtherUse:
+            case UntypedUse:
                 handleMasqueradesAsUndefined();
+                break;
+            default:
+                break;
+            }
             break;
             
         case NewArray:

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (185001 => 185002)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp	2015-05-29 20:26:37 UTC (rev 185002)
@@ -6127,9 +6127,9 @@
     {
         switch (edge.useKind()) {
         case BooleanUse:
-            return lowBoolean(m_node->child1());
+            return lowBoolean(edge);
         case Int32Use:
-            return m_out.notZero32(lowInt32(m_node->child1()));
+            return m_out.notZero32(lowInt32(edge));
         case DoubleRepUse:
             return m_out.doubleNotEqual(lowDouble(edge), m_out.doubleZero);
         case ObjectOrOtherUse:
@@ -6138,32 +6138,108 @@
                     edge, CellCaseSpeculatesObject, SpeculateNullOrUndefined,
                     ManualOperandSpeculation));
         case StringUse: {
-            LValue stringValue = lowString(m_node->child1());
+            LValue stringValue = lowString(edge);
             LValue length = m_out.load32NonNegative(stringValue, m_heaps.JSString_length);
             return m_out.notEqual(length, m_out.int32Zero);
         }
         case UntypedUse: {
-            LValue value = lowJSValue(m_node->child1());
+            LValue value = lowJSValue(edge);
             
-            LBasicBlock slowCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped slow case"));
-            LBasicBlock fastCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped fast case"));
+            // Implements the following control flow structure:
+            // if (value is cell) {
+            //     if (value is string)
+            //         result = !!value->length
+            //     else {
+            //         do evil things for masquerades-as-undefined
+            //         result = true
+            //     }
+            // } else if (value is int32) {
+            //     result = !!unboxInt32(value)
+            // } else if (value is number) {
+            //     result = !!unboxDouble(value)
+            // } else {
+            //     result = value == jsTrue
+            // }
+            
+            LBasicBlock cellCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped cell case"));
+            LBasicBlock stringCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped string case"));
+            LBasicBlock notStringCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped not string case"));
+            LBasicBlock notCellCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped not cell case"));
+            LBasicBlock int32Case = FTL_NEW_BLOCK(m_out, ("Boolify untyped int32 case"));
+            LBasicBlock notInt32Case = FTL_NEW_BLOCK(m_out, ("Boolify untyped not int32 case"));
+            LBasicBlock doubleCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped double case"));
+            LBasicBlock notDoubleCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped not double case"));
             LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("Boolify untyped continuation"));
             
+            Vector<ValueFromBlock> results;
+            
+            m_out.branch(isCell(value, provenType(edge)), unsure(cellCase), unsure(notCellCase));
+            
+            LBasicBlock lastNext = m_out.appendTo(cellCase, stringCase);
             m_out.branch(
-                isNotBoolean(value, provenType(m_node->child1())),
-                rarely(slowCase), usually(fastCase));
+                isString(value, provenType(edge) & SpecCell),
+                unsure(stringCase), unsure(notStringCase));
             
-            LBasicBlock lastNext = m_out.appendTo(fastCase, slowCase);
-            ValueFromBlock fastResult = m_out.anchor(unboxBoolean(value));
+            m_out.appendTo(stringCase, notStringCase);
+            LValue nonEmptyString = m_out.notZero32(
+                m_out.load32NonNegative(value, m_heaps.JSString_length));
+            results.append(m_out.anchor(nonEmptyString));
             m_out.jump(continuation);
             
-            m_out.appendTo(slowCase, continuation);
-            ValueFromBlock slowResult = m_out.anchor(m_out.notNull(vmCall(
-                m_out.operation(operationConvertJSValueToBoolean), m_callFrame, value)));
+            m_out.appendTo(notStringCase, notCellCase);
+            LValue isTruthyObject;
+            if (masqueradesAsUndefinedWatchpointIsStillValid())
+                isTruthyObject = m_out.booleanTrue;
+            else {
+                LBasicBlock masqueradesCase = FTL_NEW_BLOCK(m_out, ("Boolify untyped masquerades case"));
+                
+                results.append(m_out.anchor(m_out.booleanFalse));
+                
+                m_out.branch(
+                    m_out.testIsZero8(
+                        m_out.load8(value, m_heaps.JSCell_typeInfoFlags),
+                        m_out.constInt8(MasqueradesAsUndefined)),
+                    usually(continuation), rarely(masqueradesCase));
+                
+                m_out.appendTo(masqueradesCase);
+                
+                isTruthyObject = m_out.notEqual(
+                    m_out.constIntPtr(m_graph.globalObjectFor(m_node->origin.semantic)),
+                    m_out.loadPtr(loadStructure(value), m_heaps.Structure_globalObject));
+            }
+            results.append(m_out.anchor(isTruthyObject));
             m_out.jump(continuation);
             
+            m_out.appendTo(notCellCase, int32Case);
+            m_out.branch(
+                isInt32(value, provenType(edge) & ~SpecCell),
+                unsure(int32Case), unsure(notInt32Case));
+            
+            m_out.appendTo(int32Case, notInt32Case);
+            results.append(m_out.anchor(m_out.notZero32(unboxInt32(value))));
+            m_out.jump(continuation);
+            
+            m_out.appendTo(notInt32Case, doubleCase);
+            m_out.branch(
+                isNumber(value, provenType(edge) & ~SpecCell),
+                unsure(doubleCase), unsure(notDoubleCase));
+            
+            m_out.appendTo(doubleCase, notDoubleCase);
+            // Note that doubleNotEqual() really means not-equal-and-ordered. It will return false
+            // if value is NaN.
+            LValue doubleIsTruthy = m_out.doubleNotEqual(
+                unboxDouble(value), m_out.constDouble(0));
+            results.append(m_out.anchor(doubleIsTruthy));
+            m_out.jump(continuation);
+            
+            m_out.appendTo(notDoubleCase, continuation);
+            LValue miscIsTruthy = m_out.equal(
+                value, m_out.constInt64(JSValue::encode(jsBoolean(true))));
+            results.append(m_out.anchor(miscIsTruthy));
+            m_out.jump(continuation);
+            
             m_out.appendTo(continuation, lastNext);
-            return m_out.phi(m_out.boolean, fastResult, slowResult);
+            return m_out.phi(m_out.boolean, results);
         }
         default:
             DFG_CRASH(m_graph, m_node, "Bad use kind");
@@ -7168,8 +7244,16 @@
         return m_out.aShr(value, m_out.constInt64(JSValue::int52ShiftAmount));
     }
     
-    LValue isNotInt32(LValue jsValue)
+    LValue isInt32(LValue jsValue, SpeculatedType type = SpecFullTop)
     {
+        if (LValue proven = isProvenValue(type, SpecInt32))
+            return proven;
+        return m_out.aboveOrEqual(jsValue, m_tagTypeNumber);
+    }
+    LValue isNotInt32(LValue jsValue, SpeculatedType type = SpecFullTop)
+    {
+        if (LValue proven = isProvenValue(type, ~SpecInt32))
+            return proven;
         return m_out.below(jsValue, m_tagTypeNumber);
     }
     LValue unboxInt32(LValue jsValue)

Modified: trunk/Source/_javascript_Core/runtime/JSCellInlines.h (185001 => 185002)


--- trunk/Source/_javascript_Core/runtime/JSCellInlines.h	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/Source/_javascript_Core/runtime/JSCellInlines.h	2015-05-29 20:26:37 UTC (rev 185002)
@@ -250,8 +250,6 @@
 {
     if (isString())
         return static_cast<const JSString*>(this)->toBoolean();
-    if (isSymbol())
-        return static_cast<const Symbol*>(this)->toBoolean();
     return !structure()->masqueradesAsUndefined(exec->lexicalGlobalObject());
 }
 
@@ -260,7 +258,7 @@
     if (isString())
         return static_cast<const JSString*>(this)->toBoolean() ? TrueTriState : FalseTriState;
     if (isSymbol())
-        return static_cast<const Symbol*>(this)->toBoolean() ? TrueTriState : FalseTriState;
+        return TrueTriState;
     return MixedTriState;
 }
 

Modified: trunk/Source/_javascript_Core/runtime/JSString.cpp (185001 => 185002)


--- trunk/Source/_javascript_Core/runtime/JSString.cpp	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/Source/_javascript_Core/runtime/JSString.cpp	2015-05-29 20:26:37 UTC (rev 185002)
@@ -389,11 +389,6 @@
     return false;
 }
 
-bool JSString::toBoolean() const
-{
-    return m_length;
-}
-
 double JSString::toNumber(ExecState* exec) const
 {
     return jsToNumber(view(exec));

Modified: trunk/Source/_javascript_Core/runtime/JSString.h (185001 => 185002)


--- trunk/Source/_javascript_Core/runtime/JSString.h	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/Source/_javascript_Core/runtime/JSString.h	2015-05-29 20:26:37 UTC (rev 185002)
@@ -150,7 +150,7 @@
     unsigned length() const { return m_length; }
 
     JSValue toPrimitive(ExecState*, PreferredPrimitiveType) const;
-    JS_EXPORT_PRIVATE bool toBoolean() const;
+    bool toBoolean() const { return !!m_length; }
     bool getPrimitiveNumber(ExecState*, double& number, JSValue&) const;
     JSObject* toObject(ExecState*, JSGlobalObject*) const;
     double toNumber(ExecState*) const;

Modified: trunk/Source/_javascript_Core/runtime/Symbol.cpp (185001 => 185002)


--- trunk/Source/_javascript_Core/runtime/Symbol.cpp	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/Source/_javascript_Core/runtime/Symbol.cpp	2015-05-29 20:26:37 UTC (rev 185002)
@@ -65,11 +65,6 @@
     return const_cast<Symbol*>(this);
 }
 
-bool Symbol::toBoolean() const
-{
-    return true;
-}
-
 bool Symbol::getPrimitiveNumber(ExecState* exec, double& number, JSValue& result) const
 {
     result = this;

Modified: trunk/Source/_javascript_Core/runtime/Symbol.h (185001 => 185002)


--- trunk/Source/_javascript_Core/runtime/Symbol.h	2015-05-29 20:13:09 UTC (rev 185001)
+++ trunk/Source/_javascript_Core/runtime/Symbol.h	2015-05-29 20:26:37 UTC (rev 185002)
@@ -75,7 +75,6 @@
     String descriptiveString() const;
 
     JSValue toPrimitive(ExecState*, PreferredPrimitiveType) const;
-    JS_EXPORT_PRIVATE bool toBoolean() const;
     bool getPrimitiveNumber(ExecState*, double& number, JSValue&) const;
     JSObject* toObject(ExecState*, JSGlobalObject*) const;
     double toNumber(ExecState*) const;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to