[Compiler] Change ShouldUseFullCodegen to check literal instead of SFI.

Changes the ShouldUseFullCodegen to use the flags on the literal instead
of the SharedFunctionInfo. Also moves the setting of the SFI flags based
on the literal to be in the final stage of unoptimized compilation since
they are no longer needed on the SFI during compilation. This is in
preparation to enable shared function infos to be created after
bytecode generation (to enable off-thread bytecode generation).

BUG=v8:5203, v8:6409

Change-Id: I15754979a704123b56dad9e1dfd5c3bb468b85c7
Reviewed-on: https://chromium-review.googlesource.com/570249
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46684}

src/ast/ast-numbering.cc

@@ -26,6 +26,7 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
         slot_cache_(zone),
         disable_fullcodegen_reason_(kNoReason),
         dont_optimize_reason_(kNoReason),
+        dont_self_optimize_(false),
         collect_type_profile_(collect_type_profile) {
     InitializeAstVisitor(stack_limit);
   }
@@ -57,16 +58,13 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
   }
 
   void IncrementNodeCount() { properties_.add_node_count(1); }
-  void DisableSelfOptimization() {
-    properties_.flags() |= AstProperties::kDontSelfOptimize;
-  }
+  void DisableSelfOptimization() { dont_self_optimize_ = true; }
   void DisableOptimization(BailoutReason reason) {
     dont_optimize_reason_ = reason;
     DisableSelfOptimization();
   }
   void DisableFullCodegen(BailoutReason reason) {
     disable_fullcodegen_reason_ = reason;
-    properties_.flags() |= AstProperties::kMustUseIgnition;
   }
 
   template <typename Node>
@@ -102,6 +100,7 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
   FeedbackSlotCache slot_cache_;
   BailoutReason disable_fullcodegen_reason_;
   BailoutReason dont_optimize_reason_;
+  bool dont_self_optimize_;
   bool collect_type_profile_;
 
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
@@ -227,8 +226,7 @@ void AstNumberingVisitor::VisitReturnStatement(ReturnStatement* node) {
   IncrementNodeCount();
   Visit(node->expression());
 
-  DCHECK(!node->is_async_return() ||
-         properties_.flags() & AstProperties::kMustUseIgnition);
+  DCHECK(!node->is_async_return() || disable_fullcodegen_reason_ != kNoReason);
 }
 
 void AstNumberingVisitor::VisitSuspend(Suspend* node) {
@@ -647,8 +645,12 @@ bool AstNumberingVisitor::Renumber(FunctionLiteral* node) {
   node->set_dont_optimize_reason(dont_optimize_reason());
   node->set_suspend_count(suspend_count_);
 
-  if (FLAG_trace_opt && FLAG_stress_fullcodegen) {
-    if (disable_fullcodegen_reason_ != kNoReason) {
+  if (dont_self_optimize_) {
+    node->set_dont_self_optimize();
+  }
+  if (disable_fullcodegen_reason_ != kNoReason) {
+    node->set_must_use_ignition();
+    if (FLAG_trace_opt && FLAG_stress_fullcodegen) {
       // TODO(leszeks): This is a quick'n'dirty fix to allow the debug name of
       // the function to be accessed in the below print. This DCHECK will fail
       // if we move ast numbering off the main thread, but that won't be before

src/ast/ast.h

@@ -9,7 +9,6 @@
 #include "src/ast/modules.h"
 #include "src/ast/variables.h"
 #include "src/bailout-reason.h"
-#include "src/base/flags.h"
 #include "src/factory.h"
 #include "src/globals.h"
 #include "src/isolate.h"
@@ -157,18 +156,8 @@ class FeedbackSlotCache {
 
 class AstProperties final BASE_EMBEDDED {
  public:
-  enum Flag {
-    kNoFlags = 0,
-    kDontSelfOptimize = 1 << 0,
-    kMustUseIgnition = 1 << 1
-  };
-
-  typedef base::Flags<Flag> Flags;
-
   explicit AstProperties(Zone* zone) : node_count_(0), spec_(zone) {}
 
-  Flags& flags() { return flags_; }
-  Flags flags() const { return flags_; }
   int node_count() { return node_count_; }
   void add_node_count(int count) { node_count_ += count; }
 
@@ -176,12 +165,10 @@ class AstProperties final BASE_EMBEDDED {
   FeedbackVectorSpec* get_spec() { return &spec_; }
 
  private:
-  Flags flags_;
   int node_count_;
   FeedbackVectorSpec spec_;
 };
 
-DEFINE_OPERATORS_FOR_FLAGS(AstProperties::Flags)
 
 class AstNode: public ZoneObject {
  public:
@@ -2513,14 +2500,24 @@ class FunctionLiteral final : public Expression {
   }
   FunctionKind kind() const;
 
-  int ast_node_count() { return ast_properties_.node_count(); }
-  AstProperties::Flags flags() const { return ast_properties_.flags(); }
   void set_ast_properties(AstProperties* ast_properties) {
     ast_properties_ = *ast_properties;
   }
+  int ast_node_count() { return ast_properties_.node_count(); }
   const FeedbackVectorSpec* feedback_vector_spec() const {
     return ast_properties_.get_spec();
   }
+
+  bool must_use_ignition() { return MustUseIgnitionField::decode(bit_field_); }
+  void set_must_use_ignition() {
+    bit_field_ = MustUseIgnitionField::update(bit_field_, true);
+  }
+
+  bool dont_self_optimize() { return DontSelfOptimize::decode(bit_field_); }
+  void set_dont_self_optimize() {
+    bit_field_ = DontSelfOptimize::update(bit_field_, true);
+  }
+
   bool dont_optimize() { return dont_optimize_reason() != kNoReason; }
   BailoutReason dont_optimize_reason() {
     return DontOptimizeReasonField::decode(bit_field_);
@@ -2592,9 +2589,13 @@ class FunctionLiteral final : public Expression {
   class HasDuplicateParameters : public BitField<bool, Pretenure::kNext, 1> {};
   class ShouldNotBeUsedOnceHintField
       : public BitField<bool, HasDuplicateParameters::kNext, 1> {};
+  class MustUseIgnitionField
+      : public BitField<bool, ShouldNotBeUsedOnceHintField::kNext, 1> {};
+  // TODO(6409): Remove when Full-Codegen dies.
+  class DontSelfOptimize
+      : public BitField<bool, MustUseIgnitionField::kNext, 1> {};
   class DontOptimizeReasonField
-      : public BitField<BailoutReason, ShouldNotBeUsedOnceHintField::kNext, 8> {
-  };
+      : public BitField<BailoutReason, DontSelfOptimize::kNext, 8> {};
 
   int expected_property_count_;
   int parameter_count_;

src/compilation-info.cc

@@ -120,8 +120,9 @@ bool CompilationInfo::is_this_defined() const { return !IsStub(); }
 // Primitive functions are unlikely to be picked up by the stack-walking
 // profiler, so they trigger their own optimization when they're called
 // for the SharedFunctionInfo::kCallsUntilPrimitiveOptimization-th time.
+// TODO(6409) Remove when Full-Codegen dies.
 bool CompilationInfo::ShouldSelfOptimize() {
-  return FLAG_opt && !(literal()->flags() & AstProperties::kDontSelfOptimize) &&
+  return FLAG_opt && !literal()->dont_self_optimize() &&
          !literal()->dont_optimize() &&
          literal()->scope()->AllowsLazyCompilation() &&
          !shared_info()->optimization_disabled();

src/compiler.cc

@@ -266,27 +266,22 @@ void EnsureFeedbackMetadata(CompilationInfo* info) {
       info->literal()->feedback_vector_spec()));
 }
 
-bool ShouldUseFullCodegen(Handle<SharedFunctionInfo> shared) {
+bool ShouldUseFullCodegen(FunctionLiteral* literal) {
   // Code which can't be supported by the old pipeline should use Ignition.
-  if (shared->must_use_ignition()) return false;
+  if (literal->must_use_ignition()) return false;
 
   // Resumable functions are not supported by {FullCodeGenerator}, suspended
   // activations stored as {JSGeneratorObject} on the heap always assume the
   // underlying code to be based on the bytecode array.
-  DCHECK(!IsResumableFunction(shared->kind()));
+  DCHECK(!IsResumableFunction(literal->kind()));
 
   // Use full-codegen for asm.js functions.
-  if (shared->asm_function()) return true;
+  if (literal->scope()->asm_function()) return true;
 
   // If stressing full-codegen then use it for all functions it can support.
   return FLAG_stress_fullcodegen;
 }
 
-bool ShouldUseFullCodegen(CompilationInfo* info) {
-  DCHECK(info->has_shared_info());
-  return ShouldUseFullCodegen(info->shared_info());
-}
-
 bool UseAsmWasm(DeclarationScope* scope, Handle<SharedFunctionInfo> shared_info,
                 bool is_debug) {
   // Check whether asm.js validation is enabled.
@@ -322,7 +317,7 @@ CompilationJob* GetUnoptimizedCompilationJob(CompilationInfo* info) {
   DCHECK_NOT_NULL(info->literal());
   DCHECK_NOT_NULL(info->scope());
 
-  if (ShouldUseFullCodegen(info)) {
+  if (ShouldUseFullCodegen(info->literal())) {
     return FullCodeGenerator::NewCompilationJob(info);
   } else {
     return interpreter::Interpreter::NewCompilationJob(info);
@@ -362,22 +357,6 @@ void InstallUnoptimizedCode(CompilationInfo* info) {
   }
 }
 
-CompilationJob::Status FinalizeUnoptimizedCompilationJob(CompilationJob* job) {
-  CompilationJob::Status status = job->FinalizeJob();
-  if (status == CompilationJob::SUCCEEDED) {
-    CompilationInfo* info = job->info();
-    EnsureFeedbackMetadata(info);
-    DCHECK(!info->code().is_null());
-    if (info->parse_info()->literal()->should_be_used_once_hint()) {
-      info->code()->MarkToBeExecutedOnce(info->isolate());
-    }
-    InstallUnoptimizedCode(info);
-    RecordFunctionCompilation(CodeEventListener::FUNCTION_TAG, info);
-    job->RecordUnoptimizedCompilationStats();
-  }
-  return status;
-}
-
 void SetSharedFunctionFlagsFromLiteral(FunctionLiteral* literal,
                                        Handle<SharedFunctionInfo> shared_info) {
   // Don't overwrite values set by the bootstrapper.
@@ -391,9 +370,24 @@ void SetSharedFunctionFlagsFromLiteral(FunctionLiteral* literal,
   if (literal->dont_optimize_reason() != kNoReason) {
     shared_info->DisableOptimization(literal->dont_optimize_reason());
   }
-  if (literal->flags() & AstProperties::kMustUseIgnition) {
-    shared_info->set_must_use_ignition(true);
+}
+
+CompilationJob::Status FinalizeUnoptimizedCompilationJob(CompilationJob* job) {
+  CompilationInfo* info = job->info();
+  SetSharedFunctionFlagsFromLiteral(info->literal(), info->shared_info());
+
+  CompilationJob::Status status = job->FinalizeJob();
+  if (status == CompilationJob::SUCCEEDED) {
+    EnsureFeedbackMetadata(info);
+    DCHECK(!info->code().is_null());
+    if (info->parse_info()->literal()->should_be_used_once_hint()) {
+      info->code()->MarkToBeExecutedOnce(info->isolate());
+    }
+    InstallUnoptimizedCode(info);
+    RecordFunctionCompilation(CodeEventListener::FUNCTION_TAG, info);
+    job->RecordUnoptimizedCompilationStats();
   }
+  return status;
 }
 
 bool Renumber(ParseInfo* parse_info,
@@ -419,10 +413,6 @@ bool Renumber(ParseInfo* parse_info,
                               collect_type_profile)) {
     return false;
   }
-  if (!parse_info->shared_info().is_null()) {
-    SetSharedFunctionFlagsFromLiteral(parse_info->literal(),
-                                      parse_info->shared_info());
-  }
   return true;
 }
 
@@ -468,10 +458,6 @@ bool CompileUnoptimizedInnerFunctions(
         Compiler::GetSharedFunctionInfo(literal, script, outer_info);
     if (shared->is_compiled()) continue;
 
-    // The {literal} has already been numbered because AstNumbering decends into
-    // eagerly compiled function literals.
-    SetSharedFunctionFlagsFromLiteral(literal, shared);
-
     // Try to enqueue the eager function on the compiler dispatcher.
     CompilerDispatcher* dispatcher = isolate->compiler_dispatcher();
     if (UseCompilerDispatcher(inner_function_mode, dispatcher, literal->scope(),
@@ -604,9 +590,6 @@ MUST_USE_RESULT MaybeHandle<Code> GetUnoptimizedCode(
     }
   }
 
-  if (info->parse_info()->is_toplevel()) {
-    EnsureSharedFunctionInfosArrayOnScript(info);
-  }
   DCHECK_EQ(info->shared_info()->language_mode(),
             info->literal()->language_mode());
 
@@ -801,9 +784,9 @@ MaybeHandle<Code> GetOptimizedCode(Handle<JSFunction> function,
   DCHECK(!isolate->has_pending_exception());
   PostponeInterruptsScope postpone(isolate);
   bool has_script = shared->script()->IsScript();
-  // BUG(5946): This DCHECK is necessary to make certain that we won't tolerate
-  // the lack of a script without bytecode.
-  DCHECK_IMPLIES(!has_script, !ShouldUseFullCodegen(shared));
+  // BUG(5946): This DCHECK is necessary to make certain that we won't
+  // tolerate the lack of a script without bytecode.
+  DCHECK_IMPLIES(!has_script, shared->HasBytecodeArray());
   std::unique_ptr<CompilationJob> job(
       compiler::Pipeline::NewCompilationJob(function, has_script));
   CompilationInfo* info = job->info();
@@ -1127,9 +1110,6 @@ bool Compiler::Analyze(CompilationInfo* info,
 
 bool Compiler::ParseAndAnalyze(ParseInfo* info, Isolate* isolate) {
   if (!parsing::ParseAny(info, isolate)) return false;
-  if (info->is_toplevel()) {
-    EnsureSharedFunctionInfosArrayOnScript(info, isolate);
-  }
   if (!Compiler::Analyze(info, isolate)) return false;
   DCHECK_NOT_NULL(info->literal());
   DCHECK_NOT_NULL(info->scope());
@@ -1277,11 +1257,7 @@ bool Compiler::EnsureBytecode(CompilationInfo* info) {
     }
   }
   DCHECK(info->shared_info()->is_compiled());
-
   if (info->shared_info()->HasAsmWasmData()) return false;
-
-  DCHECK_EQ(ShouldUseFullCodegen(info),
-            !info->shared_info()->HasBytecodeArray());
   return info->shared_info()->HasBytecodeArray();
 }
 

src/full-codegen/full-codegen.cc

@@ -83,7 +83,7 @@ bool FullCodeGenerator::MakeCode(CompilationInfo* info) {
 bool FullCodeGenerator::MakeCode(CompilationInfo* info, uintptr_t stack_limit) {
   Isolate* isolate = info->isolate();
 
-  DCHECK(!info->shared_info()->must_use_ignition());
+  DCHECK(!info->literal()->must_use_ignition());
   DCHECK(!FLAG_minimal);
   RuntimeCallTimerScope runtimeTimer(isolate,
                                      &RuntimeCallStats::CompileFullCode);

src/objects/shared-function-info-inl.h

@@ -101,8 +101,6 @@ BIT_FIELD_ACCESSORS(SharedFunctionInfo, compiler_hints, native,
                     SharedFunctionInfo::IsNativeBit)
 BIT_FIELD_ACCESSORS(SharedFunctionInfo, compiler_hints, force_inline,
                     SharedFunctionInfo::ForceInlineBit)
-BIT_FIELD_ACCESSORS(SharedFunctionInfo, compiler_hints, must_use_ignition,
-                    SharedFunctionInfo::MustUseIgnitionBit)
 BIT_FIELD_ACCESSORS(SharedFunctionInfo, compiler_hints, is_asm_wasm_broken,
                     SharedFunctionInfo::IsAsmWasmBrokenBit)
 BIT_FIELD_ACCESSORS(SharedFunctionInfo, compiler_hints, optimization_disabled,

src/objects/shared-function-info.h

@@ -312,10 +312,6 @@ class SharedFunctionInfo : public HeapObject {
   // Indicate that this function should always be inlined in optimized code.
   DECL_BOOLEAN_ACCESSORS(force_inline)
 
-  // Indicates that code for this function must be compiled through the
-  // Ignition / TurboFan pipeline, and is unsupported by FullCodegen.
-  DECL_BOOLEAN_ACCESSORS(must_use_ignition)
-
   // Indicates that this function is an asm function.
   DECL_BOOLEAN_ACCESSORS(asm_function)
 
@@ -502,11 +498,10 @@ class SharedFunctionInfo : public HeapObject {
   V(NeedsHomeObjectBit, bool, 1, _)        \
   V(ForceInlineBit, bool, 1, _)            \
   V(IsAsmFunctionBit, bool, 1, _)          \
-  V(MustUseIgnitionBit, bool, 1, _)        \
   V(IsDeclarationBit, bool, 1, _)          \
   V(IsAsmWasmBrokenBit, bool, 1, _)        \
   V(FunctionMapIndexBits, int, 5, _)       \
-  /* Bits 27-31 are unused. */
+  /* Bits 26-31 are unused. */
 
   DEFINE_BIT_FIELDS(COMPILER_HINTS_BIT_FIELDS)
 #undef COMPILER_HINTS_BIT_FIELDS