From 09961e4b798b98e1e35688ce692094186a8f5d07 Mon Sep 17 00:00:00 2001 From: Filip Pizlo Date: Wed, 27 Mar 2013 10:34:00 +0100 Subject: DFG is too aggressive with eliding overflow checks in loops https://bugs.webkit.org/show_bug.cgi?id=105226 Reviewed by Mark Hahnenberg and Oliver Hunt. Source/JavaScriptCore: If we see a variable's live range cross basic block boundaries, conservatively assume that it may be part of a data-flow back-edge, and as a result, we may have entirely integer operations that could lead to the creation of an integer that is out of range of 2^52 (the significand of a double float). This does not seem to regress any of the benchmarks we care about, and it fixes the bug. In future we may want to actually look at whether or not there was a data-flow back-edge instead of being super conservative about it. But we have no evidence, yet, that this would help us on real code. * dfg/DFGNodeFlags.h: (DFG): * dfg/DFGPredictionPropagationPhase.cpp: (JSC::DFG::PredictionPropagationPhase::propagate): LayoutTests: * fast/js/dfg-int-overflow-in-loop-expected.txt: Added. * fast/js/dfg-int-overflow-in-loop.html: Added. * fast/js/jsc-test-list: * fast/js/script-tests/dfg-int-overflow-in-loop.js: Added. (foo): Change-Id: I9df2d6d17ba404802456f4e2da313e47f0f4f62e git-svn-id: http://svn.webkit.org/repository/webkit/trunk@137963 268f45cc-cd09-0410-ab3c-d52691b4dbfc Reviewed-by: Jocelyn Turcotte --- Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp') diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp index 5b6c28ff7..4226fcc6a 100644 --- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp @@ -211,7 +211,13 @@ private: case SetLocal: { VariableAccessData* variableAccessData = node.variableAccessData(); changed |= variableAccessData->predict(m_graph[node.child1()].prediction()); - changed |= m_graph[node.child1()].mergeFlags(variableAccessData->flags()); + + // Assume conservatively that a SetLocal implies that the value may flow through a loop, + // and so we would have overflow leading to the program "observing" numbers even if all + // users of the value are doing toInt32. It might be worthwhile to revisit this at some + // point and actually check if the data flow involves loops, but right now I don't think + // we have evidence that this would be beneficial for benchmarks. + changed |= m_graph[node.child1()].mergeFlags(variableAccessData->flags() | NodeUsedAsNumber); break; } -- cgit v1.2.1