Log Message
DFG prediction propagation should agree with fixup phase over the return type of GetByVal https://bugs.webkit.org/show_bug.cgi?id=133134
Reviewed by Mark Hahnenberg.
Make prediction propagator use ArrayMode refinement to decide the return type.
Also introduce a heap prediction intrinsic that allows us to test weird corner cases
like this. The only way we'll see a mismatch like this in the real world is probably
through a gnarly race condition.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleIntrinsic):
* dfg/DFGNode.h:
(JSC::DFG::Node::setHeapPrediction):
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* jsc.cpp:
(GlobalObject::finishCreation):
(functionFalse1):
(functionFalse2):
(functionUndefined1):
(functionUndefined2):
(functionFalse): Deleted.
(functionOtherFalse): Deleted.
(functionUndefined): Deleted.
* runtime/Intrinsic.h:
* tests/stress/get-by-val-double-predicted-int.js: Added.
(foo):
Modified Paths
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp
- trunk/Source/_javascript_Core/dfg/DFGNode.h
- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp
- trunk/Source/_javascript_Core/jsc.cpp
- trunk/Source/_javascript_Core/runtime/Intrinsic.h
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (169144 => 169145)
--- trunk/Source/_javascript_Core/ChangeLog 2014-05-21 01:55:59 UTC (rev 169144)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-05-21 03:49:16 UTC (rev 169145)
@@ -1,3 +1,35 @@
+2014-05-20 Filip Pizlo <[email protected]>
+
+ DFG prediction propagation should agree with fixup phase over the return type of GetByVal
+ https://bugs.webkit.org/show_bug.cgi?id=133134
+
+ Reviewed by Mark Hahnenberg.
+
+ Make prediction propagator use ArrayMode refinement to decide the return type.
+
+ Also introduce a heap prediction intrinsic that allows us to test weird corner cases
+ like this. The only way we'll see a mismatch like this in the real world is probably
+ through a gnarly race condition.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleIntrinsic):
+ * dfg/DFGNode.h:
+ (JSC::DFG::Node::setHeapPrediction):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ (JSC::DFG::PredictionPropagationPhase::propagate):
+ * jsc.cpp:
+ (GlobalObject::finishCreation):
+ (functionFalse1):
+ (functionFalse2):
+ (functionUndefined1):
+ (functionUndefined2):
+ (functionFalse): Deleted.
+ (functionOtherFalse): Deleted.
+ (functionUndefined): Deleted.
+ * runtime/Intrinsic.h:
+ * tests/stress/get-by-val-double-predicted-int.js: Added.
+ (foo):
+
2014-05-20 Mark Hahnenberg <[email protected]>
Watchdog timer should be lazily allocated
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (169144 => 169145)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2014-05-21 01:55:59 UTC (rev 169144)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2014-05-21 03:49:16 UTC (rev 169145)
@@ -1737,6 +1737,16 @@
return true;
}
+ case SetInt32HeapPredictionIntrinsic: {
+ for (int i = 1; i < argumentCountIncludingThis; ++i) {
+ Node* node = get(virtualRegisterForArgument(i, registerOffset));
+ if (node->hasHeapPrediction())
+ node->setHeapPrediction(SpecInt32);
+ }
+ set(VirtualRegister(resultOperand), constantUndefined());
+ return true;
+ }
+
default:
return false;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGNode.h (169144 => 169145)
--- trunk/Source/_javascript_Core/dfg/DFGNode.h 2014-05-21 01:55:59 UTC (rev 169144)
+++ trunk/Source/_javascript_Core/dfg/DFGNode.h 2014-05-21 03:49:16 UTC (rev 169145)
@@ -1031,6 +1031,12 @@
return mergeSpeculation(m_opInfo2, prediction);
}
+ void setHeapPrediction(SpeculatedType prediction)
+ {
+ ASSERT(hasHeapPrediction());
+ m_opInfo2 = prediction;
+ }
+
bool hasFunction()
{
switch (op()) {
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (169144 => 169145)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2014-05-21 01:55:59 UTC (rev 169144)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2014-05-21 03:49:16 UTC (rev 169145)
@@ -369,19 +369,36 @@
case GetByVal: {
if (!node->child1()->prediction())
break;
- if (!node->getHeapPrediction())
+
+ ArrayMode arrayMode = node->arrayMode().refine(
+ m_graph, node,
+ node->child1()->prediction(),
+ node->child2()->prediction(),
+ SpecNone, node->flags());
+
+ switch (arrayMode.type()) {
+ case Array::Double:
+ if (arrayMode.isOutOfBounds())
+ changed |= mergePrediction(node->getHeapPrediction() | SpecDoubleReal);
+ else
+ changed |= mergePrediction(SpecDoubleReal);
break;
-
- if (node->child1()->shouldSpeculateFloat32Array()
- || node->child1()->shouldSpeculateFloat64Array())
+ case Array::Float32Array:
+ case Array::Float64Array:
changed |= mergePrediction(SpecFullDouble);
- else if (node->child1()->shouldSpeculateUint32Array()) {
+ break;
+ case Array::Uint32Array:
if (isInt32Speculation(node->getHeapPrediction()))
changed |= mergePrediction(SpecInt32);
+ else if (enableInt52())
+ changed |= mergePrediction(SpecMachineInt);
else
- changed |= mergePrediction(SpecInt52);
- } else
+ changed |= mergePrediction(SpecInt32 | SpecInt52AsDouble);
+ break;
+ default:
changed |= mergePrediction(node->getHeapPrediction());
+ break;
+ }
break;
}
Modified: trunk/Source/_javascript_Core/jsc.cpp (169144 => 169145)
--- trunk/Source/_javascript_Core/jsc.cpp 2014-05-21 01:55:59 UTC (rev 169144)
+++ trunk/Source/_javascript_Core/jsc.cpp 2014-05-21 03:49:16 UTC (rev 169145)
@@ -333,9 +333,10 @@
static EncodedJSValue JSC_HOST_CALL functionReoptimizationRetryCount(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionTransferArrayBuffer(ExecState*);
static NO_RETURN_WITH_VALUE EncodedJSValue JSC_HOST_CALL functionQuit(ExecState*);
-static EncodedJSValue JSC_HOST_CALL functionFalse(ExecState*);
-static EncodedJSValue JSC_HOST_CALL functionOtherFalse(ExecState*); // Need a separate function to break hash-consing of native executables.
-static EncodedJSValue JSC_HOST_CALL functionUndefined(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionFalse1(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionFalse2(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionUndefined1(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionUndefined2(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionEffectful42(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionMakeMasquerader(ExecState*);
@@ -473,9 +474,10 @@
addFunction(vm, "getElement", functionGetElement, 1);
addFunction(vm, "setElementRoot", functionSetElementRoot, 2);
- putDirectNativeFunction(vm, this, Identifier(&vm, "DFGTrue"), 0, functionFalse, DFGTrueIntrinsic, DontEnum | JSC::Function);
- putDirectNativeFunction(vm, this, Identifier(&vm, "OSRExit"), 0, functionUndefined, OSRExitIntrinsic, DontEnum | JSC::Function);
- putDirectNativeFunction(vm, this, Identifier(&vm, "isFinalTier"), 0, functionOtherFalse, IsFinalTierIntrinsic, DontEnum | JSC::Function);
+ putDirectNativeFunction(vm, this, Identifier(&vm, "DFGTrue"), 0, functionFalse1, DFGTrueIntrinsic, DontEnum | JSC::Function);
+ putDirectNativeFunction(vm, this, Identifier(&vm, "OSRExit"), 0, functionUndefined1, OSRExitIntrinsic, DontEnum | JSC::Function);
+ putDirectNativeFunction(vm, this, Identifier(&vm, "isFinalTier"), 0, functionFalse2, IsFinalTierIntrinsic, DontEnum | JSC::Function);
+ putDirectNativeFunction(vm, this, Identifier(&vm, "predictInt32"), 0, functionUndefined2, SetInt32HeapPredictionIntrinsic, DontEnum | JSC::Function);
addFunction(vm, "effectful42", functionEffectful42, 0);
addFunction(vm, "makeMasquerader", functionMakeMasquerader, 0);
@@ -893,21 +895,12 @@
#endif
}
-EncodedJSValue JSC_HOST_CALL functionFalse(ExecState*)
-{
- return JSValue::encode(jsBoolean(false));
-}
+EncodedJSValue JSC_HOST_CALL functionFalse1(ExecState*) { return JSValue::encode(jsBoolean(false)); }
+EncodedJSValue JSC_HOST_CALL functionFalse2(ExecState*) { return JSValue::encode(jsBoolean(false)); }
-EncodedJSValue JSC_HOST_CALL functionOtherFalse(ExecState*)
-{
- return JSValue::encode(jsBoolean(false));
-}
+EncodedJSValue JSC_HOST_CALL functionUndefined1(ExecState*) { return JSValue::encode(jsUndefined()); }
+EncodedJSValue JSC_HOST_CALL functionUndefined2(ExecState*) { return JSValue::encode(jsUndefined()); }
-EncodedJSValue JSC_HOST_CALL functionUndefined(ExecState*)
-{
- return JSValue::encode(jsUndefined());
-}
-
EncodedJSValue JSC_HOST_CALL functionEffectful42(ExecState*)
{
return JSValue::encode(jsNumber(42));
Modified: trunk/Source/_javascript_Core/runtime/Intrinsic.h (169144 => 169145)
--- trunk/Source/_javascript_Core/runtime/Intrinsic.h 2014-05-21 01:55:59 UTC (rev 169144)
+++ trunk/Source/_javascript_Core/runtime/Intrinsic.h 2014-05-21 03:49:16 UTC (rev 169145)
@@ -59,7 +59,8 @@
// Debugging intrinsics
DFGTrueIntrinsic,
OSRExitIntrinsic,
- IsFinalTierIntrinsic
+ IsFinalTierIntrinsic,
+ SetInt32HeapPredictionIntrinsic
};
} // namespace JSC
Added: trunk/Source/_javascript_Core/tests/stress/get-by-val-double-predicted-int.js (0 => 169145)
--- trunk/Source/_javascript_Core/tests/stress/get-by-val-double-predicted-int.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/get-by-val-double-predicted-int.js 2014-05-21 03:49:16 UTC (rev 169145)
@@ -0,0 +1,16 @@
+function foo(a, i) {
+ var x = a[i];
+ predictInt32(x);
+ return x + 2000000000;
+}
+
+noInline(foo);
+
+var array = [2000000000.5];
+
+for (var i = 0; i < 1000000; ++i) {
+ var result = foo(array, 0);
+ if (result != 4000000000.5)
+ throw "Error: bad result: " + result;
+}
+
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
