From 47157e8a56fcec0f88e2ba2deedb96436b04a958 Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Thu, 19 May 2011 13:15:57 +0000 Subject: [PATCH] When inlining fails, disable optimization of the proper function. Also, refactor disabling of optimization to make it easier to ensure that both SharedFunctionInfo and Code get disabled. R=whesse@chromium.org BUG= TEST= Review URL: http://codereview.chromium.org/7033020 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7963 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/compiler.cc | 53 +++++++++++++++++++++---------------------------- src/compiler.h | 4 ++++ src/hydrogen.cc | 26 ++++++++++++++++++++---- src/hydrogen.h | 19 +++++------------- src/objects.cc | 23 +++++++++++++++++++++ src/objects.h | 5 +++++ 6 files changed, 82 insertions(+), 48 deletions(-) diff --git a/src/compiler.cc b/src/compiler.cc index c57510d19d..51c2149181 100755 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -94,6 +94,7 @@ CompilationInfo::CompilationInfo(Handle closure) } +// Disable optimization for the rest of the compilation pipeline. void CompilationInfo::DisableOptimization() { bool is_optimizable_closure = FLAG_optimize_closures && @@ -105,6 +106,14 @@ void CompilationInfo::DisableOptimization() { } +void CompilationInfo::AbortOptimization() { + Handle code(shared_info()->code()); + SetCode(code); + Isolate* isolate = code->GetIsolate(); + isolate->compilation_cache()->MarkForLazyOptimizing(closure()); +} + + // Determine whether to use the full compiler for all code. If the flag // --always-full-compiler is specified this is the case. For the virtual frame // based compiler the full compiler is also used if a debugger is connected, as @@ -156,31 +165,6 @@ static void FinishOptimization(Handle function, int64_t start) { } -static void AbortAndDisable(CompilationInfo* info) { - // Disable optimization for the shared function info and mark the - // code as non-optimizable. The marker on the shared function info - // is there because we flush non-optimized code thereby loosing the - // non-optimizable information for the code. When the code is - // regenerated and set on the shared function info it is marked as - // non-optimizable if optimization is disabled for the shared - // function info. - Handle shared = info->shared_info(); - shared->set_optimization_disabled(true); - Handle code = Handle(shared->code()); - ASSERT(code->kind() == Code::FUNCTION); - code->set_optimizable(false); - info->SetCode(code); - Isolate* isolate = code->GetIsolate(); - isolate->compilation_cache()->MarkForLazyOptimizing(info->closure()); - if (FLAG_trace_opt) { - PrintF("[disabled optimization for: "); - info->closure()->PrintName(); - PrintF(" / %" V8PRIxPTR "]\n", - reinterpret_cast(*info->closure())); - } -} - - static bool MakeCrankshaftCode(CompilationInfo* info) { // Test if we can optimize this function when asked to. We can only // do this after the scopes are computed. @@ -214,7 +198,9 @@ static bool MakeCrankshaftCode(CompilationInfo* info) { const int kMaxOptCount = FLAG_deopt_every_n_times == 0 ? Compiler::kDefaultMaxOptCount : 1000; if (info->shared_info()->opt_count() > kMaxOptCount) { - AbortAndDisable(info); + info->AbortOptimization(); + Handle closure = info->closure(); + info->shared_info()->DisableOptimization(*closure); // True indicates the compilation pipeline is still going, not // necessarily that we optimized the code. return true; @@ -231,7 +217,9 @@ static bool MakeCrankshaftCode(CompilationInfo* info) { Scope* scope = info->scope(); if ((scope->num_parameters() + 1) > limit || scope->num_stack_slots() > limit) { - AbortAndDisable(info); + info->AbortOptimization(); + Handle closure = info->closure(); + info->shared_info()->DisableOptimization(*closure); // True indicates the compilation pipeline is still going, not // necessarily that we optimized the code. return true; @@ -304,9 +292,14 @@ static bool MakeCrankshaftCode(CompilationInfo* info) { } } - // Compilation with the Hydrogen compiler failed. Keep using the - // shared code but mark it as unoptimizable. - AbortAndDisable(info); + // Keep using the shared code. + info->AbortOptimization(); + if (!builder.inline_bailout()) { + // Mark the shared code as unoptimizable unless it was an inlined + // function that bailed out. + Handle closure = info->closure(); + info->shared_info()->DisableOptimization(*closure); + } // True indicates the compilation pipeline is still going, not necessarily // that we optimized the code. return true; diff --git a/src/compiler.h b/src/compiler.h index 1c02f68d2b..6a2d48f446 100644 --- a/src/compiler.h +++ b/src/compiler.h @@ -144,6 +144,10 @@ class CompilationInfo BASE_EMBEDDED { return V8::UseCrankshaft() && !closure_.is_null(); } + // Disable all optimization attempts of this info for the rest of the + // current compilation pipeline. + void AbortOptimization(); + private: Isolate* isolate_; diff --git a/src/hydrogen.cc b/src/hydrogen.cc index d003b62cea..32ff28b435 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -521,6 +521,22 @@ HConstant* HGraph::GetConstantFalse() { return GetConstant(&constant_false_, isolate()->heap()->false_value()); } +HGraphBuilder::HGraphBuilder(CompilationInfo* info, + TypeFeedbackOracle* oracle) + : function_state_(NULL), + initial_function_state_(this, info, oracle), + ast_context_(NULL), + break_scope_(NULL), + graph_(NULL), + current_block_(NULL), + inlined_count_(0), + zone_(info->isolate()->zone()), + inline_bailout_(false) { + // This is not initialized in the initializer list because the + // constructor for the initial state relies on function_state_ == NULL + // to know it's the initial state. + function_state_= &initial_function_state_; +} HBasicBlock* HGraphBuilder::CreateJoin(HBasicBlock* first, HBasicBlock* second, @@ -4040,8 +4056,9 @@ bool HGraphBuilder::TryInline(Call* expr) { // Precondition: call is monomorphic and we have found a target with the // appropriate arity. - Handle target = expr->target(); Handle caller = info()->closure(); + Handle target = expr->target(); + Handle target_shared(target->shared()); // Do a quick check on source code length to avoid parsing large // inlining candidates. @@ -4078,7 +4095,7 @@ bool HGraphBuilder::TryInline(Call* expr) { } // Don't inline recursive functions. - if (target->shared() == outer_info->closure()->shared()) { + if (*target_shared == outer_info->closure()->shared()) { TraceInline(target, caller, "target is recursive"); return false; } @@ -4098,7 +4115,7 @@ bool HGraphBuilder::TryInline(Call* expr) { if (target_info.isolate()->has_pending_exception()) { // Parse or scope error, never optimize this function. SetStackOverflow(); - target->shared()->set_optimization_disabled(true); + target_shared->DisableOptimization(*target); } TraceInline(target, caller, "parse failure"); return false; @@ -4127,7 +4144,6 @@ bool HGraphBuilder::TryInline(Call* expr) { // Don't inline functions that uses the arguments object or that // have a mismatching number of parameters. - Handle target_shared(target->shared()); int arity = expr->arguments()->length(); if (function->scope()->arguments() != NULL || arity != target_shared->formal_parameter_count()) { @@ -4185,6 +4201,8 @@ bool HGraphBuilder::TryInline(Call* expr) { // Bail out if the inline function did, as we cannot residualize a call // instead. TraceInline(target, caller, "inline graph construction failed"); + target_shared->DisableOptimization(*target); + inline_bailout_ = true; return true; } diff --git a/src/hydrogen.h b/src/hydrogen.h index 4b943c1be1..1827d87d51 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -660,20 +660,7 @@ class HGraphBuilder: public AstVisitor { BreakAndContinueScope* next_; }; - HGraphBuilder(CompilationInfo* info, TypeFeedbackOracle* oracle) - : function_state_(NULL), - initial_function_state_(this, info, oracle), - ast_context_(NULL), - break_scope_(NULL), - graph_(NULL), - current_block_(NULL), - inlined_count_(0), - zone_(info->isolate()->zone()) { - // This is not initialized in the initializer list because the - // constructor for the initial state relies on function_state_ == NULL - // to know it's the initial state. - function_state_= &initial_function_state_; - } + HGraphBuilder(CompilationInfo* info, TypeFeedbackOracle* oracle); HGraph* CreateGraph(); @@ -688,6 +675,8 @@ class HGraphBuilder: public AstVisitor { return current_block()->last_environment(); } + bool inline_bailout() { return inline_bailout_; } + // Adding instructions. HInstruction* AddInstruction(HInstruction* instr); void AddSimulate(int id); @@ -976,6 +965,8 @@ class HGraphBuilder: public AstVisitor { Zone* zone_; + bool inline_bailout_; + friend class FunctionState; // Pushes and pops the state stack. friend class AstContext; // Pushes and pops the AST context stack. diff --git a/src/objects.cc b/src/objects.cc index a0fcb7a014..d81e1980c6 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -6096,6 +6096,29 @@ void SharedFunctionInfo::EnableDeoptimizationSupport(Code* recompiled) { } +void SharedFunctionInfo::DisableOptimization(JSFunction* function) { + // Disable optimization for the shared function info and mark the + // code as non-optimizable. The marker on the shared function info + // is there because we flush non-optimized code thereby loosing the + // non-optimizable information for the code. When the code is + // regenerated and set on the shared function info it is marked as + // non-optimizable if optimization is disabled for the shared + // function info. + set_optimization_disabled(true); + // Code should be the lazy compilation stub or else unoptimized. If the + // latter, disable optimization for the code too. + ASSERT(code()->kind() == Code::FUNCTION || code()->kind() == Code::BUILTIN); + if (code()->kind() == Code::FUNCTION) { + code()->set_optimizable(false); + } + if (FLAG_trace_opt) { + PrintF("[disabled optimization for: "); + function->PrintName(); + PrintF(" / %" V8PRIxPTR "]\n", reinterpret_cast(function)); + } +} + + bool SharedFunctionInfo::VerifyBailoutId(int id) { // TODO(srdjan): debugging ARM crashes in hydrogen. OK to disable while // we are always bailing out on ARM. diff --git a/src/objects.h b/src/objects.h index de4cffcfb7..b51121338b 100644 --- a/src/objects.h +++ b/src/objects.h @@ -4368,6 +4368,11 @@ class SharedFunctionInfo: public HeapObject { // Enable deoptimization support through recompiled code. void EnableDeoptimizationSupport(Code* recompiled); + // Disable (further) attempted optimization of all functions sharing this + // shared function info. The function is the one we actually tried to + // optimize. + void DisableOptimization(JSFunction* function); + // Lookup the bailout ID and ASSERT that it exists in the non-optimized // code, returns whether it asserted (i.e., always true if assertions are // disabled).