[liftoff] Refactor options for Liftoff compilation

The number of arguments for the LiftoffCompiler has grown significantly
since its initial implementation, and it becomes hard to keep track of
all options at the call sites.

This CL refactors all optional parameters into a {LiftoffOptions} struct
which has a factory-like interface.
This will allow us to add more options in the future, e.g. for dynamic
tiering.

R=thibaudm@chromium.org

Change-Id: I66697bb2f99b676a84c158304cc3a285e1b077d0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3069148
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76098}

src/compiler/wasm-compiler.cc

@@ -7915,6 +7915,8 @@ wasm::WasmCompilationResult ExecuteTurbofanWasmCompilation(
 
   std::vector<WasmLoopInfo> loop_infos;
 
+  wasm::WasmFeatures unused_detected_features;
+  if (!detected) detected = &unused_detected_features;
   if (!BuildGraphForWasmFunction(env, func_body, func_index, detected, mcgraph,
                                  &loop_infos, node_origins, source_positions)) {
     return wasm::WasmCompilationResult{};

src/wasm/baseline/liftoff-compiler.cc

@@ -6275,10 +6275,7 @@ constexpr base::EnumSet<ValueKind> LiftoffCompiler::kUnconditionallySupported;
 
 WasmCompilationResult ExecuteLiftoffCompilation(
     CompilationEnv* env, const FunctionBody& func_body, int func_index,
-    ForDebugging for_debugging, Counters* counters, WasmFeatures* detected,
-    base::Vector<const int> breakpoints,
-    std::unique_ptr<DebugSideTable>* debug_sidetable, int dead_breakpoint,
-    int32_t* max_steps, int32_t* nondeterminism) {
+    ForDebugging for_debugging, const LiftoffOptions& compiler_options) {
   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",
@@ -6294,20 +6291,25 @@ WasmCompilationResult ExecuteLiftoffCompilation(
   // have to grow more often.
   int initial_buffer_size = static_cast<int>(128 + code_size_estimate * 4 / 3);
   std::unique_ptr<DebugSideTableBuilder> debug_sidetable_builder;
-  if (debug_sidetable) {
+  if (compiler_options.debug_sidetable) {
     debug_sidetable_builder = std::make_unique<DebugSideTableBuilder>();
   }
-  DCHECK_IMPLIES(max_steps, for_debugging == kForDebugging);
+  DCHECK_IMPLIES(compiler_options.max_steps, for_debugging == kForDebugging);
+  WasmFeatures unused_detected_features;
   WasmFullDecoder<Decoder::kBooleanValidation, LiftoffCompiler> decoder(
-      &zone, env->module, env->enabled_features, detected, func_body,
-      call_descriptor, env, &zone, NewAssemblerBuffer(initial_buffer_size),
-      debug_sidetable_builder.get(), for_debugging, func_index, breakpoints,
-      dead_breakpoint, max_steps, nondeterminism);
+      &zone, env->module, env->enabled_features,
+      compiler_options.detected_features ? compiler_options.detected_features
+                                         : &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);
   decoder.Decode();
   LiftoffCompiler* compiler = &decoder.interface();
   if (decoder.failed()) compiler->OnFirstError(&decoder);
 
-  if (counters) {
+  if (auto* counters = compiler_options.counters) {
     // Check that the histogram for the bailout reasons has the correct size.
     DCHECK_EQ(0, counters->liftoff_bailout_reasons()->min());
     DCHECK_EQ(kNumBailoutReasons - 1,
@@ -6331,7 +6333,7 @@ WasmCompilationResult ExecuteLiftoffCompilation(
   result.func_index = func_index;
   result.result_tier = ExecutionTier::kLiftoff;
   result.for_debugging = for_debugging;
-  if (debug_sidetable) {
+  if (auto* debug_sidetable = compiler_options.debug_sidetable) {
     *debug_sidetable = debug_sidetable_builder->GenerateDebugSideTable();
   }
 

src/wasm/baseline/liftoff-compiler.h

@@ -53,12 +53,46 @@ enum LiftoffBailoutReason : int8_t {
   kNumBailoutReasons
 };
 
-V8_EXPORT_PRIVATE WasmCompilationResult ExecuteLiftoffCompilation(
-    CompilationEnv*, const FunctionBody&, int func_index, ForDebugging,
-    Counters*, WasmFeatures* detected_features,
-    base::Vector<const int> breakpoints = {},
-    std::unique_ptr<DebugSideTable>* = nullptr, int dead_breakpoint = 0,
-    int32_t* max_steps = nullptr, int32_t* nondeterminism = nullptr);
+struct LiftoffOptions {
+  Counters* counters = nullptr;
+  WasmFeatures* detected_features = nullptr;
+  base::Vector<const int> breakpoints = {};
+  std::unique_ptr<DebugSideTable>* debug_sidetable = nullptr;
+  int dead_breakpoint = 0;
+  int32_t* max_steps = nullptr;
+  int32_t* nondeterminism = nullptr;
+
+  // 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); \
+  }
+
+  SETTER(counters)
+  SETTER(detected_features)
+  SETTER(breakpoints)
+  SETTER(debug_sidetable)
+  SETTER(dead_breakpoint)
+  SETTER(max_steps)
+  SETTER(nondeterminism)
+
+#undef SETTER
+
+ 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;
+    return *this;
+  }
+};
+
+V8_EXPORT_PRIVATE WasmCompilationResult
+ExecuteLiftoffCompilation(CompilationEnv*, const FunctionBody&, int func_index,
+                          ForDebugging, const LiftoffOptions& = {});
 
 V8_EXPORT_PRIVATE std::unique_ptr<DebugSideTable> GenerateLiftoffDebugSideTable(
     const WasmCode*);

src/wasm/function-compiler.cc

@@ -107,18 +107,20 @@ WasmCompilationResult WasmCompilationUnit::ExecuteFunctionCompilation(
       if (V8_LIKELY(FLAG_wasm_tier_mask_for_testing == 0) ||
           func_index_ >= 32 ||
           ((FLAG_wasm_tier_mask_for_testing & (1 << func_index_)) == 0)) {
-        if (V8_LIKELY(func_index_ >= 32 || (FLAG_wasm_debug_mask_for_testing &
-                                            (1 << func_index_)) == 0)) {
-          result = ExecuteLiftoffCompilation(
-              env, func_body, func_index_, for_debugging_, counters, detected);
-        } else {
-          // We don't use the debug side table, we only pass it to cover
-          // different code paths in Liftoff for testing.
-          std::unique_ptr<DebugSideTable> debug_sidetable;
-          result = ExecuteLiftoffCompilation(env, func_body, func_index_,
-                                             kForDebugging, counters, detected,
-                                             {}, &debug_sidetable);
+        // We do not use the debug side table, we only (optionally) pass it to
+        // cover different code paths in Liftoff for testing.
+        std::unique_ptr<DebugSideTable> unused_debug_sidetable;
+        std::unique_ptr<DebugSideTable>* debug_sidetable_ptr = nullptr;
+        if (V8_UNLIKELY(func_index_ < 32 && (FLAG_wasm_debug_mask_for_testing &
+                                             (1 << func_index_)) != 0)) {
+          debug_sidetable_ptr = &unused_debug_sidetable;
         }
+        result = ExecuteLiftoffCompilation(
+            env, func_body, func_index_, for_debugging_,
+            LiftoffOptions{}
+                .set_counters(counters)
+                .set_detected_features(detected)
+                .set_debug_sidetable(debug_sidetable_ptr));
         if (result.succeeded()) break;
       }
 

src/wasm/wasm-debug.cc

@@ -280,12 +280,13 @@ class DebugInfoImpl {
 
     // Debug side tables for stepping are generated lazily.
     bool generate_debug_sidetable = for_debugging == kWithBreakpoints;
-    Counters* counters = nullptr;
-    WasmFeatures unused_detected;
     WasmCompilationResult result = ExecuteLiftoffCompilation(
-        &env, body, func_index, for_debugging, counters, &unused_detected,
-        offsets, generate_debug_sidetable ? &debug_sidetable : nullptr,
-        dead_breakpoint);
+        &env, body, func_index, for_debugging,
+        LiftoffOptions{}
+            .set_breakpoints(offsets)
+            .set_dead_breakpoint(dead_breakpoint)
+            .set_debug_sidetable(generate_debug_sidetable ? &debug_sidetable
+                                                          : nullptr));
     // Liftoff compilation failure is a FATAL error. We rely on complete Liftoff
     // support for debugging.
     if (!result.succeeded()) FATAL("Liftoff compilation failed");

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

@@ -46,10 +46,10 @@ class LiftoffCompileEnvironment {
     WasmFeatures detected2;
     WasmCompilationResult result1 = ExecuteLiftoffCompilation(
         &env, test_func.body, test_func.code->index(), kNoDebugging,
-        isolate_->counters(), &detected1);
+        LiftoffOptions{}.set_detected_features(&detected1));
     WasmCompilationResult result2 = ExecuteLiftoffCompilation(
         &env, test_func.body, test_func.code->index(), kNoDebugging,
-        isolate_->counters(), &detected2);
+        LiftoffOptions{}.set_detected_features(&detected2));
 
     CHECK(result1.succeeded());
     CHECK(result2.succeeded());
@@ -71,11 +71,12 @@ class LiftoffCompileEnvironment {
     auto test_func = AddFunction(return_types, param_types, raw_function_bytes);
 
     CompilationEnv env = wasm_runner_.builder().CreateCompilationEnv();
-    WasmFeatures detected;
     std::unique_ptr<DebugSideTable> debug_side_table_via_compilation;
     auto result = ExecuteLiftoffCompilation(
-        &env, test_func.body, 0, kForDebugging, nullptr, &detected,
-        base::VectorOf(breakpoints), &debug_side_table_via_compilation);
+        &env, test_func.body, 0, kForDebugging,
+        LiftoffOptions{}
+            .set_breakpoints(base::VectorOf(breakpoints))
+            .set_debug_sidetable(&debug_side_table_via_compilation));
     CHECK(result.succeeded());
 
     // If there are no breakpoint, then {ExecuteLiftoffCompilation} should

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

@@ -549,21 +549,20 @@ void WasmFunctionCompiler::Build(const byte* start, const byte* end) {
   ForDebugging for_debugging =
       native_module->IsTieredDown() ? kForDebugging : kNoDebugging;
 
-  WasmFeatures unused_detected_features;
-
   base::Optional<WasmCompilationResult> result;
   if (builder_->test_execution_tier() ==
       TestExecutionTier::kLiftoffForFuzzing) {
     result.emplace(ExecuteLiftoffCompilation(
         &env, func_body, function_->func_index, kForDebugging,
-        isolate()->counters(), &unused_detected_features, {}, nullptr, 0,
-        builder_->max_steps_ptr(), builder_->non_determinism_ptr()));
+        LiftoffOptions{}
+            .set_max_steps(builder_->max_steps_ptr())
+            .set_nondeterminism(builder_->non_determinism_ptr())));
   } else {
     WasmCompilationUnit unit(function_->func_index, builder_->execution_tier(),
                              for_debugging);
     result.emplace(unit.ExecuteCompilation(
         &env, native_module->compilation_state()->GetWireBytesStorage().get(),
-        isolate()->counters(), &unused_detected_features));
+        nullptr, nullptr));
   }
   WasmCode* code = native_module->PublishCode(
       native_module->AddCompiledCode(std::move(*result)));

test/fuzzer/wasm-fuzzer-common.cc

@@ -59,12 +59,12 @@ Handle<WasmModuleObject> CompileReferenceModule(Zone* zone, Isolate* isolate,
        ++i) {
     auto& func = module->functions[i];
     base::Vector<const uint8_t> func_code = wire_bytes.GetFunctionBytes(&func);
-    WasmFeatures unused_detected_features;
     FunctionBody func_body(func.sig, func.code.offset(), func_code.begin(),
                            func_code.end());
     auto result = ExecuteLiftoffCompilation(
-        &env, func_body, func.func_index, kForDebugging, isolate->counters(),
-        &unused_detected_features, {}, nullptr, 0, max_steps, nondeterminism);
+        &env, func_body, func.func_index, kForDebugging,
+        LiftoffOptions{}.set_max_steps(max_steps).set_nondeterminism(
+            nondeterminism));
     native_module->PublishCode(
         native_module->AddCompiledCode(std::move(result)));
   }