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

@@ -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<String> inferred_name_;
   bool try_full_codegen_;

src/compiler.cc

@@ -104,15 +104,9 @@ static Handle<Code> MakeCode(Handle<Context> 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<SharedFunctionInfo> 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<SharedFunctionInfo>::null();

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<Declaration*>* decls);
-  void VisitStatements(ZoneList<Statement*>* 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

src/parser.cc

@@ -341,9 +341,7 @@ class Parser {
 template <typename T, int initial_size>
 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<FixedArray> 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<String> 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<String> 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<String> 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);
     }

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;