Title: [226208] trunk/Source/_javascript_Core
Revision
226208
Author
sbar...@apple.com
Date
2017-12-20 17:54:46 -0800 (Wed, 20 Dec 2017)

Log Message

GetPropertyEnumerator in DFG/FTL should not unconditionally speculate cell
https://bugs.webkit.org/show_bug.cgi?id=181054

Reviewed by Mark Lam.

Speedometer's react subtest has a function that is in an OSR exit loop because
we used to unconditionally speculate cell for the operand to GetPropertyEnumerator.
This fix doesn't seem to speed up Speedometer at all, but it's good hygiene
for our compiler to not have this pathology. This patch adds a generic
GetPropertyEnumerator to prevent the exit loop.

* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileGetPropertyEnumerator):
* jit/JITOperations.cpp:
* jit/JITOperations.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (226207 => 226208)


--- trunk/Source/_javascript_Core/ChangeLog	2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-12-21 01:54:46 UTC (rev 226208)
@@ -1,3 +1,27 @@
+2017-12-20  Saam Barati  <sbar...@apple.com>
+
+        GetPropertyEnumerator in DFG/FTL should not unconditionally speculate cell
+        https://bugs.webkit.org/show_bug.cgi?id=181054
+
+        Reviewed by Mark Lam.
+
+        Speedometer's react subtest has a function that is in an OSR exit loop because
+        we used to unconditionally speculate cell for the operand to GetPropertyEnumerator.
+        This fix doesn't seem to speed up Speedometer at all, but it's good hygiene 
+        for our compiler to not have this pathology. This patch adds a generic
+        GetPropertyEnumerator to prevent the exit loop.
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetPropertyEnumerator):
+        * jit/JITOperations.cpp:
+        * jit/JITOperations.h:
+
 2017-12-20  Daniel Bates  <daba...@apple.com>
 
         Remove Alternative Presentation Button

Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (226207 => 226208)


--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp	2017-12-21 01:54:46 UTC (rev 226208)
@@ -1725,7 +1725,8 @@
             break;
         }
         case GetPropertyEnumerator: {
-            fixEdge<CellUse>(node->child1());
+            if (node->child1()->shouldSpeculateCell())
+                fixEdge<CellUse>(node->child1());
             break;
         }
         case GetEnumeratorStructurePname: {

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (226207 => 226208)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2017-12-21 01:54:46 UTC (rev 226208)
@@ -5358,14 +5358,25 @@
         break;
     }
     case GetPropertyEnumerator: {
-        SpeculateCellOperand base(this, node->child1());
-        GPRFlushedCallResult result(this);
-        GPRReg resultGPR = result.gpr();
+        if (node->child1().useKind() == CellUse) {
+            SpeculateCellOperand base(this, node->child1());
+            GPRFlushedCallResult result(this);
+            GPRReg resultGPR = result.gpr();
 
-        flushRegisters();
-        callOperation(operationGetPropertyEnumerator, resultGPR, base.gpr());
-        m_jit.exceptionCheck();
-        cellResult(resultGPR, node);
+            flushRegisters();
+            callOperation(operationGetPropertyEnumeratorCell, resultGPR, base.gpr());
+            m_jit.exceptionCheck();
+            cellResult(resultGPR, node);
+        } else {
+            JSValueOperand base(this, node->child1());
+            GPRFlushedCallResult result(this);
+            GPRReg resultGPR = result.gpr();
+
+            flushRegisters();
+            callOperation(operationGetPropertyEnumerator, resultGPR, base.jsValueRegs());
+            m_jit.exceptionCheck();
+            cellResult(resultGPR, node);
+        }
         break;
     }
     case GetEnumeratorStructurePname:

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (226207 => 226208)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2017-12-21 01:54:46 UTC (rev 226208)
@@ -5802,12 +5802,19 @@
         break;
     }
     case GetPropertyEnumerator: {
-        SpeculateCellOperand base(this, node->child1());
+        JSValueOperand base(this, node->child1(), ManualOperandSpeculation);
         GPRFlushedCallResult result(this);
         GPRReg resultGPR = result.gpr();
+        GPRReg baseGPR = base.gpr();
 
+        if (node->child1().useKind() == CellUse)
+            speculateCell(node->child1());
+
         flushRegisters();
-        callOperation(operationGetPropertyEnumerator, resultGPR, base.gpr());
+        if (node->child1().useKind() == CellUse)
+            callOperation(operationGetPropertyEnumeratorCell, resultGPR, baseGPR);
+        else
+            callOperation(operationGetPropertyEnumerator, resultGPR, baseGPR);
         m_jit.exceptionCheck();
         cellResult(resultGPR, node);
         break;

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (226207 => 226208)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2017-12-21 01:54:46 UTC (rev 226208)
@@ -9733,8 +9733,10 @@
 
     void compileGetPropertyEnumerator()
     {
-        LValue base = lowCell(m_node->child1());
-        setJSValue(vmCall(Int64, m_out.operation(operationGetPropertyEnumerator), m_callFrame, base));
+        if (m_node->child1().useKind() == CellUse)
+            setJSValue(vmCall(Int64, m_out.operation(operationGetPropertyEnumeratorCell), m_callFrame, lowCell(m_node->child1())));
+        else
+            setJSValue(vmCall(Int64, m_out.operation(operationGetPropertyEnumerator), m_callFrame, lowJSValue(m_node->child1())));
     }
 
     void compileGetEnumeratorStructurePname()

Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (226207 => 226208)


--- trunk/Source/_javascript_Core/jit/JITOperations.cpp	2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp	2017-12-21 01:54:46 UTC (rev 226208)
@@ -2394,7 +2394,7 @@
     return JSValue::encode(jsBoolean(base->hasPropertyGeneric(exec, asString(propertyName)->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty)));
 }
 
-JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState* exec, JSCell* cell)
+JSCell* JIT_OPERATION operationGetPropertyEnumeratorCell(ExecState* exec, JSCell* cell)
 {
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
@@ -2407,6 +2407,23 @@
     return propertyNameEnumerator(exec, base);
 }
 
+JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState* exec, EncodedJSValue encodedBase)
+{
+    VM& vm = exec->vm();
+    NativeCallFrameTracer tracer(&vm, exec);
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    JSValue base = JSValue::decode(encodedBase);
+    if (base.isUndefinedOrNull())
+        return JSPropertyNameEnumerator::create(vm);
+
+    JSObject* baseObject = base.toObject(exec);
+    RETURN_IF_EXCEPTION(scope, { });
+
+    scope.release();
+    return propertyNameEnumerator(exec, baseObject);
+}
+
 EncodedJSValue JIT_OPERATION operationNextEnumeratorPname(ExecState* exec, JSCell* enumeratorCell, int32_t index)
 {
     VM& vm = exec->vm();

Modified: trunk/Source/_javascript_Core/jit/JITOperations.h (226207 => 226208)


--- trunk/Source/_javascript_Core/jit/JITOperations.h	2017-12-21 01:49:00 UTC (rev 226207)
+++ trunk/Source/_javascript_Core/jit/JITOperations.h	2017-12-21 01:54:46 UTC (rev 226208)
@@ -461,7 +461,8 @@
 int32_t JIT_OPERATION operationInstanceOfCustom(ExecState*, EncodedJSValue encodedValue, JSObject* constructor, EncodedJSValue encodedHasInstance) WTF_INTERNAL;
 
 EncodedJSValue JIT_OPERATION operationHasGenericProperty(ExecState*, EncodedJSValue, JSCell*);
-JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState*, JSCell*);
+JSCell* JIT_OPERATION operationGetPropertyEnumeratorCell(ExecState*, JSCell*);
+JSCell* JIT_OPERATION operationGetPropertyEnumerator(ExecState*, EncodedJSValue);
 EncodedJSValue JIT_OPERATION operationNextEnumeratorPname(ExecState*, JSCell*, int32_t);
 JSCell* JIT_OPERATION operationToIndexString(ExecState*, int32_t);
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to