[parser] Skipping inner funcs: Distinguish between skippable and non-skippable functions.

We collect function data for 2 purposes:
- Variable allocation data for lazy parsed functions which contain skippable functions.
- Data needed for creating FunctionLiterals for skippable functions.

In some cases, recompilation happens, and we need to make sure we're not trying
to skip a non-skippable function.

At the moment, we don't collect data for eagerly parsed scopes, since the
assumption is that they'll never get recompiled. (Fixing that will bigger design
changes.)

After this, we're down to 2 failures for mjsunit + --experimental-preparser-scope-analysis.


BUG=v8:5516

Change-Id: I704d488269f6d20a4b14596f2a0acc342ede32cb
Reviewed-on: https://chromium-review.googlesource.com/486802
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44848}

src/parsing/parser.cc

@@ -2863,7 +2863,8 @@ Parser::LazyParsingResult Parser::SkipFunction(FunctionKind kind,
   if (preparsed_scope_data_->Consuming()) {
     DCHECK(FLAG_experimental_preparser_scope_analysis);
     const PreParseData::FunctionData& data =
-        preparsed_scope_data_->FindFunction(function_scope->start_position());
+        preparsed_scope_data_->FindSkippableFunction(
+            function_scope->start_position());
     if (data.is_valid()) {
       function_scope->set_is_skipped_function(true);
       function_scope->outer_scope()->SetMustUsePreParsedScopeData();

src/parsing/preparsed-scope-data.cc

@@ -20,7 +20,7 @@ class VariableMaybeAssignedField
 class VariableContextAllocatedField
     : public BitField16<bool, VariableMaybeAssignedField::kNext, 1> {};
 
-const int kFunctionDataSize = 7;
+const int kFunctionDataSize = 8;
 
 }  // namespace
 
@@ -88,6 +88,12 @@ void PreParsedScopeData::SaveData(Scope* scope) {
   backing_store_[data_end_index] = static_cast<uint32_t>(backing_store_.size());
 }
 
+void PreParsedScopeData::AddSkippableFunction(
+    int start_position, const PreParseData::FunctionData& function_data) {
+  AddFunction(start_position, function_data);
+  skippable_functions_.insert(start_position);
+}
+
 void PreParsedScopeData::AddFunction(
     int start_position, const PreParseData::FunctionData& function_data) {
   function_index_.AddFunctionData(start_position, function_data);
@@ -120,7 +126,7 @@ void PreParsedScopeData::RestoreData(Scope* scope, uint32_t* index_ptr) const {
   if (scope->scope_type() == ScopeType::FUNCTION_SCOPE &&
       !scope->AsDeclarationScope()->is_arrow_scope()) {
     const PreParseData::FunctionData& data =
-        FindFunction(scope->start_position());
+        function_index_.GetFunctionData(scope->start_position());
     DCHECK_EQ(data.end, scope->end_position());
     // FIXME(marja): unify num_parameters too and DCHECK here.
     DCHECK_EQ(data.language_mode, scope->language_mode());
@@ -185,11 +191,14 @@ Handle<PodArray<uint32_t>> PreParsedScopeData::Serialize(
     array->set(i++, function_data.num_inner_functions);
     array->set(i++, function_data.language_mode);
     array->set(i++, function_data.uses_super_property);
+    array->set(i++, skippable_functions_.find(item.first) !=
+                        skippable_functions_.end());
   }
 
   for (size_t j = 0; j < backing_store_.size(); ++j) {
     array->set(i++, static_cast<uint32_t>(backing_store_[j]));
   }
+  DCHECK_EQ(array->length(), length);
   return array;
 }
 
@@ -212,6 +221,9 @@ void PreParsedScopeData::Deserialize(PodArray<uint32_t>* array) {
         start, PreParseData::FunctionData(
                    array->get(i + 2), array->get(i + 3), array->get(i + 4),
                    LanguageMode(array->get(i + 5)), array->get(i + 6)));
+    if (array->get(i + 7)) {
+      skippable_functions_.insert(start);
+    }
   }
   CHECK_EQ(function_index_.size(), function_count);
 
@@ -221,8 +233,11 @@ void PreParsedScopeData::Deserialize(PodArray<uint32_t>* array) {
   }
 }
 
-PreParseData::FunctionData PreParsedScopeData::FindFunction(
+PreParseData::FunctionData PreParsedScopeData::FindSkippableFunction(
     int start_pos) const {
+  if (skippable_functions_.find(start_pos) == skippable_functions_.end()) {
+    return PreParseData::FunctionData();
+  }
   return function_index_.GetFunctionData(start_pos);
 }
 

src/parsing/preparsed-scope-data.h

@@ -5,6 +5,7 @@
 #ifndef V8_PARSING_PREPARSED_SCOPE_DATA_H_
 #define V8_PARSING_PREPARSED_SCOPE_DATA_H_
 
+#include <set>
 #include <unordered_map>
 #include <vector>
 
@@ -65,8 +66,15 @@ class PreParsedScopeData {
   // subscopes') variables.
   void SaveData(Scope* scope);
 
-  // Saves data for a function that can be later skipped, or a function for
-  // which we save variable allocation data.
+  // Save data for a function we might skip later. The data is used later for
+  // creating a FunctionLiteral.
+  void AddSkippableFunction(int start_position,
+                            const PreParseData::FunctionData& function_data);
+
+  // Save variable allocation data for function which contains skippable
+  // functions.
+  void AddFunction(int start_position,
+                   const PreParseData::FunctionData& function_data);
 
   // FIXME(marja): We need different kinds of data for the two types of
   // functions. For a skippable function we need the end position + the data
@@ -74,8 +82,6 @@ class PreParsedScopeData {
   // skippable functions, we need the data affecting context allocation status
   // of the variables (but e.g., no end position). Currently we just save the
   // same data for both. Here we can save less data.
-  void AddFunction(int start_position,
-                   const PreParseData::FunctionData& function_data);
 
   // Restores the information needed for allocating the Scopes's (and its
   // subscopes') variables.
@@ -89,7 +95,7 @@ class PreParsedScopeData {
 
   bool Producing() const { return !has_data_; }
 
-  PreParseData::FunctionData FindFunction(int start_pos) const;
+  PreParseData::FunctionData FindSkippableFunction(int start_pos) const;
 
  private:
   friend class ScopeTestHelper;
@@ -112,6 +118,8 @@ class PreParsedScopeData {
   PreParseData function_index_;
   // Start pos -> position in backing_store_.
   std::unordered_map<uint32_t, uint32_t> function_data_positions_;
+  // Start positions of skippable functions.
+  std::set<uint32_t> skippable_functions_;
 
   bool has_data_ = false;
 

src/parsing/preparser.cc

@@ -217,6 +217,8 @@ PreParser::PreParseResult PreParser::PreParseFunction(
 
     if (FLAG_experimental_preparser_scope_analysis &&
         preparsed_scope_data_ != nullptr) {
+      // We're not going to skip this function, but it might contain skippable
+      // functions inside it.
       preparsed_scope_data_->AddFunction(
           scope()->start_position(),
           PreParseData::FunctionData(
@@ -355,7 +357,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
 
   if (FLAG_experimental_preparser_scope_analysis &&
       track_unresolved_variables_ && preparsed_scope_data_ != nullptr) {
-    preparsed_scope_data_->AddFunction(
+    preparsed_scope_data_->AddSkippableFunction(
         start_position,
         PreParseData::FunctionData(
             end_position, scope()->num_parameters(),