Reviewers: Kevin Millikin,
http://codereview.chromium.org/6524039/diff/2001/src/runtime.cc
File src/runtime.cc (right):
http://codereview.chromium.org/6524039/diff/2001/src/runtime.cc#newcode7172
src/runtime.cc:7172: if (CompileOptimized(function, ast_id,
CLEAR_EXCEPTION) &&
The exception is thrown inside TryInline. Would it be reasonable to drop
the exception there instead, and just not inline that function call,
instead of failing to optimize the entire function?
http://codereview.chromium.org/6524039/diff/2001/src/runtime.cc#newcode7218
src/runtime.cc:7218: }
I'm a little wary at this.
If I don't unmark the function again, it's attempted recompiled again
and again and again. Unmarking it again helps a little, but it is still
attempted recompiled a few times after a while.
That might be reasonable if the problem was, e.g., a stack overflow, or
something else that's potentially transient, but for a syntax error,
it's certain that it will happen again.
Description:
Make exception be ignored when trying to optimize.
BUG=v8::1145
TEST=
Please review this at http://codereview.chromium.org/6524039/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge/build-ia32
Affected files:
M src/handles.h
M src/handles.cc
M src/runtime.cc
A test/mjsunit/regress/regress-1145.js
Index: src/handles.cc
diff --git a/src/handles.cc b/src/handles.cc
index
d625d644c776e744251d69bfc7f582b77751c1b2..ee1c2bece71d92b33f3502367c37e25fbb221784
100644
--- a/src/handles.cc
+++ b/src/handles.cc
@@ -871,10 +871,12 @@ bool CompileLazyInLoop(Handle<JSFunction> function,
}
-bool CompileOptimized(Handle<JSFunction> function, int osr_ast_id) {
+bool CompileOptimized(Handle<JSFunction> function,
+ int osr_ast_id,
+ ClearExceptionFlag flag) {
CompilationInfo info(function);
info.SetOptimizing(osr_ast_id);
- bool result = CompileLazyHelper(&info, KEEP_EXCEPTION);
+ bool result = CompileLazyHelper(&info, flag);
if (result) PROFILE(FunctionCreateEvent(*function));
return result;
}
Index: src/handles.h
diff --git a/src/handles.h b/src/handles.h
index
d95ca9117022d8d33c3bdfc5cc9dc32c8e7762e5..dfdc2626029dce238613f2042c119db3c6d77c44
100644
--- a/src/handles.h
+++ b/src/handles.h
@@ -354,7 +354,9 @@ bool CompileLazy(Handle<JSFunction> function,
ClearExceptionFlag flag);
bool CompileLazyInLoop(Handle<JSFunction> function, ClearExceptionFlag
flag);
-bool CompileOptimized(Handle<JSFunction> function, int osr_ast_id);
+bool CompileOptimized(Handle<JSFunction> function,
+ int osr_ast_id,
+ ClearExceptionFlag flag);
class NoHandleAllocation BASE_EMBEDDED {
public:
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
48ff69f5d273071e0615743e2b2760f2abb71e70..c6df893d4701b61db0aa3923a853bb2f72289842
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -7000,7 +7000,7 @@ static MaybeObject* Runtime_LazyRecompile(Arguments
args) {
function->ReplaceCode(function->shared()->code());
return function->code();
}
- if (CompileOptimized(function, AstNode::kNoNumber)) {
+ if (CompileOptimized(function, AstNode::kNoNumber, KEEP_EXCEPTION)) {
return function->code();
}
if (FLAG_trace_opt) {
@@ -7169,7 +7169,8 @@ static MaybeObject*
Runtime_CompileForOnStackReplacement(Arguments args) {
// Try to compile the optimized code. A true return value from
// CompileOptimized means that compilation succeeded, not necessarily
// that optimization succeeded.
- if (CompileOptimized(function, ast_id) && function->IsOptimized()) {
+ if (CompileOptimized(function, ast_id, CLEAR_EXCEPTION) &&
+ function->IsOptimized()) {
DeoptimizationInputData* data = DeoptimizationInputData::cast(
function->code()->deoptimization_data());
if (data->OsrPcOffset()->value() >= 0) {
@@ -7212,6 +7213,9 @@ static MaybeObject*
Runtime_CompileForOnStackReplacement(Arguments args) {
ASSERT(function->code()->kind() == Code::OPTIMIZED_FUNCTION);
return Smi::FromInt(ast_id);
} else {
+ if (function->IsMarkedForLazyRecompilation()) {
+ function->set_code(function->shared()->code());
+ }
return Smi::FromInt(-1);
}
}
Index: test/mjsunit/regress/regress-1145.js
diff --git a/test/mjsunit/regress/regress-1145.js
b/test/mjsunit/regress/regress-1145.js
new file mode 100644
index
0000000000000000000000000000000000000000..1ee8d53163f5db7ef8f64733f08412c9aaf03958
--- /dev/null
+++ b/test/mjsunit/regress/regress-1145.js
@@ -0,0 +1,54 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --opt-eagerly
+
+// See: http://code.google.com/p/v8/issues/detail?id=1145
+// Should not throw a syntax error exception (change this if we make
lazily.
+// compiled functions with syntax errors into early errors).
+// Should not hit an assertion in debug mode.
+
+// A lazily compiled function with a syntax error that is attempted inlined
+// would set a pending exception that is then ignored (until it triggers
+// an assert).
+// This file must be at least 1024 bytes long to trigger lazy compilation.
+
+function f() { return 1; }
+
+// Must be lazy. Must throw SyntaxError during compilation.
+function fail() { continue; }
+
+function opt_me() {
+ var x = 1;
+ // Do lots of function calls and hope to be optimized.
+ for (var i = 0; i < 1000000; i++) {
+ x = f();
+ }
+ if (x == 0) fail(); // Hope to be inlined during optimization.
+}
+
+opt_me();
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
