[liftoff] Move more options into LiftoffOptions

{LiftoffOptions} already contains many (optional) parameters for Liftoff
compilation, but not all of them.
This CL moves the function index and the {for_debugging} field also into
that struct, to further reduce the number of parameters to
{ExecuteLiftoffCompilation} and to improve readability by having a
factory-like initialization of the {LiftoffOptions} struct.
That struct is now also passed down to the LiftoffCompiler directly
instead of unpacking the fields again.

R=thibaudm@chromium.org

Bug: v8:12809
Change-Id: I8824a1908f214cbf4c21f113934fef3ece1bf88b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3513894Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80032}

src/wasm/baseline/liftoff-compiler.cc

@@ -491,31 +491,30 @@ class LiftoffCompiler {
                   CompilationEnv* env, Zone* compilation_zone,
                   std::unique_ptr<AssemblerBuffer> buffer,
                   DebugSideTableBuilder* debug_sidetable_builder,
-                  ForDebugging for_debugging, int func_index,
-                  base::Vector<const int> breakpoints = {},
-                  int dead_breakpoint = 0, int32_t* max_steps = nullptr,
-                  int32_t* nondeterminism = nullptr)
+                  const LiftoffOptions& options)
       : asm_(std::move(buffer)),
         descriptor_(
             GetLoweredCallDescriptor(compilation_zone, call_descriptor)),
         env_(env),
         debug_sidetable_builder_(debug_sidetable_builder),
-        for_debugging_(for_debugging),
-        func_index_(func_index),
+        for_debugging_(options.for_debugging),
+        func_index_(options.func_index),
         out_of_line_code_(compilation_zone),
         source_position_table_builder_(compilation_zone),
         protected_instructions_(compilation_zone),
         compilation_zone_(compilation_zone),
         safepoint_table_builder_(compilation_zone_),
-        next_breakpoint_ptr_(breakpoints.begin()),
-        next_breakpoint_end_(breakpoints.end()),
-        dead_breakpoint_(dead_breakpoint),
+        next_breakpoint_ptr_(options.breakpoints.begin()),
+        next_breakpoint_end_(options.breakpoints.end()),
+        dead_breakpoint_(options.dead_breakpoint),
         handlers_(compilation_zone),
-        max_steps_(max_steps),
-        nondeterminism_(nondeterminism) {
-    if (breakpoints.empty()) {
-      next_breakpoint_ptr_ = next_breakpoint_end_ = nullptr;
-    }
+        max_steps_(options.max_steps),
+        nondeterminism_(options.nondeterminism) {
+    DCHECK(options.is_initialized());
+    // If there are no breakpoints, both pointers should be nullptr.
+    DCHECK_IMPLIES(
+        next_breakpoint_ptr_ == next_breakpoint_end_,
+        next_breakpoint_ptr_ == nullptr && next_breakpoint_end_ == nullptr);
   }
 
   bool did_bailout() const { return bailout_reason_ != kSuccess; }
@@ -6566,16 +6565,17 @@ constexpr base::EnumSet<ValueKind> LiftoffCompiler::kUnconditionallySupported;
 }  // namespace
 
 WasmCompilationResult ExecuteLiftoffCompilation(
-    CompilationEnv* env, const FunctionBody& func_body, int func_index,
-    ForDebugging for_debugging, const LiftoffOptions& compiler_options) {
+    CompilationEnv* env, const FunctionBody& func_body,
+    const LiftoffOptions& compiler_options) {
+  DCHECK(compiler_options.is_initialized());
   base::TimeTicks start_time;
   if (V8_UNLIKELY(FLAG_trace_wasm_compilation_times)) {
     start_time = base::TimeTicks::Now();
   }
   int func_body_size = static_cast<int>(func_body.end - func_body.start);
   TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("v8.wasm.detailed"),
-               "wasm.CompileBaseline", "funcIndex", func_index, "bodySize",
-               func_body_size);
+               "wasm.CompileBaseline", "funcIndex", compiler_options.func_index,
+               "bodySize", func_body_size);
 
   Zone zone(GetWasmEngine()->allocator(), "LiftoffCompilationZone");
   auto call_descriptor = compiler::GetWasmCallDescriptor(&zone, func_body.sig);
@@ -6590,7 +6590,8 @@ WasmCompilationResult ExecuteLiftoffCompilation(
   if (compiler_options.debug_sidetable) {
     debug_sidetable_builder = std::make_unique<DebugSideTableBuilder>();
   }
-  DCHECK_IMPLIES(compiler_options.max_steps, for_debugging == kForDebugging);
+  DCHECK_IMPLIES(compiler_options.max_steps,
+                 compiler_options.for_debugging == kForDebugging);
   WasmFeatures unused_detected_features;
   WasmFullDecoder<Decoder::kBooleanValidation, LiftoffCompiler> decoder(
       &zone, env->module, env->enabled_features,
@@ -6598,9 +6599,7 @@ WasmCompilationResult ExecuteLiftoffCompilation(
                                          : &unused_detected_features,
       func_body, call_descriptor, env, &zone,
       NewAssemblerBuffer(initial_buffer_size), debug_sidetable_builder.get(),
-      for_debugging, func_index, compiler_options.breakpoints,
-      compiler_options.dead_breakpoint, compiler_options.max_steps,
-      compiler_options.nondeterminism);
+      compiler_options);
   decoder.Decode();
   LiftoffCompiler* compiler = &decoder.interface();
   if (decoder.failed()) compiler->OnFirstError(&decoder);
@@ -6627,9 +6626,9 @@ WasmCompilationResult ExecuteLiftoffCompilation(
   result.frame_slot_count = compiler->GetTotalFrameSlotCountForGC();
   auto* lowered_call_desc = GetLoweredCallDescriptor(&zone, call_descriptor);
   result.tagged_parameter_slots = lowered_call_desc->GetTaggedParameterSlots();
-  result.func_index = func_index;
+  result.func_index = compiler_options.func_index;
   result.result_tier = ExecutionTier::kLiftoff;
-  result.for_debugging = for_debugging;
+  result.for_debugging = compiler_options.for_debugging;
   if (auto* debug_sidetable = compiler_options.debug_sidetable) {
     *debug_sidetable = debug_sidetable_builder->GenerateDebugSideTable();
   }
@@ -6640,7 +6639,7 @@ WasmCompilationResult ExecuteLiftoffCompilation(
     int codesize = result.code_desc.body_size();
     StdoutStream{} << "Compiled function "
                    << reinterpret_cast<const void*>(env->module) << "#"
-                   << func_index << " using Liftoff, took "
+                   << compiler_options.func_index << " using Liftoff, took "
                    << time.InMilliseconds() << " ms and "
                    << zone.allocation_size() << " bytes; bodysize "
                    << func_body_size << " codesize " << codesize << std::endl;
@@ -6676,8 +6675,11 @@ std::unique_ptr<DebugSideTable> GenerateLiftoffDebugSideTable(
       &zone, native_module->module(), env.enabled_features, &detected,
       func_body, call_descriptor, &env, &zone,
       NewAssemblerBuffer(AssemblerBase::kDefaultBufferSize),
-      &debug_sidetable_builder, code->for_debugging(), code->index(),
-      breakpoints);
+      &debug_sidetable_builder,
+      LiftoffOptions{}
+          .set_func_index(code->index())
+          .set_for_debugging(code->for_debugging())
+          .set_breakpoints(breakpoints));
   decoder.Decode();
   DCHECK(decoder.ok());
   DCHECK(!decoder.interface().did_bailout());

src/wasm/baseline/liftoff-compiler.h

@@ -54,6 +54,8 @@ enum LiftoffBailoutReason : int8_t {
 };
 
 struct LiftoffOptions {
+  int func_index = -1;
+  ForDebugging for_debugging = kNoDebugging;
   Counters* counters = nullptr;
   WasmFeatures* detected_features = nullptr;
   base::Vector<const int> breakpoints = {};
@@ -62,14 +64,18 @@ struct LiftoffOptions {
   int32_t* max_steps = nullptr;
   int32_t* nondeterminism = nullptr;
 
+  // Check that all non-optional fields have been initialized.
+  bool is_initialized() const { return func_index >= 0; }
+
   // We keep the macro as small as possible by offloading the actual DCHECK and
   // assignment to another function. This makes debugging easier.
 #define SETTER(field)                                               \
-  template <typename T>                             \
-  LiftoffOptions& set_##field(T new_value) {        \
-    return Set<decltype(field)>(&field, new_value); \
+  LiftoffOptions& set_##field(decltype(field) new_value) {          \
+    return Set<decltype(field)>(&LiftoffOptions::field, new_value); \
   }
 
+  SETTER(func_index)
+  SETTER(for_debugging)
   SETTER(counters)
   SETTER(detected_features)
   SETTER(breakpoints)
@@ -82,17 +88,16 @@ struct LiftoffOptions {
 
  private:
   template <typename T>
-  LiftoffOptions& Set(T* ptr, T new_value) {
-    // The field must still have its default value.
-    DCHECK_EQ(*ptr, T{});
-    *ptr = new_value;
+  LiftoffOptions& Set(T LiftoffOptions::*field_ptr, T new_value) {
+    // The field must still have its default value (set each field only once).
+    DCHECK_EQ(this->*field_ptr, LiftoffOptions{}.*field_ptr);
+    this->*field_ptr = new_value;
     return *this;
   }
 };
 
-V8_EXPORT_PRIVATE WasmCompilationResult
-ExecuteLiftoffCompilation(CompilationEnv*, const FunctionBody&, int func_index,
-                          ForDebugging, const LiftoffOptions& = {});
+V8_EXPORT_PRIVATE WasmCompilationResult ExecuteLiftoffCompilation(
+    CompilationEnv*, const FunctionBody&, const LiftoffOptions&);
 
 V8_EXPORT_PRIVATE std::unique_ptr<DebugSideTable> GenerateLiftoffDebugSideTable(
     const WasmCode*);

src/wasm/function-compiler.cc

@@ -120,8 +120,10 @@ WasmCompilationResult WasmCompilationUnit::ExecuteFunctionCompilation(
           debug_sidetable_ptr = &unused_debug_sidetable;
         }
         result = ExecuteLiftoffCompilation(
-            env, func_body, func_index_, for_debugging_,
+            env, func_body,
             LiftoffOptions{}
+                .set_func_index(func_index_)
+                .set_for_debugging(for_debugging_)
                 .set_counters(counters)
                 .set_detected_features(detected)
                 .set_debug_sidetable(debug_sidetable_ptr));

src/wasm/wasm-debug.cc

@@ -281,8 +281,10 @@ class DebugInfoImpl {
     // Debug side tables for stepping are generated lazily.
     bool generate_debug_sidetable = for_debugging == kWithBreakpoints;
     WasmCompilationResult result = ExecuteLiftoffCompilation(
-        &env, body, func_index, for_debugging,
+        &env, body,
         LiftoffOptions{}
+            .set_func_index(func_index)
+            .set_for_debugging(for_debugging)
             .set_breakpoints(offsets)
             .set_dead_breakpoint(dead_breakpoint)
             .set_debug_sidetable(generate_debug_sidetable ? &debug_sidetable

test/cctest/wasm/test-liftoff-inspection.cc

@@ -44,12 +44,16 @@ class LiftoffCompileEnvironment {
     CompilationEnv env = wasm_runner_.builder().CreateCompilationEnv();
     WasmFeatures detected1;
     WasmFeatures detected2;
-    WasmCompilationResult result1 = ExecuteLiftoffCompilation(
-        &env, test_func.body, test_func.code->index(), kNoDebugging,
-        LiftoffOptions{}.set_detected_features(&detected1));
-    WasmCompilationResult result2 = ExecuteLiftoffCompilation(
-        &env, test_func.body, test_func.code->index(), kNoDebugging,
-        LiftoffOptions{}.set_detected_features(&detected2));
+    WasmCompilationResult result1 =
+        ExecuteLiftoffCompilation(&env, test_func.body,
+                                  LiftoffOptions{}
+                                      .set_func_index(test_func.code->index())
+                                      .set_detected_features(&detected1));
+    WasmCompilationResult result2 =
+        ExecuteLiftoffCompilation(&env, test_func.body,
+                                  LiftoffOptions{}
+                                      .set_func_index(test_func.code->index())
+                                      .set_detected_features(&detected2));
 
     CHECK(result1.succeeded());
     CHECK(result2.succeeded());
@@ -73,8 +77,10 @@ class LiftoffCompileEnvironment {
     CompilationEnv env = wasm_runner_.builder().CreateCompilationEnv();
     std::unique_ptr<DebugSideTable> debug_side_table_via_compilation;
     auto result = ExecuteLiftoffCompilation(
-        &env, test_func.body, 0, kForDebugging,
+        &env, test_func.body,
         LiftoffOptions{}
+            .set_func_index(0)
+            .set_for_debugging(kForDebugging)
             .set_breakpoints(base::VectorOf(breakpoints))
             .set_debug_sidetable(&debug_side_table_via_compilation));
     CHECK(result.succeeded());

test/cctest/wasm/wasm-run-utils.cc

@@ -582,8 +582,10 @@ void WasmFunctionCompiler::Build(const byte* start, const byte* end) {
   if (builder_->test_execution_tier() ==
       TestExecutionTier::kLiftoffForFuzzing) {
     result.emplace(ExecuteLiftoffCompilation(
-        &env, func_body, function_->func_index, kForDebugging,
+        &env, func_body,
         LiftoffOptions{}
+            .set_func_index(function_->func_index)
+            .set_for_debugging(kForDebugging)
             .set_max_steps(builder_->max_steps_ptr())
             .set_nondeterminism(builder_->non_determinism_ptr())));
   } else {

test/fuzzer/wasm-fuzzer-common.cc

@@ -67,10 +67,13 @@ Handle<WasmModuleObject> CompileReferenceModule(Zone* zone, Isolate* isolate,
     base::Vector<const uint8_t> func_code = wire_bytes.GetFunctionBytes(&func);
     FunctionBody func_body(func.sig, func.code.offset(), func_code.begin(),
                            func_code.end());
-    auto result = ExecuteLiftoffCompilation(
-        &env, func_body, func.func_index, kForDebugging,
-        LiftoffOptions{}.set_max_steps(max_steps).set_nondeterminism(
-            nondeterminism));
+    auto result =
+        ExecuteLiftoffCompilation(&env, func_body,
+                                  LiftoffOptions{}
+                                      .set_func_index(func.func_index)
+                                      .set_for_debugging(kForDebugging)
+                                      .set_max_steps(max_steps)
+                                      .set_nondeterminism(nondeterminism));
     native_module->PublishCode(
         native_module->AddCompiledCode(std::move(result)));
   }