From 34534588fb68cb2bce31f3224578b38c97cac583 Mon Sep 17 00:00:00 2001 From: "fschneider@chromium.org" Date: Fri, 14 Oct 2011 08:43:27 +0000 Subject: [PATCH] Fix a number of bugs with inlining calls as function. 1. Record AST id for CallFunctionStub. 2. Correctly extract cached target from CallFunctionStub inline cache. 3. Fix a bug when inling call as a function in effect or value context: Handle abnormal exits correcty. 4. Fix a bug when inlining call as a function in test context: drop function correctly from true and false block. 5. Avoid inlining mutually recursive functions by checking the stack of function states before inlining. This was not a bug, but is just a more general check to avoid recursive inlining. Review URL: http://codereview.chromium.org/8258012 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9619 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen.cc | 26 +++++++++---- src/ia32/full-codegen-ia32.cc | 2 +- src/type-info.cc | 91 ++++++++++++++++++++----------------------- src/type-info.h | 1 - 4 files changed, 62 insertions(+), 58 deletions(-) diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 0cd0f97..045bd98 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -4539,9 +4539,9 @@ bool HGraphBuilder::TryInline(Call* expr) { return false; } - CompilationInfo* outer_info = info(); #if !defined(V8_TARGET_ARCH_IA32) // Target must be able to use caller's context. + CompilationInfo* outer_info = info(); if (target->context() != outer_info->closure()->context() || outer_info->scope()->contains_with() || outer_info->scope()->num_heap_slots() > 0) { @@ -4564,9 +4564,13 @@ bool HGraphBuilder::TryInline(Call* expr) { } // Don't inline recursive functions. - if (*target_shared == outer_info->closure()->shared()) { - TraceInline(target, caller, "target is recursive"); - return false; + for (FunctionState* state = function_state(); + state != NULL; + state = state->outer()) { + if (state->compilation_info()->closure()->shared() == *target_shared) { + TraceInline(target, caller, "target is recursive"); + return false; + } } // We don't want to add more than a certain number of nodes from inlining. @@ -5071,18 +5075,25 @@ void HGraphBuilder::VisitCall(Call* expr) { // The function is lingering in the deoptimization environment. // Handle it by case analysis on the AST context. if (ast_context()->IsEffect()) { + if (current_block() == NULL) return; + ASSERT(Top() == function); Drop(1); } else if (ast_context()->IsValue()) { + if (current_block() == NULL) return; HValue* result = Pop(); + ASSERT(Top() == function); Drop(1); Push(result); } else if (ast_context()->IsTest()) { + ASSERT(current_block() == NULL); TestContext* context = TestContext::cast(ast_context()); - if (context->if_true()->HasPredecessor()) { + if (context->if_true()->HasPredecessor() && + context->if_true()->last_environment()->Top() == function) { context->if_true()->last_environment()->Drop(1); } - if (context->if_false()->HasPredecessor()) { - context->if_true()->last_environment()->Drop(1); + if (context->if_false()->HasPredecessor() && + context->if_false()->last_environment()->Top() == function) { + context->if_false()->last_environment()->Drop(1); } } else { UNREACHABLE(); @@ -5302,7 +5313,6 @@ void HGraphBuilder::VisitBitNot(UnaryOperation* expr) { void HGraphBuilder::VisitNot(UnaryOperation* expr) { - // TODO(svenpanne) Perhaps a switch/virtual function is nicer here. if (ast_context()->IsTest()) { TestContext* context = TestContext::cast(ast_context()); VisitForControl(expr->expression(), diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 33d5cab..d245348 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -2104,7 +2104,7 @@ void FullCodeGenerator::EmitCallWithStub(Call* expr, CallFunctionFlags flags) { flags = static_cast(flags | RECORD_CALL_TARGET); } CallFunctionStub stub(arg_count, flags); - __ CallStub(&stub); + __ CallStub(&stub, expr->id()); if (record_call_target) { // There is a one element cache in the instruction stream. #ifdef DEBUG diff --git a/src/type-info.cc b/src/type-info.cc index a4b16f4..bdda0dd 100644 --- a/src/type-info.cc +++ b/src/type-info.cc @@ -496,61 +496,56 @@ void TypeFeedbackOracle::RelocateRelocInfos(ZoneList* infos, void TypeFeedbackOracle::ProcessRelocInfos(ZoneList* infos) { for (int i = 0; i < infos->length(); i++) { - Address target_address = (*infos)[i].target_address(); + RelocInfo reloc_entry = (*infos)[i]; + Address target_address = reloc_entry.target_address(); unsigned ast_id = static_cast((*infos)[i].data()); - ProcessTargetAt(target_address, ast_id); - } -} - + Code* target = Code::GetCodeFromTargetAddress(target_address); + switch (target->kind()) { + case Code::LOAD_IC: + case Code::STORE_IC: + case Code::CALL_IC: + case Code::KEYED_CALL_IC: + if (target->ic_state() == MONOMORPHIC) { + if (target->kind() == Code::CALL_IC && + target->check_type() != RECEIVER_MAP_CHECK) { + SetInfo(ast_id, Smi::FromInt(target->check_type())); + } else { + Object* map = target->FindFirstMap(); + SetInfo(ast_id, map == NULL ? static_cast(target) : map); + } + } else if (target->ic_state() == MEGAMORPHIC) { + SetInfo(ast_id, target); + } + break; -void TypeFeedbackOracle::ProcessTargetAt(Address target_address, - unsigned ast_id) { - Code* target = Code::GetCodeFromTargetAddress(target_address); - switch (target->kind()) { - case Code::LOAD_IC: - case Code::STORE_IC: - case Code::CALL_IC: - case Code::KEYED_CALL_IC: - if (target->ic_state() == MONOMORPHIC) { - if (target->kind() == Code::CALL_IC && - target->check_type() != RECEIVER_MAP_CHECK) { - SetInfo(ast_id, Smi::FromInt(target->check_type())); - } else { - Object* map = target->FindFirstMap(); - SetInfo(ast_id, map == NULL ? static_cast(target) : map); + case Code::KEYED_LOAD_IC: + case Code::KEYED_STORE_IC: + if (target->ic_state() == MONOMORPHIC || + target->ic_state() == MEGAMORPHIC) { + SetInfo(ast_id, target); } - } else if (target->ic_state() == MEGAMORPHIC) { - SetInfo(ast_id, target); - } - break; + break; - case Code::KEYED_LOAD_IC: - case Code::KEYED_STORE_IC: - if (target->ic_state() == MONOMORPHIC || - target->ic_state() == MEGAMORPHIC) { + case Code::UNARY_OP_IC: + case Code::BINARY_OP_IC: + case Code::COMPARE_IC: + case Code::TO_BOOLEAN_IC: SetInfo(ast_id, target); - } - break; - - case Code::UNARY_OP_IC: - case Code::BINARY_OP_IC: - case Code::COMPARE_IC: - case Code::TO_BOOLEAN_IC: - SetInfo(ast_id, target); - break; - - case Code::STUB: - if (target->major_key() == CodeStub::CallFunction && - target->has_function_cache()) { - Object* value = CallFunctionStub::GetCachedValue(target_address); - if (value->IsJSFunction()) { - SetInfo(ast_id, value); + break; + + case Code::STUB: + if (target->major_key() == CodeStub::CallFunction && + target->has_function_cache()) { + Object* value = CallFunctionStub::GetCachedValue(reloc_entry.pc()); + if (value->IsJSFunction()) { + SetInfo(ast_id, value); + } } - } - break; + break; - default: - break; + default: + break; + } } } diff --git a/src/type-info.h b/src/type-info.h index 0ba10aa..2c3543e 100644 --- a/src/type-info.h +++ b/src/type-info.h @@ -277,7 +277,6 @@ class TypeFeedbackOracle BASE_EMBEDDED { byte* old_start, byte* new_start); void ProcessRelocInfos(ZoneList* infos); - void ProcessTargetAt(Address target_address, unsigned ast_id); // Returns an element from the backing store. Returns undefined if // there is no information. -- 2.7.4