Title: [164243] trunk/Source/_javascript_Core
Revision
164243
Author
[email protected]
Date
2014-02-17 12:23:34 -0800 (Mon, 17 Feb 2014)

Log Message

FTL should support ToPrimitive and the DFG should fold it correctly
https://bugs.webkit.org/show_bug.cgi?id=128892

Reviewed by Geoffrey Garen.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::compileNode):
(JSC::FTL::LowerDFGToLLVM::compileToPrimitive):
* tests/stress/fold-to-primitive-in-cfa.js: Added.
(foo):
(.result.foo):
* tests/stress/fold-to-primitive-to-identity-in-cfa.js: Added.
(foo):
(.result.foo):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (164242 => 164243)


--- trunk/Source/_javascript_Core/ChangeLog	2014-02-17 20:19:42 UTC (rev 164242)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-02-17 20:23:34 UTC (rev 164243)
@@ -1,5 +1,30 @@
 2014-02-17  Filip Pizlo  <[email protected]>
 
+        FTL should support ToPrimitive and the DFG should fold it correctly
+        https://bugs.webkit.org/show_bug.cgi?id=128892
+
+        Reviewed by Geoffrey Garen.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::foldConstants):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::compileNode):
+        (JSC::FTL::LowerDFGToLLVM::compileToPrimitive):
+        * tests/stress/fold-to-primitive-in-cfa.js: Added.
+        (foo):
+        (.result.foo):
+        * tests/stress/fold-to-primitive-to-identity-in-cfa.js: Added.
+        (foo):
+        (.result.foo):
+
+2014-02-17  Filip Pizlo  <[email protected]>
+
         Register preservation wrapper should know about the possibility of callee-saved FPRs
         https://bugs.webkit.org/show_bug.cgi?id=128923
 

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (164242 => 164243)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2014-02-17 20:19:42 UTC (rev 164242)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2014-02-17 20:23:34 UTC (rev 164243)
@@ -1176,39 +1176,20 @@
         
         ASSERT(node->child1().useKind() == UntypedUse);
         
-        AbstractValue& source = forNode(node->child1());
-        AbstractValue& destination = forNode(node);
+        if (!forNode(node->child1()).m_type) {
+            m_state.setIsValid(false);
+            break;
+        }
         
-        // NB. The more canonical way of writing this would have been:
-        //
-        // destination = source;
-        // if (destination.m_type & !(SpecFullNumber | SpecString | SpecBoolean)) {
-        //     destination.filter(SpecFullNumber | SpecString | SpecBoolean);
-        //     AbstractValue string;
-        //     string.set(vm->stringStructure);
-        //     destination.merge(string);
-        // }
-        //
-        // The reason why this would, in most other cases, have been better is that
-        // then destination would preserve any non-SpeculatedType knowledge of source.
-        // As it stands, the code below forgets any non-SpeculatedType knowledge that
-        // source would have had. Fortunately, though, for things like strings and
-        // numbers and booleans, we don't care about the non-SpeculatedType knowedge:
-        // the structure won't tell us anything we don't already know, and neither
-        // will ArrayModes. And if the source was a meaningful constant then we
-        // would have handled that above. Unfortunately, this does mean that
-        // ToPrimitive will currently forget string constants. But that's not a big
-        // deal since we don't do any optimization on those currently.
+        if (!(forNode(node->child1()).m_type & ~(SpecFullNumber | SpecBoolean | SpecString))) {
+            m_state.setFoundConstants(true);
+            forNode(node) = forNode(node->child1());
+            break;
+        }
         
         clobberWorld(node->origin.semantic, clobberLimit);
         
-        SpeculatedType type = source.m_type;
-        if (type & ~(SpecFullNumber | SpecString | SpecBoolean))
-            type = (SpecHeapTop & ~SpecCell) | SpecString;
-
-        destination.setType(type);
-        if (destination.isClear())
-            m_state.setIsValid(false);
+        forNode(node).setType((SpecHeapTop & ~SpecCell) | SpecString);
         break;
     }
         

Modified: trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp (164242 => 164243)


--- trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp	2014-02-17 20:19:42 UTC (rev 164242)
+++ trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp	2014-02-17 20:23:34 UTC (rev 164243)
@@ -326,8 +326,11 @@
                 break;
             }
 
-            case StoreBarrier:
-            case StoreBarrierWithNullCheck: {
+            case ToPrimitive: {
+                if (m_state.forNode(node->child1()).m_type & ~(SpecFullNumber | SpecBoolean | SpecString))
+                    break;
+                
+                node->convertToIdentity();
                 break;
             }
 

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (164242 => 164243)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2014-02-17 20:19:42 UTC (rev 164242)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2014-02-17 20:23:34 UTC (rev 164243)
@@ -3416,19 +3416,15 @@
         
         op1.use();
         
-        if (!(m_state.forNode(node->child1()).m_type & ~(SpecFullNumber | SpecBoolean)))
-            m_jit.move(op1GPR, resultGPR);
-        else {
-            MacroAssembler::Jump alreadyPrimitive = m_jit.branchTest64(MacroAssembler::NonZero, op1GPR, GPRInfo::tagMaskRegister);
-            MacroAssembler::Jump notPrimitive = m_jit.branchPtr(MacroAssembler::NotEqual, MacroAssembler::Address(op1GPR, JSCell::structureOffset()), MacroAssembler::TrustedImmPtr(m_jit.vm()->stringStructure.get()));
-            
-            alreadyPrimitive.link(&m_jit);
-            m_jit.move(op1GPR, resultGPR);
-            
-            addSlowPathGenerator(
-                slowPathCall(notPrimitive, this, operationToPrimitive, resultGPR, op1GPR));
-        }
+        MacroAssembler::Jump alreadyPrimitive = m_jit.branchTest64(MacroAssembler::NonZero, op1GPR, GPRInfo::tagMaskRegister);
+        MacroAssembler::Jump notPrimitive = m_jit.branchPtr(MacroAssembler::NotEqual, MacroAssembler::Address(op1GPR, JSCell::structureOffset()), MacroAssembler::TrustedImmPtr(m_jit.vm()->stringStructure.get()));
         
+        alreadyPrimitive.link(&m_jit);
+        m_jit.move(op1GPR, resultGPR);
+        
+        addSlowPathGenerator(
+            slowPathCall(notPrimitive, this, operationToPrimitive, resultGPR, op1GPR));
+        
         jsValueResult(resultGPR, node, UseChildrenCalledExplicitly);
         break;
     }

Modified: trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp (164242 => 164243)


--- trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2014-02-17 20:19:42 UTC (rev 164242)
+++ trunk/Source/_javascript_Core/ftl/FTLCapabilities.cpp	2014-02-17 20:23:34 UTC (rev 164243)
@@ -138,6 +138,7 @@
     case GetById:
     case ToThis:
     case MultiGetByOffset:
+    case ToPrimitive:
         // These are OK.
         break;
     case PutByIdDirect:

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (164242 => 164243)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp	2014-02-17 20:19:42 UTC (rev 164242)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp	2014-02-17 20:23:34 UTC (rev 164243)
@@ -452,6 +452,9 @@
         case ToString:
             compileToString();
             break;
+        case ToPrimitive:
+            compileToPrimitive();
+            break;
         case MakeRope:
             compileMakeRope();
             break;
@@ -2854,6 +2857,32 @@
         }
     }
     
+    void compileToPrimitive()
+    {
+        LValue value = lowJSValue(m_node->child1());
+        
+        LBasicBlock isCellCase = FTL_NEW_BLOCK(m_out, ("ToPrimitive cell case"));
+        LBasicBlock isObjectCase = FTL_NEW_BLOCK(m_out, ("ToPrimitive object case"));
+        LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("ToPrimitive continuation"));
+        
+        Vector<ValueFromBlock, 3> results;
+        
+        results.append(m_out.anchor(value));
+        m_out.branch(isCell(value), isCellCase, continuation);
+        
+        LBasicBlock lastNext = m_out.appendTo(isCellCase, isObjectCase);
+        results.append(m_out.anchor(value));
+        m_out.branch(isObject(value), isObjectCase, continuation);
+        
+        m_out.appendTo(isObjectCase, continuation);
+        results.append(m_out.anchor(vmCall(
+            m_out.operation(operationToPrimitive), m_callFrame, value)));
+        m_out.jump(continuation);
+        
+        m_out.appendTo(continuation, lastNext);
+        setJSValue(m_out.phi(m_out.int64, results));
+    }
+    
     void compileMakeRope()
     {
         LValue kids[3];

Added: trunk/Source/_javascript_Core/tests/stress/fold-to-primitive-in-cfa.js (0 => 164243)


--- trunk/Source/_javascript_Core/tests/stress/fold-to-primitive-in-cfa.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/fold-to-primitive-in-cfa.js	2014-02-17 20:23:34 UTC (rev 164243)
@@ -0,0 +1,14 @@
+function foo(x) {
+    if (DFGTrue())
+        x = "hello";
+    return x + " world";
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+    var result = foo({toString:function() { return "foo" }});
+    if (result != "foo world" && result != "hello world")
+        throw "Error: bad result: " + result;
+}
+

Added: trunk/Source/_javascript_Core/tests/stress/fold-to-primitive-to-identity-in-cfa.js (0 => 164243)


--- trunk/Source/_javascript_Core/tests/stress/fold-to-primitive-to-identity-in-cfa.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/fold-to-primitive-to-identity-in-cfa.js	2014-02-17 20:23:34 UTC (rev 164243)
@@ -0,0 +1,14 @@
+function foo(x, p) {
+    if (DFGTrue())
+        x = p ? "hello" : "bar";
+    return x + " world";
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+    var result = foo({toString:function() { return "foo" }}, i & 1);
+    if (result != "foo world" && result != "hello world" && result != "bar world")
+        throw "Error: bad result: " + result;
+}
+
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to