From 3527e29e8ee0a5b963021a212efd262840446a81 Mon Sep 17 00:00:00 2001 From: "kasperl@chromium.org" Date: Tue, 24 Aug 2010 09:04:17 +0000 Subject: [PATCH] Remove the full codegen syntax checker completely but be careful to avoid making code with loops run too slowly. Review URL: http://codereview.chromium.org/3107033 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5324 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ast.h | 6 +- src/compiler.cc | 27 +-- src/full-codegen.cc | 409 +---------------------------------- src/full-codegen.h | 23 -- src/parser.cc | 28 ++- test/cctest/test-log-stack-tracer.cc | 22 +- 6 files changed, 45 insertions(+), 470 deletions(-) diff --git a/src/ast.h b/src/ast.h index 7dc8c8d..dfef84d 100644 --- a/src/ast.h +++ b/src/ast.h @@ -1416,7 +1416,8 @@ class FunctionLiteral: public Expression { int num_parameters, int start_position, int end_position, - bool is_expression) + bool is_expression, + bool contains_loops) : name_(name), scope_(scope), body_(body), @@ -1429,6 +1430,7 @@ class FunctionLiteral: public Expression { start_position_(start_position), end_position_(end_position), is_expression_(is_expression), + contains_loops_(contains_loops), function_token_position_(RelocInfo::kNoPosition), inferred_name_(Heap::empty_string()), try_full_codegen_(false) { @@ -1450,6 +1452,7 @@ class FunctionLiteral: public Expression { int start_position() const { return start_position_; } int end_position() const { return end_position_; } bool is_expression() const { return is_expression_; } + bool contains_loops() const { return contains_loops_; } int materialized_literal_count() { return materialized_literal_count_; } int expected_property_count() { return expected_property_count_; } @@ -1490,6 +1493,7 @@ class FunctionLiteral: public Expression { int start_position_; int end_position_; bool is_expression_; + bool contains_loops_; int function_token_position_; Handle inferred_name_; bool try_full_codegen_; diff --git a/src/compiler.cc b/src/compiler.cc index 8d3deb5..bdeb4aa 100755 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -104,15 +104,9 @@ static Handle MakeCode(Handle context, CompilationInfo* info) { bool is_run_once = (shared.is_null()) ? info->scope()->is_global_scope() : (shared->is_toplevel() || shared->try_full_codegen()); - - if (AlwaysFullCompiler()) { + bool use_full = FLAG_full_compiler && !function->contains_loops(); + if (AlwaysFullCompiler() || (use_full && is_run_once)) { return FullCodeGenerator::MakeCode(info); - } else if (FLAG_full_compiler && is_run_once) { - FullCodeGenSyntaxChecker checker; - checker.Check(function); - if (checker.has_supported_syntax()) { - return FullCodeGenerator::MakeCode(info); - } } AssignedVariablesAnalyzer ava(function); @@ -476,21 +470,10 @@ Handle Compiler::BuildFunctionInfo(FunctionLiteral* literal, CompilationInfo info(literal, script, false); bool is_run_once = literal->try_full_codegen(); - bool is_compiled = false; - - if (AlwaysFullCompiler()) { + bool use_full = FLAG_full_compiler && !literal->contains_loops(); + if (AlwaysFullCompiler() || (use_full && is_run_once)) { code = FullCodeGenerator::MakeCode(&info); - is_compiled = true; - } else if (FLAG_full_compiler && is_run_once) { - FullCodeGenSyntaxChecker checker; - checker.Check(literal); - if (checker.has_supported_syntax()) { - code = FullCodeGenerator::MakeCode(&info); - is_compiled = true; - } - } - - if (!is_compiled) { + } else { // We fall back to the classic V8 code generator. AssignedVariablesAnalyzer ava(literal); if (!ava.Analyze()) return Handle::null(); diff --git a/src/full-codegen.cc b/src/full-codegen.cc index dfc8706..becda8f 100644 --- a/src/full-codegen.cc +++ b/src/full-codegen.cc @@ -38,412 +38,6 @@ namespace v8 { namespace internal { -#define BAILOUT(reason) \ - do { \ - if (FLAG_trace_bailout) { \ - PrintF("%s\n", reason); \ - } \ - has_supported_syntax_ = false; \ - return; \ - } while (false) - - -#define CHECK_BAILOUT \ - do { \ - if (!has_supported_syntax_) return; \ - } while (false) - - -void FullCodeGenSyntaxChecker::Check(FunctionLiteral* fun) { - Scope* scope = fun->scope(); - VisitDeclarations(scope->declarations()); - CHECK_BAILOUT; - - VisitStatements(fun->body()); -} - - -void FullCodeGenSyntaxChecker::VisitDeclarations( - ZoneList* decls) { - for (int i = 0; i < decls->length(); i++) { - Visit(decls->at(i)); - CHECK_BAILOUT; - } -} - - -void FullCodeGenSyntaxChecker::VisitStatements(ZoneList* stmts) { - for (int i = 0, len = stmts->length(); i < len; i++) { - Visit(stmts->at(i)); - CHECK_BAILOUT; - } -} - - -void FullCodeGenSyntaxChecker::VisitDeclaration(Declaration* decl) { - Property* prop = decl->proxy()->AsProperty(); - if (prop != NULL) { - Visit(prop->obj()); - Visit(prop->key()); - } - - if (decl->fun() != NULL) { - Visit(decl->fun()); - } -} - - -void FullCodeGenSyntaxChecker::VisitBlock(Block* stmt) { - VisitStatements(stmt->statements()); -} - - -void FullCodeGenSyntaxChecker::VisitExpressionStatement( - ExpressionStatement* stmt) { - Visit(stmt->expression()); -} - - -void FullCodeGenSyntaxChecker::VisitEmptyStatement(EmptyStatement* stmt) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitIfStatement(IfStatement* stmt) { - Visit(stmt->condition()); - CHECK_BAILOUT; - Visit(stmt->then_statement()); - CHECK_BAILOUT; - Visit(stmt->else_statement()); -} - - -void FullCodeGenSyntaxChecker::VisitContinueStatement(ContinueStatement* stmt) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitBreakStatement(BreakStatement* stmt) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitReturnStatement(ReturnStatement* stmt) { - Visit(stmt->expression()); -} - - -void FullCodeGenSyntaxChecker::VisitWithEnterStatement( - WithEnterStatement* stmt) { - Visit(stmt->expression()); -} - - -void FullCodeGenSyntaxChecker::VisitWithExitStatement(WithExitStatement* stmt) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitSwitchStatement(SwitchStatement* stmt) { - BAILOUT("SwitchStatement"); -} - - -void FullCodeGenSyntaxChecker::VisitDoWhileStatement(DoWhileStatement* stmt) { - Visit(stmt->cond()); - CHECK_BAILOUT; - Visit(stmt->body()); -} - - -void FullCodeGenSyntaxChecker::VisitWhileStatement(WhileStatement* stmt) { - Visit(stmt->cond()); - CHECK_BAILOUT; - Visit(stmt->body()); -} - - -void FullCodeGenSyntaxChecker::VisitForStatement(ForStatement* stmt) { - if (!FLAG_always_full_compiler) BAILOUT("ForStatement"); - if (stmt->init() != NULL) { - Visit(stmt->init()); - CHECK_BAILOUT; - } - if (stmt->cond() != NULL) { - Visit(stmt->cond()); - CHECK_BAILOUT; - } - Visit(stmt->body()); - if (stmt->next() != NULL) { - CHECK_BAILOUT; - Visit(stmt->next()); - } -} - - -void FullCodeGenSyntaxChecker::VisitForInStatement(ForInStatement* stmt) { - BAILOUT("ForInStatement"); -} - - -void FullCodeGenSyntaxChecker::VisitTryCatchStatement(TryCatchStatement* stmt) { - Visit(stmt->try_block()); - CHECK_BAILOUT; - Visit(stmt->catch_block()); -} - - -void FullCodeGenSyntaxChecker::VisitTryFinallyStatement( - TryFinallyStatement* stmt) { - Visit(stmt->try_block()); - CHECK_BAILOUT; - Visit(stmt->finally_block()); -} - - -void FullCodeGenSyntaxChecker::VisitDebuggerStatement( - DebuggerStatement* stmt) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitFunctionLiteral(FunctionLiteral* expr) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitSharedFunctionInfoLiteral( - SharedFunctionInfoLiteral* expr) { - BAILOUT("SharedFunctionInfoLiteral"); -} - - -void FullCodeGenSyntaxChecker::VisitConditional(Conditional* expr) { - Visit(expr->condition()); - CHECK_BAILOUT; - Visit(expr->then_expression()); - CHECK_BAILOUT; - Visit(expr->else_expression()); -} - - -void FullCodeGenSyntaxChecker::VisitSlot(Slot* expr) { - UNREACHABLE(); -} - - -void FullCodeGenSyntaxChecker::VisitVariableProxy(VariableProxy* expr) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitLiteral(Literal* expr) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitRegExpLiteral(RegExpLiteral* expr) { - // Supported. -} - - -void FullCodeGenSyntaxChecker::VisitObjectLiteral(ObjectLiteral* expr) { - ZoneList* properties = expr->properties(); - - for (int i = 0, len = properties->length(); i < len; i++) { - ObjectLiteral::Property* property = properties->at(i); - if (property->IsCompileTimeValue()) continue; - Visit(property->key()); - CHECK_BAILOUT; - Visit(property->value()); - CHECK_BAILOUT; - } -} - - -void FullCodeGenSyntaxChecker::VisitArrayLiteral(ArrayLiteral* expr) { - ZoneList* subexprs = expr->values(); - for (int i = 0, len = subexprs->length(); i < len; i++) { - Expression* subexpr = subexprs->at(i); - if (subexpr->AsLiteral() != NULL) continue; - if (CompileTimeValue::IsCompileTimeValue(subexpr)) continue; - Visit(subexpr); - CHECK_BAILOUT; - } -} - - -void FullCodeGenSyntaxChecker::VisitCatchExtensionObject( - CatchExtensionObject* expr) { - Visit(expr->key()); - CHECK_BAILOUT; - Visit(expr->value()); -} - - -void FullCodeGenSyntaxChecker::VisitAssignment(Assignment* expr) { - Token::Value op = expr->op(); - if (op == Token::INIT_CONST) BAILOUT("initialize constant"); - - Variable* var = expr->target()->AsVariableProxy()->AsVariable(); - Property* prop = expr->target()->AsProperty(); - ASSERT(var == NULL || prop == NULL); - if (var != NULL) { - if (var->mode() == Variable::CONST) BAILOUT("Assignment to const"); - // All other variables are supported. - } else if (prop != NULL) { - Visit(prop->obj()); - CHECK_BAILOUT; - Visit(prop->key()); - CHECK_BAILOUT; - } else { - // This is a throw reference error. - BAILOUT("non-variable/non-property assignment"); - } - - Visit(expr->value()); -} - - -void FullCodeGenSyntaxChecker::VisitThrow(Throw* expr) { - Visit(expr->exception()); -} - - -void FullCodeGenSyntaxChecker::VisitProperty(Property* expr) { - Visit(expr->obj()); - CHECK_BAILOUT; - Visit(expr->key()); -} - - -void FullCodeGenSyntaxChecker::VisitCall(Call* expr) { - Expression* fun = expr->expression(); - ZoneList* args = expr->arguments(); - Variable* var = fun->AsVariableProxy()->AsVariable(); - - // Check for supported calls - if (var != NULL && var->is_possibly_eval()) { - BAILOUT("call to the identifier 'eval'"); - } else if (var != NULL && !var->is_this() && var->is_global()) { - // Calls to global variables are supported. - } else if (var != NULL && var->slot() != NULL && - var->slot()->type() == Slot::LOOKUP) { - BAILOUT("call to a lookup slot"); - } else if (fun->AsProperty() != NULL) { - Property* prop = fun->AsProperty(); - Visit(prop->obj()); - CHECK_BAILOUT; - Visit(prop->key()); - CHECK_BAILOUT; - } else { - // Otherwise the call is supported if the function expression is. - Visit(fun); - } - // Check all arguments to the call. - for (int i = 0; i < args->length(); i++) { - Visit(args->at(i)); - CHECK_BAILOUT; - } -} - - -void FullCodeGenSyntaxChecker::VisitCallNew(CallNew* expr) { - Visit(expr->expression()); - CHECK_BAILOUT; - ZoneList* args = expr->arguments(); - // Check all arguments to the call - for (int i = 0; i < args->length(); i++) { - Visit(args->at(i)); - CHECK_BAILOUT; - } -} - - -void FullCodeGenSyntaxChecker::VisitCallRuntime(CallRuntime* expr) { - // Check for inline runtime call - if (expr->name()->Get(0) == '_' && - CodeGenerator::FindInlineRuntimeLUT(expr->name()) != NULL) { - BAILOUT("inlined runtime call"); - } - // Check all arguments to the call. (Relies on TEMP meaning STACK.) - for (int i = 0; i < expr->arguments()->length(); i++) { - Visit(expr->arguments()->at(i)); - CHECK_BAILOUT; - } -} - - -void FullCodeGenSyntaxChecker::VisitUnaryOperation(UnaryOperation* expr) { - switch (expr->op()) { - case Token::ADD: - case Token::BIT_NOT: - case Token::NOT: - case Token::SUB: - case Token::TYPEOF: - case Token::VOID: - Visit(expr->expression()); - break; - case Token::DELETE: - BAILOUT("UnaryOperation: DELETE"); - default: - UNREACHABLE(); - } -} - - -void FullCodeGenSyntaxChecker::VisitCountOperation(CountOperation* expr) { - Variable* var = expr->expression()->AsVariableProxy()->AsVariable(); - Property* prop = expr->expression()->AsProperty(); - ASSERT(var == NULL || prop == NULL); - if (var != NULL) { - // All global variables are supported. - if (!var->is_global()) { - ASSERT(var->slot() != NULL); - Slot::Type type = var->slot()->type(); - if (type == Slot::LOOKUP) { - BAILOUT("CountOperation with lookup slot"); - } - } - } else if (prop != NULL) { - Visit(prop->obj()); - CHECK_BAILOUT; - Visit(prop->key()); - CHECK_BAILOUT; - } else { - // This is a throw reference error. - BAILOUT("CountOperation non-variable/non-property expression"); - } -} - - -void FullCodeGenSyntaxChecker::VisitBinaryOperation(BinaryOperation* expr) { - Visit(expr->left()); - CHECK_BAILOUT; - Visit(expr->right()); -} - - -void FullCodeGenSyntaxChecker::VisitCompareOperation(CompareOperation* expr) { - Visit(expr->left()); - CHECK_BAILOUT; - Visit(expr->right()); -} - - -void FullCodeGenSyntaxChecker::VisitCompareToNull(CompareToNull* expr) { - Visit(expr->expression()); -} - - -void FullCodeGenSyntaxChecker::VisitThisFunction(ThisFunction* expr) { - // Supported. -} - -#undef BAILOUT -#undef CHECK_BAILOUT - - void BreakableStatementChecker::Check(Statement* stmt) { Visit(stmt); } @@ -868,10 +462,9 @@ void FullCodeGenerator::EmitInlineRuntimeCall(CallRuntime* expr) { Emit##name(expr->arguments()); \ return; \ } - INLINE_RUNTIME_FUNCTION_LIST(CHECK_EMIT_INLINE_CALL) - UNREACHABLE(); #undef CHECK_EMIT_INLINE_CALL + UNREACHABLE(); } diff --git a/src/full-codegen.h b/src/full-codegen.h index bd1f756..a11aeb1 100644 --- a/src/full-codegen.h +++ b/src/full-codegen.h @@ -36,29 +36,6 @@ namespace v8 { namespace internal { -class FullCodeGenSyntaxChecker: public AstVisitor { - public: - FullCodeGenSyntaxChecker() : has_supported_syntax_(true) {} - - void Check(FunctionLiteral* fun); - - bool has_supported_syntax() { return has_supported_syntax_; } - - private: - void VisitDeclarations(ZoneList* decls); - void VisitStatements(ZoneList* stmts); - - // AST node visit functions. -#define DECLARE_VISIT(type) virtual void Visit##type(type* node); - AST_NODE_LIST(DECLARE_VISIT) -#undef DECLARE_VISIT - - bool has_supported_syntax_; - - DISALLOW_COPY_AND_ASSIGN(FullCodeGenSyntaxChecker); -}; - - // AST node visitor which can tell whether a given statement will be breakable // when the code is compiled by the full compiler in the debugger. This means // that there will be an IC (load/store/call) in the code generated for the diff --git a/src/parser.cc b/src/parser.cc index daf5387..58194ad 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -341,9 +341,7 @@ class Parser { template class BufferedZoneList { public: - - BufferedZoneList() : - list_(NULL), last_(NULL) {} + BufferedZoneList() : list_(NULL), last_(NULL) {} // Adds element at end of list. This element is buffered and can // be read using last() or removed using RemoveLast until a new Add or until @@ -414,6 +412,7 @@ class BufferedZoneList { T* last_; }; + // Accumulates RegExp atoms and assertions into lists of terms and alternatives. class RegExpBuilder: public ZoneObject { public: @@ -652,6 +651,7 @@ class RegExpParser { static const int kMaxCaptures = 1 << 16; static const uc32 kEndMarker = (1 << 21); + private: enum SubexpressionType { INITIAL, @@ -747,6 +747,10 @@ class TemporaryScope BASE_EMBEDDED { void AddProperty() { expected_property_count_++; } int expected_property_count() { return expected_property_count_; } + + void AddLoop() { loop_count_++; } + bool ContainsLoops() const { return loop_count_ > 0; } + private: // Captures the number of literals that need materialization in the // function. Includes regexp literals, and boilerplate for object @@ -756,9 +760,14 @@ class TemporaryScope BASE_EMBEDDED { // Properties count estimation. int expected_property_count_; + // Keeps track of assignments to properties of this. Used for + // optimizing constructors. bool only_simple_this_property_assignments_; Handle this_property_assignments_; + // Captures the number of loops inside the scope. + int loop_count_; + // Bookkeeping Parser* parser_; TemporaryScope* parent_; @@ -772,6 +781,7 @@ TemporaryScope::TemporaryScope(Parser* parser) expected_property_count_(0), only_simple_this_property_assignments_(false), this_property_assignments_(Factory::empty_fixed_array()), + loop_count_(0), parser_(parser), parent_(parser->temp_scope_) { parser->temp_scope_ = this; @@ -1282,7 +1292,8 @@ FunctionLiteral* Parser::ParseProgram(Handle source, 0, 0, source->length(), - false)); + false, + temp_scope.ContainsLoops())); } else if (scanner().stack_overflow()) { Top::StackOverflow(); } @@ -1382,7 +1393,8 @@ FunctionLiteral* Parser::ParseJson(Handle source) { 0, 0, source->length(), - false)); + false, + temp_scope.ContainsLoops())); } else if (scanner().stack_overflow()) { Top::StackOverflow(); } @@ -2653,6 +2665,7 @@ DoWhileStatement* Parser::ParseDoWhileStatement(ZoneStringList* labels, // DoStatement :: // 'do' Statement 'while' '(' Expression ')' ';' + temp_scope_->AddLoop(); DoWhileStatement* loop = NEW(DoWhileStatement(labels)); Target target(this, loop); @@ -2685,6 +2698,7 @@ WhileStatement* Parser::ParseWhileStatement(ZoneStringList* labels, bool* ok) { // WhileStatement :: // 'while' '(' Expression ')' Statement + temp_scope_->AddLoop(); WhileStatement* loop = NEW(WhileStatement(labels)); Target target(this, loop); @@ -2704,6 +2718,7 @@ Statement* Parser::ParseForStatement(ZoneStringList* labels, bool* ok) { // ForStatement :: // 'for' '(' Expression? ';' Expression? ';' Expression? ')' Statement + temp_scope_->AddLoop(); Statement* init = NULL; Expect(Token::FOR, CHECK_OK); @@ -3955,7 +3970,8 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, num_parameters, start_pos, end_pos, - function_name->length() > 0)); + function_name->length() > 0, + temp_scope.ContainsLoops())); if (!is_pre_parsing_) { function_literal->set_function_token_position(function_token_position); } diff --git a/test/cctest/test-log-stack-tracer.cc b/test/cctest/test-log-stack-tracer.cc index 6da1a75..312a443 100644 --- a/test/cctest/test-log-stack-tracer.cc +++ b/test/cctest/test-log-stack-tracer.cc @@ -216,25 +216,25 @@ namespace internal { class CodeGeneratorPatcher { public: CodeGeneratorPatcher() { - CodeGenerator::InlineRuntimeLUT genGetFramePointer = + CodeGenerator::InlineRuntimeLUT gen_get_frame_pointer = {&CodeGenerator::GenerateGetFramePointer, "_GetFramePointer", 0}; // _RandomHeapNumber is just used as a dummy function that has zero // arguments, the same as the _GetFramePointer function we actually patch // in. bool result = CodeGenerator::PatchInlineRuntimeEntry( NewString("_RandomHeapNumber"), - genGetFramePointer, &oldInlineEntry); + gen_get_frame_pointer, &old_inline_entry); CHECK(result); } ~CodeGeneratorPatcher() { CHECK(CodeGenerator::PatchInlineRuntimeEntry( NewString("_GetFramePointer"), - oldInlineEntry, NULL)); + old_inline_entry, NULL)); } private: - CodeGenerator::InlineRuntimeLUT oldInlineEntry; + CodeGenerator::InlineRuntimeLUT old_inline_entry; }; } } // namespace v8::internal @@ -273,9 +273,10 @@ static void CreateTraceCallerFunction(const char* func_name, // StackTracer uses Top::c_entry_fp as a starting point for stack // walking. TEST(CFromJSStackTrace) { - // TODO(711) The hack of replacing the inline runtime function - // RandomHeapNumber with GetFrameNumber does not work with the way the full - // compiler generates inline runtime calls. + // TODO(711): The hack of replacing the inline runtime function + // RandomHeapNumber with GetFrameNumber does not work with the way + // the full compiler generates inline runtime calls. + i::FLAG_full_compiler = false; i::FLAG_always_full_compiler = false; TickSample sample; @@ -313,9 +314,10 @@ TEST(CFromJSStackTrace) { // Top::c_entry_fp value. In this case, StackTracer uses passed frame // pointer value as a starting point for stack walking. TEST(PureJSStackTrace) { - // TODO(711) The hack of replacing the inline runtime function - // RandomHeapNumber with GetFrameNumber does not work with the way the full - // compiler generates inline runtime calls. + // TODO(711): The hack of replacing the inline runtime function + // RandomHeapNumber with GetFrameNumber does not work with the way + // the full compiler generates inline runtime calls. + i::FLAG_full_compiler = false; i::FLAG_always_full_compiler = false; TickSample sample; -- 2.7.4