- Revision
- 98188
- Author
- fpi...@apple.com
- Date
- 2011-10-21 20:58:55 -0700 (Fri, 21 Oct 2011)
Log Message
DFG inlining sometimes fails to reset constant references
https://bugs.webkit.org/show_bug.cgi?id=70668
Source/_javascript_Core:
Reviewed by Anders Carlsson.
Reset constant references when we need to (new block created) and not
when we don't (change of inlining depth).
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleInlining):
(JSC::DFG::ByteCodeParser::prepareToParseBlock):
(JSC::DFG::ByteCodeParser::parseBlock):
(JSC::DFG::ByteCodeParser::parseCodeBlock):
LayoutTests:
Reviewed by Anders Carlsson.
Added a new test that covers this specific bug as well as the general
ability to perform inlining, and the ability to OSR out of an inline
stack.
* fast/js/dfg-inline-constant-expected.txt: Added.
* fast/js/dfg-inline-constant.html: Added.
* fast/js/script-tests/dfg-inline-constant.js: Added.
(foo):
(bar):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (98187 => 98188)
--- trunk/LayoutTests/ChangeLog 2011-10-22 03:56:01 UTC (rev 98187)
+++ trunk/LayoutTests/ChangeLog 2011-10-22 03:58:55 UTC (rev 98188)
@@ -1,3 +1,20 @@
+2011-10-21 Filip Pizlo <fpi...@apple.com>
+
+ DFG inlining sometimes fails to reset constant references
+ https://bugs.webkit.org/show_bug.cgi?id=70668
+
+ Reviewed by Anders Carlsson.
+
+ Added a new test that covers this specific bug as well as the general
+ ability to perform inlining, and the ability to OSR out of an inline
+ stack.
+
+ * fast/js/dfg-inline-constant-expected.txt: Added.
+ * fast/js/dfg-inline-constant.html: Added.
+ * fast/js/script-tests/dfg-inline-constant.js: Added.
+ (foo):
+ (bar):
+
2011-10-21 Ojan Vafai <o...@chromium.org>
Fixup test expectations now that http://trac.webkit.org/changeset/98173 has landed.
Added: trunk/LayoutTests/fast/js/dfg-inline-constant-expected.txt (0 => 98188)
--- trunk/LayoutTests/fast/js/dfg-inline-constant-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-inline-constant-expected.txt 2011-10-22 03:58:55 UTC (rev 98188)
@@ -0,0 +1,13 @@
+This tests that function inlining in the DFG JIT doesn't get confused by constants being reused between inliner and inlinee.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS bar(6, 0) is 10
+PASS bar(6, 1) is 15
+PASS bar(6, false) is 10
+PASS bar(6, true) is 15
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/js/dfg-inline-constant.html (0 => 98188)
--- trunk/LayoutTests/fast/js/dfg-inline-constant.html (rev 0)
+++ trunk/LayoutTests/fast/js/dfg-inline-constant.html 2011-10-22 03:58:55 UTC (rev 98188)
@@ -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-inline-constant.js (0 => 98188)
--- trunk/LayoutTests/fast/js/script-tests/dfg-inline-constant.js (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/dfg-inline-constant.js 2011-10-22 03:58:55 UTC (rev 98188)
@@ -0,0 +1,23 @@
+description(
+"This tests that function inlining in the DFG JIT doesn't get confused by constants being reused between inliner and inlinee."
+);
+
+function foo(a, b) {
+ if (b)
+ return a + 4;
+ return b + 5;
+}
+
+function bar(a, b) {
+ return foo(a, b) + 5;
+}
+
+for (var i = 0; i < 1000; ++i)
+ bar(i, i + 1);
+
+shouldBe("bar(6, 0)", "10");
+shouldBe("bar(6, 1)", "15");
+shouldBe("bar(6, false)", "10");
+shouldBe("bar(6, true)", "15");
+
+var successfullyParsed = true;
Modified: trunk/Source/_javascript_Core/ChangeLog (98187 => 98188)
--- trunk/Source/_javascript_Core/ChangeLog 2011-10-22 03:56:01 UTC (rev 98187)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-10-22 03:58:55 UTC (rev 98188)
@@ -1,5 +1,21 @@
2011-10-21 Filip Pizlo <fpi...@apple.com>
+ DFG inlining sometimes fails to reset constant references
+ https://bugs.webkit.org/show_bug.cgi?id=70668
+
+ Reviewed by Anders Carlsson.
+
+ Reset constant references when we need to (new block created) and not
+ when we don't (change of inlining depth).
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleInlining):
+ (JSC::DFG::ByteCodeParser::prepareToParseBlock):
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ (JSC::DFG::ByteCodeParser::parseCodeBlock):
+
+2011-10-21 Filip Pizlo <fpi...@apple.com>
+
DFG should have inlining
https://bugs.webkit.org/show_bug.cgi?id=69996
Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (98187 => 98188)
--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2011-10-22 03:56:01 UTC (rev 98187)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2011-10-22 03:58:55 UTC (rev 98188)
@@ -82,6 +82,8 @@
bool handleInlining(bool usesResult, int callTarget, NodeIndex callTargetNodeIndex, int resultOperand, bool certainAboutExpectedFunction, JSFunction*, int firstArg, int lastArg, unsigned nextOffset);
// Handle intrinsic functions. Return true if it succeeded, false if we need to plant a call.
bool handleIntrinsic(bool usesResult, int resultOperand, Intrinsic, int firstArg, int lastArg, PredictedType prediction);
+ // Prepare to parse a block.
+ void prepareToParseBlock();
// Parse a single basic block of bytecode instructions.
bool parseBlock(unsigned limit);
// Find reachable code and setup predecessor links in the graph's BasicBlocks.
@@ -1027,6 +1029,7 @@
m_inlineStackTop->m_caller->m_unlinkedBlocks.append(UnlinkedBlock(m_graph.m_blocks.size()));
m_inlineStackTop->m_caller->m_blockLinkingTargets.append(m_graph.m_blocks.size());
m_graph.m_blocks.append(block.release());
+ prepareToParseBlock();
// At this point we return and continue to generate code for the caller, but
// in the new basic block.
@@ -1122,14 +1125,14 @@
}
}
+void ByteCodeParser::prepareToParseBlock()
+{
+ for (unsigned i = 0; i < m_constants.size(); ++i)
+ m_constants[i] = ConstantRecord();
+}
+
bool ByteCodeParser::parseBlock(unsigned limit)
{
- // No need to reset state initially, since it has been set by the constructor.
- if (m_currentIndex) {
- for (unsigned i = 0; i < m_constants.size(); ++i)
- m_constants[i] = ConstantRecord();
- }
-
// If we are the first basic block, introduce markers for arguments. This allows
// us to track if a use of an argument may use the actual argument passed, as
// opposed to using a value we set explicitly.
@@ -2379,6 +2382,7 @@
m_inlineStackTop->m_unlinkedBlocks.append(UnlinkedBlock(m_graph.m_blocks.size()));
m_inlineStackTop->m_blockLinkingTargets.append(m_graph.m_blocks.size());
m_graph.m_blocks.append(block.release());
+ prepareToParseBlock();
}
}
