Commit c582eb4e authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

[wasm] Use a single source of truth for feedback vector size

The number of feedback vector slots is currently stored in the
{WasmFunction}, returned in the {WasmCompilationResult}, and implicitly
stored as the size of the {call_targets} vector in
{FunctionTypeFeedback}.

This CL uses the latter as the source of truth, encapsulated in a new
{NumFeedbackSlots} function. This can be updated when adding new kinds
of feedback that need additional slots.
For now, the implementation of {NumFeedbackSlots} requires taking a
mutex, which we can hopefully avoid when productionizing speculative
inlining. We also take the mutex on every Liftoff compilation, which
adds synchronization between concurrent compilation which we previously
tried very hard to avoid (because it introduced significant overhead for
eager compilation).

As a nice side-effect, this CL reduces the per-function overhead by 8
bytes, independent of enabled features.

R=jkummerow@chromium.org

Bug: v8:13209
Change-Id: I2fe5f7fe73154328032a3f0961e88d068c5d07ae
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3899299Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83253}
parent 353d1009
...@@ -556,12 +556,6 @@ class LiftoffCompiler { ...@@ -556,12 +556,6 @@ class LiftoffCompiler {
return __ GetTotalFrameSlotCountForGC(); return __ GetTotalFrameSlotCountForGC();
} }
int GetFeedbackVectorSlots() const {
// The number of call instructions is capped by max function size.
static_assert(kV8MaxWasmFunctionSize < std::numeric_limits<int>::max() / 2);
return static_cast<int>(encountered_call_instructions_.size()) * 2;
}
void unsupported(FullDecoder* decoder, LiftoffBailoutReason reason, void unsupported(FullDecoder* decoder, LiftoffBailoutReason reason,
const char* detail) { const char* detail) {
DCHECK_NE(kSuccess, reason); DCHECK_NE(kSuccess, reason);
...@@ -7709,7 +7703,6 @@ WasmCompilationResult ExecuteLiftoffCompilation( ...@@ -7709,7 +7703,6 @@ WasmCompilationResult ExecuteLiftoffCompilation(
if (auto* debug_sidetable = compiler_options.debug_sidetable) { if (auto* debug_sidetable = compiler_options.debug_sidetable) {
*debug_sidetable = debug_sidetable_builder->GenerateDebugSideTable(); *debug_sidetable = debug_sidetable_builder->GenerateDebugSideTable();
} }
result.feedback_vector_slots = compiler->GetFeedbackVectorSlots();
if (V8_UNLIKELY(v8_flags.trace_wasm_compilation_times)) { if (V8_UNLIKELY(v8_flags.trace_wasm_compilation_times)) {
base::TimeDelta time = base::TimeTicks::Now() - start_time; base::TimeDelta time = base::TimeTicks::Now() - start_time;
......
...@@ -57,7 +57,6 @@ struct WasmCompilationResult { ...@@ -57,7 +57,6 @@ struct WasmCompilationResult {
ExecutionTier result_tier; ExecutionTier result_tier;
Kind kind = kFunction; Kind kind = kFunction;
ForDebugging for_debugging = kNoDebugging; ForDebugging for_debugging = kNoDebugging;
int feedback_vector_slots = 0;
}; };
class V8_EXPORT_PRIVATE WasmCompilationUnit final { class V8_EXPORT_PRIVATE WasmCompilationUnit final {
......
...@@ -1247,13 +1247,14 @@ bool CompileLazy(Isolate* isolate, Handle<WasmInstanceObject> instance, ...@@ -1247,13 +1247,14 @@ bool CompileLazy(Isolate* isolate, Handle<WasmInstanceObject> instance,
} }
// Allocate feedback vector if needed. // Allocate feedback vector if needed.
if (result.feedback_vector_slots > 0) { int feedback_vector_slots = NumFeedbackSlots(module, func_index);
if (feedback_vector_slots > 0) {
DCHECK(v8_flags.wasm_speculative_inlining); DCHECK(v8_flags.wasm_speculative_inlining);
// We have to save the native_module on the stack, in case the allocation // We have to save the native_module on the stack, in case the allocation
// triggers a GC and we need the module to scan WasmCompileLazy stack frame. // triggers a GC and we need the module to scan WasmCompileLazy stack frame.
*out_native_module = native_module; *out_native_module = native_module;
Handle<FixedArray> vector = isolate->factory()->NewFixedArrayWithZeroes( Handle<FixedArray> vector =
result.feedback_vector_slots); isolate->factory()->NewFixedArrayWithZeroes(feedback_vector_slots);
instance->feedback_vectors().set( instance->feedback_vectors().set(
declared_function_index(module, func_index), *vector); declared_function_index(module, func_index), *vector);
} }
......
...@@ -787,7 +787,6 @@ class ModuleDecoderTemplate : public Decoder { ...@@ -787,7 +787,6 @@ class ModuleDecoderTemplate : public Decoder {
import->index, // func_index import->index, // func_index
0, // sig_index 0, // sig_index
{0, 0}, // code {0, 0}, // code
0, // feedback slots
true, // imported true, // imported
false, // exported false, // exported
false}); // declared false}); // declared
...@@ -884,7 +883,6 @@ class ModuleDecoderTemplate : public Decoder { ...@@ -884,7 +883,6 @@ class ModuleDecoderTemplate : public Decoder {
func_index, // func_index func_index, // func_index
0, // sig_index 0, // sig_index
{0, 0}, // code {0, 0}, // code
0, // feedback slots
false, // imported false, // imported
false, // exported false, // exported
false}); // declared false}); // declared
......
...@@ -744,8 +744,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() { ...@@ -744,8 +744,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
instance->set_feedback_vectors(*vectors); instance->set_feedback_vectors(*vectors);
for (int i = 0; i < num_functions; i++) { for (int i = 0; i < num_functions; i++) {
int func_index = module_->num_imported_functions + i; int func_index = module_->num_imported_functions + i;
int slots = int slots = NumFeedbackSlots(module_, func_index);
base::Relaxed_Load(&module_->functions[func_index].feedback_slots);
if (slots == 0) continue; if (slots == 0) continue;
if (v8_flags.trace_wasm_speculative_inlining) { if (v8_flags.trace_wasm_speculative_inlining) {
PrintF("[Function %d (declared %d): allocating %d feedback slots]\n", PrintF("[Function %d (declared %d): allocating %d feedback slots]\n",
......
...@@ -2336,16 +2336,6 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode( ...@@ -2336,16 +2336,6 @@ std::vector<std::unique_ptr<WasmCode>> NativeModule::AddCompiledCode(
for (auto& result : results) { for (auto& result : results) {
DCHECK(result.succeeded()); DCHECK(result.succeeded());
total_code_space += RoundUp<kCodeAlignment>(result.code_desc.instr_size); total_code_space += RoundUp<kCodeAlignment>(result.code_desc.instr_size);
if (result.result_tier == ExecutionTier::kLiftoff) {
int index = result.func_index;
int* slots = &module()->functions[index].feedback_slots;
#if DEBUG
int current_value = base::Relaxed_Load(slots);
DCHECK(current_value == 0 ||
current_value == result.feedback_vector_slots);
#endif
base::Relaxed_Store(slots, result.feedback_vector_slots);
}
} }
base::Vector<byte> code_space; base::Vector<byte> code_space;
NativeModule::JumpTablesRef jump_tables; NativeModule::JumpTablesRef jump_tables;
......
...@@ -678,4 +678,16 @@ size_t GetWireBytesHash(base::Vector<const uint8_t> wire_bytes) { ...@@ -678,4 +678,16 @@ size_t GetWireBytesHash(base::Vector<const uint8_t> wire_bytes) {
kZeroHashSeed); kZeroHashSeed);
} }
int NumFeedbackSlots(const WasmModule* module, int func_index) {
if (!v8_flags.wasm_speculative_inlining) return 0;
// TODO(clemensb): Avoid the mutex once this ships, or at least switch to a
// shared mutex.
base::MutexGuard type_feedback_guard{&module->type_feedback.mutex};
auto it = module->type_feedback.feedback_for_function.find(func_index);
if (it == module->type_feedback.feedback_for_function.end()) return 0;
// The number of call instructions is capped by max function size.
static_assert(kV8MaxWasmFunctionSize < std::numeric_limits<int>::max() / 2);
return static_cast<int>(2 * it->second.call_targets.size());
}
} // namespace v8::internal::wasm } // namespace v8::internal::wasm
...@@ -63,10 +63,6 @@ struct WasmFunction { ...@@ -63,10 +63,6 @@ struct WasmFunction {
uint32_t func_index; // index into the function table. uint32_t func_index; // index into the function table.
uint32_t sig_index; // index into the signature table. uint32_t sig_index; // index into the signature table.
WireBytesRef code; // code of this function. WireBytesRef code; // code of this function.
// Required number of slots in a feedback vector. Marked {mutable} because
// this is computed late (by Liftoff compilation), when the rest of the
// {WasmFunction} is typically considered {const}.
mutable int feedback_slots;
bool imported; bool imported;
bool exported; bool exported;
bool declared; bool declared;
...@@ -783,6 +779,9 @@ size_t PrintSignature(base::Vector<char> buffer, const wasm::FunctionSig*, ...@@ -783,6 +779,9 @@ size_t PrintSignature(base::Vector<char> buffer, const wasm::FunctionSig*,
V8_EXPORT_PRIVATE size_t V8_EXPORT_PRIVATE size_t
GetWireBytesHash(base::Vector<const uint8_t> wire_bytes); GetWireBytesHash(base::Vector<const uint8_t> wire_bytes);
// Get the required number of feedback slots for a function.
int NumFeedbackSlots(const WasmModule* module, int func_index);
} // namespace v8::internal::wasm } // namespace v8::internal::wasm
#endif // V8_WASM_WASM_MODULE_H_ #endif // V8_WASM_WASM_MODULE_H_
...@@ -158,7 +158,6 @@ uint32_t TestingModuleBuilder::AddFunction(const FunctionSig* sig, ...@@ -158,7 +158,6 @@ uint32_t TestingModuleBuilder::AddFunction(const FunctionSig* sig,
index, // func_index index, // func_index
0, // sig_index 0, // sig_index
{0, 0}, // code {0, 0}, // code
0, // feedback slots
false, // imported false, // imported
false, // exported false, // exported
false}); // declared false}); // declared
......
...@@ -4240,7 +4240,6 @@ ControlTransferMap WasmInterpreter::ComputeControlTransfersForTesting( ...@@ -4240,7 +4240,6 @@ ControlTransferMap WasmInterpreter::ComputeControlTransfersForTesting(
0, // func_index 0, // func_index
0, // sig_index 0, // sig_index
{0, 0}, // code {0, 0}, // code
0, // feedback slots
false, // imported false, // imported
false, // exported false, // exported
false}; // declared false}; // declared
......
...@@ -183,7 +183,6 @@ class TestModuleBuilder { ...@@ -183,7 +183,6 @@ class TestModuleBuilder {
static_cast<uint32_t>(mod.functions.size()), // func_index static_cast<uint32_t>(mod.functions.size()), // func_index
sig_index, // sig_index sig_index, // sig_index
{0, 0}, // code {0, 0}, // code
0, // feedback slots
false, // import false, // import
false, // export false, // export
declared}); // declared declared}); // declared
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment