[sparkplug] CompileWithBaseline to Compiler::CompileBaseline

Move the CompileWithBaseline interface to the Compiler class, as
CompileBaseline, which will do the additional work of pre-compiling
to bytecode, ensuring there is a feedback vector, and setting the
code on the function closure.

As a drive-by, fix v8_enable_trace_unoptimized to have a blank default
value, so that v8_enable_trace_ignition/v8_enable_trace_baseline_exec
can set it.

Bug: v8:11420, v8:11429
Change-Id: If715161de71f7d9300f3fdcbb50cc678b1fcdfdf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2697352Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72819}

BUILD.gn

@@ -125,9 +125,9 @@ declare_args() {
   v8_enable_v8_checks = ""
 
   # Sets -dV8_TRACE_UNOPTIMIZED.
-  v8_enable_trace_unoptimized = false
+  v8_enable_trace_unoptimized = ""
   v8_enable_trace_ignition = false
-  v8_enable_trace_baseline = false
+  v8_enable_trace_baseline_exec = false
 
   # Sets -dV8_TRACE_FEEDBACK_UPDATES.
   v8_enable_trace_feedback_updates = false
@@ -358,11 +358,11 @@ assert(!v8_enable_concurrent_marking || v8_enable_atomic_marking_state,
        "Concurrent marking requires atomic marking state.")
 if (v8_enable_trace_unoptimized == "") {
   v8_enable_trace_unoptimized =
-      v8_enable_trace_ignition || v8_enable_trace_baseline
+      v8_enable_trace_ignition || v8_enable_trace_baseline_exec
 }
 assert(!v8_enable_trace_ignition || v8_enable_trace_unoptimized,
        "Ignition tracing requires unoptimized tracing to be enabled.")
-assert(!v8_enable_trace_baseline || v8_enable_trace_unoptimized,
+assert(!v8_enable_trace_baseline_exec || v8_enable_trace_unoptimized,
        "Baseline tracing requires unoptimized tracing to be enabled.")
 
 # Toggle pointer compression for correctness fuzzing when building the

src/baseline/baseline-compiler.cc

@@ -551,7 +551,7 @@ void BaselineCompiler::VerifyFrame() {
 
 #ifdef V8_TRACE_UNOPTIMIZED
 void BaselineCompiler::TraceBytecode(Runtime::FunctionId function_id) {
-  if (!FLAG_trace_baseline) return;
+  if (!FLAG_trace_baseline_exec) return;
 
   __ RecordComment(function_id == Runtime::kTraceUnoptimizedBytecodeEntry
                        ? "[ Trace bytecode entry"

src/baseline/baseline.cc

@@ -17,62 +17,18 @@
 namespace v8 {
 namespace internal {
 
-Handle<Code> CompileWithBaseline(
-    Isolate* isolate, Handle<SharedFunctionInfo> shared_function_info,
-    Handle<BytecodeArray> bytecode) {
+Handle<Code> GenerateBaselineCode(Isolate* isolate,
+                                  Handle<SharedFunctionInfo> shared) {
   RuntimeCallTimerScope runtimeTimer(isolate,
                                      RuntimeCallCounterId::kCompileBaseline);
-  baseline::BaselineCompiler compiler(isolate, shared_function_info, bytecode);
-
-  compiler.GenerateCode();
-
-  return compiler.Build(isolate);
-}
-
-// TODO(v8:11429): This can be the basis of Compiler::CompileBaseline
-Handle<Code> CompileWithBaseline(Isolate* isolate,
-                                 Handle<SharedFunctionInfo> shared) {
-  if (shared->HasBaselineData()) {
-    return handle(shared->baseline_data().baseline_code(), isolate);
-  }
-
-  if (FLAG_trace_opt) {
-    PrintF("[compiling method ");
-    shared->ShortPrint();
-    PrintF(" using Baseline]\n");
-  }
-
-  base::ElapsedTimer timer;
-  timer.Start();
-
-  Handle<Code> code = CompileWithBaseline(
+  baseline::BaselineCompiler compiler(
       isolate, shared, handle(shared->GetBytecodeArray(isolate), isolate));
-  Handle<HeapObject> function_data =
-      handle(HeapObject::cast(shared->function_data(kAcquireLoad)), isolate);
-  Handle<BaselineData> baseline_data =
-      isolate->factory()->NewBaselineData(code, function_data);
-  shared->set_baseline_data(*baseline_data);
 
+  compiler.GenerateCode();
+  Handle<Code> code = compiler.Build(isolate);
   if (FLAG_print_code) {
     code->Print();
   }
-
-  if (shared->script().IsScript()) {
-    Compiler::LogFunctionCompilation(
-        isolate, CodeEventListener::FUNCTION_TAG, shared,
-        handle(Script::cast(shared->script()), isolate),
-        Handle<AbstractCode>::cast(code), CodeKind::BASELINE,
-        timer.Elapsed().InMillisecondsF());
-  }
-
-  if (FLAG_trace_opt) {
-    // TODO(v8:11429): Move to Compiler.
-    PrintF("[completed compiling ");
-    shared->ShortPrint();
-    PrintF(" using Sparkplug - took %0.3f ms]\n",
-           timer.Elapsed().InMillisecondsF());
-  }
-
   return code;
 }
 
@@ -84,7 +40,7 @@ Handle<Code> CompileWithBaseline(Isolate* isolate,
 namespace v8 {
 namespace internal {
 
-Handle<Code> CompileWithBaseline(Isolate* isolate,
+Handle<Code> GenerateBaselineCode(Isolate* isolate,
                                   Handle<SharedFunctionInfo> shared) {
   UNREACHABLE();
 }

src/baseline/baseline.h

@@ -15,7 +15,7 @@ class SharedFunctionInfo;
 class BytecodeArray;
 
 // TODO(v8:11429): Restrict header visibility to just this file.
-Handle<Code> CompileWithBaseline(Isolate* isolate,
+Handle<Code> GenerateBaselineCode(Isolate* isolate,
                                   Handle<SharedFunctionInfo> shared);
 
 }  // namespace internal

src/codegen/compiler.cc

@@ -13,6 +13,7 @@
 #include "src/ast/scopes.h"
 #include "src/base/logging.h"
 #include "src/base/optional.h"
+#include "src/base/platform/time.h"
 #include "src/baseline/baseline.h"
 #include "src/codegen/assembler-inl.h"
 #include "src/codegen/compilation-cache.h"
@@ -65,24 +66,6 @@ namespace {
 
 class CompilerTracer : public AllStatic {
  public:
-  static void PrintTracePrefix(const CodeTracer::Scope& scope,
-                               const char* header,
-                               OptimizedCompilationInfo* info) {
-    PrintTracePrefix(scope, header, info->closure(), info->code_kind());
-  }
-
-  static void PrintTracePrefix(const CodeTracer::Scope& scope,
-                               const char* header, Handle<JSFunction> function,
-                               CodeKind code_kind) {
-    PrintF(scope.file(), "[%s ", header);
-    function->ShortPrint(scope.file());
-    PrintF(scope.file(), " (target %s)", CodeKindToString(code_kind));
-  }
-
-  static void PrintTraceSuffix(const CodeTracer::Scope& scope) {
-    PrintF(scope.file(), "]\n");
-  }
-
   static void TracePrepareJob(Isolate* isolate, OptimizedCompilationInfo* info,
                               const char* compiler_name) {
     if (!FLAG_trace_opt || !info->IsOptimizing()) return;
@@ -93,6 +76,15 @@ class CompilerTracer : public AllStatic {
     PrintTraceSuffix(scope);
   }
 
+  static void TraceStartBaselineCompile(Isolate* isolate,
+                                        Handle<SharedFunctionInfo> shared) {
+    if (!FLAG_trace_baseline) return;
+    CodeTracer::Scope scope(isolate->GetCodeTracer());
+    PrintTracePrefix(scope, "compiling method", shared, CodeKind::BASELINE);
+    PrintF(scope.file(), " using Sparkplug");
+    PrintTraceSuffix(scope);
+  }
+
   static void TraceCompilationStats(Isolate* isolate,
                                     OptimizedCompilationInfo* info,
                                     double ms_creategraph, double ms_optimize,
@@ -105,6 +97,16 @@ class CompilerTracer : public AllStatic {
     PrintTraceSuffix(scope);
   }
 
+  static void TraceFinishBaselineCompile(Isolate* isolate,
+                                         Handle<SharedFunctionInfo> shared,
+                                         double ms_timetaken) {
+    if (!FLAG_trace_baseline) return;
+    CodeTracer::Scope scope(isolate->GetCodeTracer());
+    PrintTracePrefix(scope, "compiling", shared, CodeKind::BASELINE);
+    PrintF(scope.file(), " - took %0.3f ms", ms_timetaken);
+    PrintTraceSuffix(scope);
+  }
+
   static void TraceCompletedJob(Isolate* isolate,
                                 OptimizedCompilationInfo* info) {
     if (!FLAG_trace_opt) return;
@@ -155,6 +157,34 @@ class CompilerTracer : public AllStatic {
     PrintF(scope.file(), " for optimized recompilation because --always-opt");
     PrintF(scope.file(), "]\n");
   }
+
+ private:
+  static void PrintTracePrefix(const CodeTracer::Scope& scope,
+                               const char* header,
+                               OptimizedCompilationInfo* info) {
+    PrintTracePrefix(scope, header, info->closure(), info->code_kind());
+  }
+
+  static void PrintTracePrefix(const CodeTracer::Scope& scope,
+                               const char* header, Handle<JSFunction> function,
+                               CodeKind code_kind) {
+    PrintF(scope.file(), "[%s ", header);
+    function->ShortPrint(scope.file());
+    PrintF(scope.file(), " (target %s)", CodeKindToString(code_kind));
+  }
+
+  static void PrintTracePrefix(const CodeTracer::Scope& scope,
+                               const char* header,
+                               Handle<SharedFunctionInfo> shared,
+                               CodeKind code_kind) {
+    PrintF(scope.file(), "[%s ", header);
+    shared->ShortPrint(scope.file());
+    PrintF(scope.file(), " (target %s)", CodeKindToString(code_kind));
+  }
+
+  static void PrintTraceSuffix(const CodeTracer::Scope& scope) {
+    PrintF(scope.file(), "]\n");
+  }
 };
 
 }  // namespace
@@ -606,6 +636,58 @@ void UpdateSharedFunctionFlagsAfterCompilation(FunctionLiteral* literal,
   shared_info.SetScopeInfo(*literal->scope()->scope_info());
 }
 
+bool CompileSharedWithBaseline(Isolate* isolate,
+                               Handle<SharedFunctionInfo> shared,
+                               Compiler::ClearExceptionFlag flag,
+                               IsCompiledScope* is_compiled_scope) {
+  DCHECK(FLAG_sparkplug);
+  DCHECK(is_compiled_scope->is_compiled());
+
+  if (shared->HasBaselineData()) return true;
+
+  StackLimitCheck check(isolate);
+  if (check.JsHasOverflowed(kStackSpaceRequiredForCompilation * KB)) {
+    if (flag == Compiler::KEEP_EXCEPTION) {
+      isolate->StackOverflow();
+    }
+    return false;
+  }
+
+  // Check if we actually have bytecode.
+  if (!shared->HasBytecodeArray()) return false;
+
+  // Do not optimize when debugger needs to hook into every call.
+  if (isolate->debug()->needs_check_on_function_call()) return false;
+
+  // Functions with breakpoints have to stay interpreted.
+  if (shared->HasBreakInfo()) return false;
+
+  CompilerTracer::TraceStartBaselineCompile(isolate, shared);
+  Handle<Code> code;
+  base::TimeDelta time_taken;
+  {
+    ScopedTimer timer(&time_taken);
+    code = GenerateBaselineCode(isolate, shared);
+
+    Handle<HeapObject> function_data =
+        handle(HeapObject::cast(shared->function_data(kAcquireLoad)), isolate);
+    Handle<BaselineData> baseline_data =
+        isolate->factory()->NewBaselineData(code, function_data);
+    shared->set_baseline_data(*baseline_data);
+  }
+  double time_taken_ms = time_taken.InMillisecondsF();
+
+  CompilerTracer::TraceFinishBaselineCompile(isolate, shared, time_taken_ms);
+
+  if (shared->script().IsScript()) {
+    Compiler::LogFunctionCompilation(
+        isolate, CodeEventListener::FUNCTION_TAG, shared,
+        handle(Script::cast(shared->script()), isolate),
+        Handle<AbstractCode>::cast(code), CodeKind::BASELINE, time_taken_ms);
+  }
+  return true;
+}
+
 // Finalize a single compilation job. This function can return
 // RETRY_ON_MAIN_THREAD if the job cannot be finalized off-thread, in which case
 // it should be safe to call it again on the main thread with the same job.
@@ -1237,10 +1319,9 @@ void FinalizeUnoptimizedCompilation(
     if (FLAG_interpreted_frames_native_stack) {
       InstallInterpreterTrampolineCopy(isolate, shared_info);
     }
-    if (FLAG_always_sparkplug && shared_info->HasBytecodeArray() &&
-        !shared_info->HasBreakInfo()) {
-      // TODO(v8:11429) Extract to Compiler::CompileX
-      CompileWithBaseline(isolate, shared_info);
+    if (FLAG_always_sparkplug) {
+      CompileSharedWithBaseline(isolate, shared_info, Compiler::KEEP_EXCEPTION,
+                                &is_compiled_scope);
     }
     Handle<CoverageInfo> coverage_info;
     if (finalize_data.coverage_info().ToHandle(&coverage_info)) {
@@ -1888,6 +1969,25 @@ bool Compiler::Compile(Isolate* isolate, Handle<JSFunction> function,
   return true;
 }
 
+// static
+bool Compiler::CompileBaseline(Isolate* isolate, Handle<JSFunction> function,
+                               ClearExceptionFlag flag,
+                               IsCompiledScope* is_compiled_scope) {
+  Handle<SharedFunctionInfo> shared(function->shared(isolate), isolate);
+  if (!CompileSharedWithBaseline(isolate, shared, flag, is_compiled_scope)) {
+    return false;
+  }
+
+  // Baseline code needs a feedback vector.
+  JSFunction::EnsureFeedbackVector(function, is_compiled_scope);
+
+  Code baseline_code = shared->baseline_data().baseline_code(isolate);
+  DCHECK_EQ(baseline_code.kind(), CodeKind::BASELINE);
+  function->set_code(baseline_code);
+
+  return true;
+}
+
 // static
 MaybeHandle<SharedFunctionInfo> Compiler::CompileToplevel(
     ParseInfo* parse_info, Handle<Script> script, Isolate* isolate,

src/codegen/compiler.h

@@ -71,6 +71,9 @@ class V8_EXPORT_PRIVATE Compiler : public AllStatic {
   static bool Compile(Isolate* isolate, Handle<JSFunction> function,
                       ClearExceptionFlag flag,
                       IsCompiledScope* is_compiled_scope);
+  static bool CompileBaseline(Isolate* isolate, Handle<JSFunction> function,
+                              ClearExceptionFlag flag,
+                              IsCompiledScope* is_compiled_scope);
   static bool CompileOptimized(Isolate* isolate, Handle<JSFunction> function,
                                ConcurrencyMode mode, CodeKind code_kind);
   static MaybeHandle<SharedFunctionInfo> CompileToplevel(

src/flags/flag-definitions.h

@@ -538,10 +538,10 @@ DEFINE_BOOL(trace_unoptimized, false,
             "trace the bytecodes executed by all unoptimized execution")
 DEFINE_BOOL(trace_ignition, false,
             "trace the bytecodes executed by the ignition interpreter")
-DEFINE_BOOL(trace_baseline, false,
+DEFINE_BOOL(trace_baseline_exec, false,
             "trace the bytecodes executed by the baseline code")
 DEFINE_WEAK_IMPLICATION(trace_unoptimized, trace_ignition)
-DEFINE_WEAK_IMPLICATION(trace_unoptimized, trace_baseline)
+DEFINE_WEAK_IMPLICATION(trace_unoptimized, trace_baseline_exec)
 #endif
 #ifdef V8_TRACE_FEEDBACK_UPDATES
 DEFINE_BOOL(
@@ -584,6 +584,7 @@ DEFINE_INT(ticks_scale_factor_for_top_tier, 10,
 // Flags for Sparkplug
 DEFINE_BOOL(sparkplug, false, "enable experimental sparkplug baseline compiler")
 DEFINE_BOOL(always_sparkplug, false, "directly tier up to sparkplug")
+DEFINE_BOOL(trace_baseline, false, "trace baseline compilation")
 DEFINE_NEG_IMPLICATION(sparkplug, write_protect_code_memory)
 DEFINE_IMPLICATION(always_sparkplug, sparkplug)
 

src/runtime/runtime-compiler.cc

@@ -433,40 +433,6 @@ RUNTIME_FUNCTION(Runtime_CompileForOnStackReplacement) {
   return Object();
 }
 
-RUNTIME_FUNCTION(Runtime_CompileBaseline) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
-
-  Handle<SharedFunctionInfo> shared(function->shared(isolate), isolate);
-  IsCompiledScope is_compiled_scope = shared->is_compiled_scope(isolate);
-
-  StackLimitCheck check(isolate);
-  if (check.JsHasOverflowed(kStackSpaceRequiredForCompilation * KB)) {
-    return isolate->StackOverflow();
-  }
-  if (!shared->IsUserJavaScript()) {
-    return *function;
-  }
-  if (!is_compiled_scope.is_compiled()) {
-    if (!Compiler::Compile(isolate, function, Compiler::KEEP_EXCEPTION,
-                           &is_compiled_scope)) {
-      return ReadOnlyRoots(isolate).exception();
-    }
-  }
-
-  // TODO(v8:11429): Add a Compiler::Compile* method for this.
-  JSFunction::EnsureFeedbackVector(function, &is_compiled_scope);
-
-  if (!shared->HasBaselineData()) {
-    Handle<Code> code = CompileWithBaseline(isolate, shared);
-    function->set_code(*code);
-  } else {
-    function->set_code(shared->baseline_data().baseline_code(isolate));
-  }
-  return *function;
-}
-
 static Object CompileGlobalEval(Isolate* isolate,
                                 Handle<i::Object> source_object,
                                 Handle<SharedFunctionInfo> outer_info,

src/runtime/runtime-internal.cc

@@ -336,12 +336,10 @@ RUNTIME_FUNCTION(Runtime_BytecodeBudgetInterruptFromBytecode) {
     IsCompiledScope is_compiled_scope(
         function->shared().is_compiled_scope(isolate));
     JSFunction::EnsureFeedbackVector(function, &is_compiled_scope);
-    if (FLAG_sparkplug && !function->shared().HasBaselineData() &&
-        !function->shared().HasBreakInfo()) {
-      // TODO(v8:11429): Expose via Compiler, do set_code there.
-      Handle<SharedFunctionInfo> shared(function->shared(), isolate);
-      Handle<Code> code = CompileWithBaseline(isolate, shared);
-      function->set_code(*code);
+    DCHECK(is_compiled_scope.is_compiled());
+    if (FLAG_sparkplug) {
+      Compiler::CompileBaseline(isolate, function, Compiler::CLEAR_EXCEPTION,
+                                &is_compiled_scope);
     }
     // Also initialize the invocation count here. This is only really needed for
     // OSR. When we OSR functions with lazy feedback allocation we want to have

src/runtime/runtime-test.cc

@@ -385,6 +385,33 @@ bool EnsureFeedbackVector(Isolate* isolate, Handle<JSFunction> function) {
 
 }  // namespace
 
+RUNTIME_FUNCTION(Runtime_CompileBaseline) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(1, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
+
+  IsCompiledScope is_compiled_scope =
+      function->shared(isolate).is_compiled_scope(isolate);
+
+  if (!function->shared(isolate).IsUserJavaScript()) {
+    return CrashUnlessFuzzing(isolate);
+  }
+
+  // First compile the bytecode, if we have to.
+  if (!is_compiled_scope.is_compiled() &&
+      !Compiler::Compile(isolate, function, Compiler::KEEP_EXCEPTION,
+                         &is_compiled_scope)) {
+    return CrashUnlessFuzzing(isolate);
+  }
+
+  if (!Compiler::CompileBaseline(isolate, function, Compiler::KEEP_EXCEPTION,
+                                 &is_compiled_scope)) {
+    return CrashUnlessFuzzing(isolate);
+  }
+
+  return *function;
+}
+
 RUNTIME_FUNCTION(Runtime_OptimizeFunctionOnNextCall) {
   HandleScope scope(isolate);
   return OptimizeFunctionOnNextCall(args, isolate, TierupKind::kTierupBytecode);

src/runtime/runtime-trace.cc

@@ -97,7 +97,7 @@ void PrintRegisters(UnoptimizedFrame* frame, std::ostream& os, bool is_input,
 }  // namespace
 
 RUNTIME_FUNCTION(Runtime_TraceUnoptimizedBytecodeEntry) {
-  if (!FLAG_trace_ignition && !FLAG_trace_baseline) {
+  if (!FLAG_trace_ignition && !FLAG_trace_baseline_exec) {
     return ReadOnlyRoots(isolate).undefined_value();
   }
 
@@ -108,7 +108,7 @@ RUNTIME_FUNCTION(Runtime_TraceUnoptimizedBytecodeEntry) {
   if (frame->is_interpreted() && !FLAG_trace_ignition) {
     return ReadOnlyRoots(isolate).undefined_value();
   }
-  if (frame->is_baseline() && !FLAG_trace_baseline) {
+  if (frame->is_baseline() && !FLAG_trace_baseline_exec) {
     return ReadOnlyRoots(isolate).undefined_value();
   }
 
@@ -148,7 +148,7 @@ RUNTIME_FUNCTION(Runtime_TraceUnoptimizedBytecodeEntry) {
 }
 
 RUNTIME_FUNCTION(Runtime_TraceUnoptimizedBytecodeExit) {
-  if (!FLAG_trace_ignition && !FLAG_trace_baseline) {
+  if (!FLAG_trace_ignition && !FLAG_trace_baseline_exec) {
     return ReadOnlyRoots(isolate).undefined_value();
   }
 
@@ -159,7 +159,7 @@ RUNTIME_FUNCTION(Runtime_TraceUnoptimizedBytecodeExit) {
   if (frame->is_interpreted() && !FLAG_trace_ignition) {
     return ReadOnlyRoots(isolate).undefined_value();
   }
-  if (frame->is_baseline() && !FLAG_trace_baseline) {
+  if (frame->is_baseline() && !FLAG_trace_baseline_exec) {
     return ReadOnlyRoots(isolate).undefined_value();
   }
 

test/mjsunit/baseline/cross-realm.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --sparkplug
 
 // Tier-up across Realms
 

test/mjsunit/baseline/test-baseline-module-helper.mjs

@@ -2,6 +2,4 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --super-ic
-
 export default 12;
\ No newline at end of file

test/mjsunit/baseline/test-baseline-module.mjs

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --super-ic
+// Flags: --allow-natives-syntax --super-ic --sparkplug
 
 export let exported = 17;
 import imported from 'test-baseline-module-helper.mjs';

test/mjsunit/baseline/test-baseline.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --super-ic
+// Flags: --allow-natives-syntax --super-ic --sparkplug
 
 function run(f, ...args) {
   try { f(...args); } catch (e) {}