Log Message
DFG constant folding should be OK with GetLocal of captured variables having a constant https://bugs.webkit.org/show_bug.cgi?id=88137
Reviewed by Gavin Barraclough. Source/_javascript_Core: * dfg/DFGConstantFoldingPhase.cpp: (JSC::DFG::ConstantFoldingPhase::run): LayoutTests: * fast/js/dfg-cse-cfa-discrepancy-expected.txt: Added. * fast/js/dfg-cse-cfa-discrepancy.html: Added. * fast/js/script-tests/dfg-cse-cfa-discrepancy.js: Added. (foo.bar.baz): (foo.bar): (foo):
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (119280 => 119281)
--- trunk/LayoutTests/ChangeLog 2012-06-01 21:26:11 UTC (rev 119280)
+++ trunk/LayoutTests/ChangeLog 2012-06-01 21:32:45 UTC (rev 119281)
@@ -1,3 +1,17 @@
+2012-06-01 Filip Pizlo <[email protected]>
+
+ DFG constant folding should be OK with GetLocal of captured variables having a constant
+ https://bugs.webkit.org/show_bug.cgi?id=88137
+
+ Reviewed by Gavin Barraclough.
+
+ * fast/js/dfg-cse-cfa-discrepancy-expected.txt: Added.
+ * fast/js/dfg-cse-cfa-discrepancy.html: Added.
+ * fast/js/script-tests/dfg-cse-cfa-discrepancy.js: Added.
+ (foo.bar.baz):
+ (foo.bar):
+ (foo):
+
2012-06-01 Jessie Berlin <[email protected]>
[Win] ASSERT(enclosingIntRect(rendererMappedResult) == enclosingIntRect(FloatQuad(result).boundingBox()))
Added: trunk/LayoutTests/fast/js/dfg-cse-cfa-discrepancy-expected.txt (0 => 119281)
--- trunk/LayoutTests/fast/js/dfg-cse-cfa-discrepancy-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-cse-cfa-discrepancy-expected.txt 2012-06-01 21:32:45 UTC (rev 119281)
@@ -0,0 +1,209 @@
+Tests stability when the CFA finds a constant in block-local data flow over captured variables but the CSE doesn't because if differences in heap modeling.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS foo(i, i + 1) is 162
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/dfg-cse-cfa-discrepancy.html (0 => 119281)
--- trunk/LayoutTests/fast/js/dfg-cse-cfa-discrepancy.html (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-cse-cfa-discrepancy.html 2012-06-01 21:32:45 UTC (rev 119281)
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<script src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/js/script-tests/dfg-cse-cfa-discrepancy.js (0 => 119281)
--- trunk/LayoutTests/fast/js/script-tests/dfg-cse-cfa-discrepancy.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-cse-cfa-discrepancy.js 2012-06-01 21:32:45 UTC (rev 119281)
@@ -0,0 +1,31 @@
+description(
+"Tests stability when the CFA finds a constant in block-local data flow over captured variables but the CSE doesn't because if differences in heap modeling."
+);
+
+function foo(a, b) {
+ var x = a + b;
+ var y = a - b;
+ function bar() {
+ var m = x + y;
+ var n = x - y;
+ function baz() {
+ return m + n;
+ }
+ m = 23;
+ if (m < n) {
+ m = 12;
+ y = 72; // PutScopedVar appears to clobber world in CSE but only clobbers structures in CFA.
+ return m + 6 + n + baz();
+ } else
+ return 91;
+ }
+ x = 42;
+ if (y < 0) {
+ return x + 5 + y + bar();
+ } else
+ return 73;
+}
+
+for (var i = 0; i < 200; ++i)
+ shouldBe("foo(i, i + 1)", "162");
+
Modified: trunk/Source/_javascript_Core/ChangeLog (119280 => 119281)
--- trunk/Source/_javascript_Core/ChangeLog 2012-06-01 21:26:11 UTC (rev 119280)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-06-01 21:32:45 UTC (rev 119281)
@@ -1,3 +1,13 @@
+2012-06-01 Filip Pizlo <[email protected]>
+
+ DFG constant folding should be OK with GetLocal of captured variables having a constant
+ https://bugs.webkit.org/show_bug.cgi?id=88137
+
+ Reviewed by Gavin Barraclough.
+
+ * dfg/DFGConstantFoldingPhase.cpp:
+ (JSC::DFG::ConstantFoldingPhase::run):
+
2012-05-31 Mark Hahnenberg <[email protected]>
JSGlobalObject does not mark m_privateNameStructure
Modified: trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp (119280 => 119281)
--- trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp 2012-06-01 21:26:11 UTC (rev 119280)
+++ trunk/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp 2012-06-01 21:32:45 UTC (rev 119281)
@@ -74,11 +74,9 @@
Node phantom(Phantom, node.codeOrigin);
if (node.op() == GetLocal) {
- ASSERT(m_graph[node.child1()].op() == Phi);
- ASSERT(!m_graph[node.child1()].hasResult());
-
NodeIndex previousLocalAccess = NoNode;
- if (block->variablesAtHead.operand(node.local()) == nodeIndex) {
+ if (block->variablesAtHead.operand(node.local()) == nodeIndex
+ && m_graph[node.child1()].op() == Phi) {
// We expect this to be the common case.
ASSERT(block->isInPhis(node.child1().index()));
previousLocalAccess = node.child1().index();
@@ -97,13 +95,6 @@
continue;
// The two must have been unified.
ASSERT(subNode.variableAccessData() == node.variableAccessData());
- // Currently, the previous node must be a flush.
- // NOTE: This assertion should be removed if we ever do
- // constant folding on captured variables. In particular,
- // this code does not require the previous node to be a flush,
- // but we are asserting this anyway because it is a constraint
- // of the IR and this is as good a place as any to assert it.
- ASSERT(subNode.op() == Flush);
previousLocalAccess = subNodeIndex;
break;
}
_______________________________________________ webkit-changes mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
