Commit 28afd1c9 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

Revert "[wasm] Further reduce the size of WasmCode"

This reverts commit 79398ab0.

Reason for revert: Makes UBSan unhappy: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20UBSan/10186

Original change's description:
> [wasm] Further reduce the size of WasmCode
> 
> Also, save dynamic allocations (plus their memory overhead).
> This is realized by storing the relocation information, source position
> table, and protected instruction information together in one "metadata"
> byte array.
> For each of the three components, we just store their size, such that
> the accessors can return the respecitive {Vector} views as before.
> 
> This makes each WasmCode object 24 bytes smaller on 64-bit
> architectures. It also saves a few more bytes per code object because
> less padding is needed for the individual allocations, and each dynamic
> allocation comes with some constant memory overhead.
> 
> Since the protected instructions will just be stored in a byte array
> now, some APIs are refactored to just return that byte array directly
> (instead of an array of {ProtectedInstructionData}). This also
> simplifies serialization and deserialization, and will allow for
> switching to a more compact representation in the future.
> 
> Drive-by: Add some more checks to {Vector::cast} to protect against
>   undefined behaviour.
> 
> R=​ahaas@chromium.org
> 
> Bug: v8:10254
> Change-Id: I81ca847023841110e3e52cc402fcb0349325d7af
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2078545
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#66596}

TBR=jkummerow@chromium.org,ahaas@chromium.org,clemensb@chromium.org,tebbi@chromium.org

Change-Id: Id80aa82cfce8942879031032b322ee66855b5600
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10254
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2089933Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66597}
parent 79398ab0
......@@ -432,9 +432,10 @@ OwnedVector<byte> CodeGenerator::GetSourcePositionTable() {
return source_position_table_builder_.ToSourcePositionTableVector();
}
OwnedVector<byte> CodeGenerator::GetProtectedInstructionsData() {
return OwnedVector<byte>::Of(
Vector<byte>::cast(VectorOf(protected_instructions_)));
OwnedVector<trap_handler::ProtectedInstructionData>
CodeGenerator::GetProtectedInstructions() {
return OwnedVector<trap_handler::ProtectedInstructionData>::Of(
protected_instructions_);
}
MaybeHandle<Code> CodeGenerator::FinalizeCode() {
......
......@@ -125,7 +125,8 @@ class V8_EXPORT_PRIVATE CodeGenerator final : public GapResolver::Assembler {
MaybeHandle<Code> FinalizeCode();
OwnedVector<byte> GetSourcePositionTable();
OwnedVector<byte> GetProtectedInstructionsData();
OwnedVector<trap_handler::ProtectedInstructionData>
GetProtectedInstructions();
InstructionSequence* instructions() const { return instructions_; }
FrameAccessState* frame_access_state() const { return frame_access_state_; }
......
......@@ -2765,8 +2765,7 @@ wasm::WasmCompilationResult Pipeline::GenerateCodeForWasmNativeStub(
static_cast<int>(code_generator->GetHandlerTableOffset()));
result.instr_buffer = instruction_buffer->ReleaseBuffer();
result.source_positions = code_generator->GetSourcePositionTable();
result.protected_instructions_data =
code_generator->GetProtectedInstructionsData();
result.protected_instructions = code_generator->GetProtectedInstructions();
result.frame_slot_count = code_generator->frame()->GetTotalFrameSlotCount();
result.tagged_parameter_slots = call_descriptor->GetTaggedParameterSlots();
result.result_tier = wasm::ExecutionTier::kTurbofan;
......@@ -2973,8 +2972,7 @@ void Pipeline::GenerateCodeForWasmFunction(
result->frame_slot_count = code_generator->frame()->GetTotalFrameSlotCount();
result->tagged_parameter_slots = call_descriptor->GetTaggedParameterSlots();
result->source_positions = code_generator->GetSourcePositionTable();
result->protected_instructions_data =
code_generator->GetProtectedInstructionsData();
result->protected_instructions = code_generator->GetProtectedInstructions();
result->result_tier = wasm::ExecutionTier::kTurbofan;
if (data.info()->trace_turbo_json_enabled()) {
......
......@@ -6666,9 +6666,8 @@ wasm::WasmCode* CompileWasmCapiCallWrapper(wasm::WasmEngine* wasm_engine,
WasmStubAssemblerOptions(), source_positions);
std::unique_ptr<wasm::WasmCode> wasm_code = native_module->AddCode(
wasm::kAnonymousFuncIndex, result.code_desc, result.frame_slot_count,
result.tagged_parameter_slots,
result.protected_instructions_data.as_vector(),
result.source_positions.as_vector(), wasm::WasmCode::kWasmToCapiWrapper,
result.tagged_parameter_slots, std::move(result.protected_instructions),
std::move(result.source_positions), wasm::WasmCode::kWasmToCapiWrapper,
wasm::ExecutionTier::kNone);
return native_module->PublishCode(std::move(wasm_code));
}
......
......@@ -9,7 +9,6 @@
#include <cstring>
#include <iterator>
#include <memory>
#include <type_traits>
#include "src/common/checks.h"
#include "src/common/globals.h"
......@@ -117,14 +116,6 @@ class Vector {
template <typename S>
static constexpr Vector<T> cast(Vector<S> input) {
// Casting is potentially dangerous, so be really restrictive here. This
// might be lifted once we have use cases for that.
STATIC_ASSERT(std::is_pod<S>::value);
STATIC_ASSERT(std::is_pod<T>::value);
#if V8_HAS_CXX14_CONSTEXPR
DCHECK_EQ(0, (input.length() * sizeof(S)) % sizeof(T));
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(input.begin()) % alignof(T));
#endif
return Vector<T>(reinterpret_cast<T*>(input.begin()),
input.length() * sizeof(S) / sizeof(T));
}
......
......@@ -321,9 +321,10 @@ class LiftoffCompiler {
return source_position_table_builder_.ToSourcePositionTableVector();
}
OwnedVector<uint8_t> GetProtectedInstructionsData() const {
return OwnedVector<uint8_t>::Of(
Vector<const uint8_t>::cast(VectorOf(protected_instructions_)));
OwnedVector<trap_handler::ProtectedInstructionData> GetProtectedInstructions()
const {
return OwnedVector<trap_handler::ProtectedInstructionData>::Of(
protected_instructions_);
}
uint32_t GetTotalFrameSlotCount() const {
......@@ -2795,7 +2796,7 @@ WasmCompilationResult ExecuteLiftoffCompilation(
compiler->GetCode(&result.code_desc);
result.instr_buffer = instruction_buffer->ReleaseBuffer();
result.source_positions = compiler->GetSourcePositionTable();
result.protected_instructions_data = compiler->GetProtectedInstructionsData();
result.protected_instructions = compiler->GetProtectedInstructions();
result.frame_slot_count = compiler->GetTotalFrameSlotCount();
result.tagged_parameter_slots = call_descriptor->GetTaggedParameterSlots();
result.func_index = func_index;
......
......@@ -67,7 +67,7 @@ struct WasmCompilationResult {
uint32_t frame_slot_count = 0;
uint32_t tagged_parameter_slots = 0;
OwnedVector<byte> source_positions;
OwnedVector<byte> protected_instructions_data;
OwnedVector<trap_handler::ProtectedInstructionData> protected_instructions;
int func_index = kAnonymousFuncIndex;
ExecutionTier requested_tier;
ExecutionTier result_tier;
......
......@@ -2972,9 +2972,8 @@ WasmCode* CompileImportWrapper(
wasm_engine, &env, kind, sig, source_positions);
std::unique_ptr<WasmCode> wasm_code = native_module->AddCode(
result.func_index, result.code_desc, result.frame_slot_count,
result.tagged_parameter_slots,
result.protected_instructions_data.as_vector(),
result.source_positions.as_vector(), GetCodeKind(result),
result.tagged_parameter_slots, std::move(result.protected_instructions),
std::move(result.source_positions), GetCodeKind(result),
ExecutionTier::kNone);
WasmCode* published_code = native_module->PublishCode(std::move(wasm_code));
(*cache_scope)[key] = published_code;
......
This diff is collapsed.
......@@ -127,12 +127,9 @@ class V8_EXPORT_PRIVATE WasmCode final {
Address instruction_start() const {
return reinterpret_cast<Address>(instructions_.begin());
}
Vector<const byte> reloc_info() const {
return {protected_instructions_data().end(),
static_cast<size_t>(reloc_info_size_)};
}
Vector<const byte> reloc_info() const { return reloc_info_.as_vector(); }
Vector<const byte> source_positions() const {
return {reloc_info().end(), static_cast<size_t>(source_positions_size_)};
return source_position_table_.as_vector();
}
// TODO(clemensb): Make this return int.
......@@ -163,15 +160,9 @@ class V8_EXPORT_PRIVATE WasmCode final {
pc < reinterpret_cast<Address>(instructions_.end());
}
Vector<const uint8_t> protected_instructions_data() const {
return {meta_data_.get(),
static_cast<size_t>(protected_instructions_size_)};
}
Vector<const trap_handler::ProtectedInstructionData> protected_instructions()
Vector<trap_handler::ProtectedInstructionData> protected_instructions()
const {
return Vector<const trap_handler::ProtectedInstructionData>::cast(
protected_instructions_data());
return protected_instructions_.as_vector();
}
void Validate() const;
......@@ -227,17 +218,15 @@ class V8_EXPORT_PRIVATE WasmCode final {
int safepoint_table_offset, int handler_table_offset,
int constant_pool_offset, int code_comments_offset,
int unpadded_binary_size,
Vector<const byte> protected_instructions_data,
Vector<const byte> reloc_info,
Vector<const byte> source_position_table, Kind kind,
OwnedVector<trap_handler::ProtectedInstructionData>
protected_instructions,
OwnedVector<const byte> reloc_info,
OwnedVector<const byte> source_position_table, Kind kind,
ExecutionTier tier)
: instructions_(instructions),
reloc_info_(std::move(reloc_info)),
source_position_table_(std::move(source_position_table)),
native_module_(native_module),
meta_data_(ConcatenateBytes(
{protected_instructions_data, reloc_info, source_position_table})),
reloc_info_size_(reloc_info.length()),
source_positions_size_(source_position_table.length()),
protected_instructions_size_(protected_instructions_data.length()),
index_(index),
kind_(kind),
constant_pool_offset_(constant_pool_offset),
......@@ -247,6 +236,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
handler_table_offset_(handler_table_offset),
code_comments_offset_(code_comments_offset),
unpadded_binary_size_(unpadded_binary_size),
protected_instructions_(std::move(protected_instructions)),
tier_(tier) {
DCHECK_LE(safepoint_table_offset, unpadded_binary_size);
DCHECK_LE(handler_table_offset, unpadded_binary_size);
......@@ -254,9 +244,6 @@ class V8_EXPORT_PRIVATE WasmCode final {
DCHECK_LE(constant_pool_offset, unpadded_binary_size);
}
std::unique_ptr<const byte[]> ConcatenateBytes(
std::initializer_list<Vector<const byte>>);
// Code objects that have been registered with the global trap handler within
// this process, will have a {trap_handler_index} associated with them.
int trap_handler_index() const {
......@@ -278,16 +265,9 @@ class V8_EXPORT_PRIVATE WasmCode final {
V8_NOINLINE bool DecRefOnPotentiallyDeadCode();
Vector<byte> instructions_;
OwnedVector<const byte> reloc_info_;
OwnedVector<const byte> source_position_table_;
NativeModule* native_module_ = nullptr;
// {meta_data_} contains several byte vectors concatenated into one:
// - protected instructions data of size {protected_instructions_size_}
// - relocation info of size {reloc_info_size_}
// - source positions of size {source_positions_size_}
// Note that the protected instructions come first to ensure alignment.
std::unique_ptr<const byte[]> meta_data_;
const int reloc_info_size_;
const int source_positions_size_;
const int protected_instructions_size_;
int index_;
Kind kind_;
int constant_pool_offset_ = 0;
......@@ -302,6 +282,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
int code_comments_offset_ = 0;
int unpadded_binary_size_ = 0;
int trap_handler_index_ = -1;
OwnedVector<trap_handler::ProtectedInstructionData> protected_instructions_;
ExecutionTier tier_;
// WasmCode is ref counted. Counters are held by:
......@@ -323,7 +304,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
// often for rather small functions.
// Increase the limit if needed, but first check if the size increase is
// justified.
STATIC_ASSERT(sizeof(WasmCode) <= 96);
STATIC_ASSERT(sizeof(WasmCode) <= 120);
WasmCode::Kind GetCodeKind(const WasmCompilationResult& result);
......@@ -430,11 +411,13 @@ class V8_EXPORT_PRIVATE NativeModule final {
// {AddCode} is thread safe w.r.t. other calls to {AddCode} or methods adding
// code below, i.e. it can be called concurrently from background threads.
// The returned code still needs to be published via {PublishCode}.
std::unique_ptr<WasmCode> AddCode(int index, const CodeDesc& desc,
int stack_slots, int tagged_parameter_slots,
Vector<const byte> protected_instructions,
Vector<const byte> source_position_table,
WasmCode::Kind kind, ExecutionTier tier);
std::unique_ptr<WasmCode> AddCode(
int index, const CodeDesc& desc, int stack_slots,
int tagged_parameter_slots,
OwnedVector<trap_handler::ProtectedInstructionData>
protected_instructions,
OwnedVector<const byte> source_position_table, WasmCode::Kind kind,
ExecutionTier tier);
// {PublishCode} makes the code available to the system by entering it into
// the code table and patching the jump table. It returns a raw pointer to the
......@@ -448,9 +431,11 @@ class V8_EXPORT_PRIVATE NativeModule final {
int tagged_parameter_slots, int safepoint_table_offset,
int handler_table_offset, int constant_pool_offset,
int code_comments_offset, int unpadded_binary_size,
Vector<const byte> protected_instructions_data,
Vector<const byte> reloc_info, Vector<const byte> source_position_table,
WasmCode::Kind kind, ExecutionTier tier);
OwnedVector<trap_handler::ProtectedInstructionData>
protected_instructions,
OwnedVector<const byte> reloc_info,
OwnedVector<const byte> source_position_table, WasmCode::Kind kind,
ExecutionTier tier);
// Adds anonymous code for testing purposes.
WasmCode* AddCodeForTesting(Handle<Code> code);
......@@ -616,8 +601,9 @@ class V8_EXPORT_PRIVATE NativeModule final {
std::unique_ptr<WasmCode> AddCodeWithCodeSpace(
int index, const CodeDesc& desc, int stack_slots,
int tagged_parameter_slots,
Vector<const byte> protected_instructions_data,
Vector<const byte> source_position_table, WasmCode::Kind kind,
OwnedVector<trap_handler::ProtectedInstructionData>
protected_instructions,
OwnedVector<const byte> source_position_table, WasmCode::Kind kind,
ExecutionTier tier, Vector<uint8_t> code_space,
const JumpTablesRef& jump_tables_ref);
......
......@@ -900,9 +900,8 @@ void WasmDebugInfo::RedirectToInterpreter(Handle<WasmDebugInfo> debug_info,
module->functions[func_index].sig);
std::unique_ptr<wasm::WasmCode> wasm_code = native_module->AddCode(
func_index, result.code_desc, result.frame_slot_count,
result.tagged_parameter_slots,
result.protected_instructions_data.as_vector(),
result.source_positions.as_vector(), wasm::WasmCode::kInterpreterEntry,
result.tagged_parameter_slots, std::move(result.protected_instructions),
std::move(result.source_positions), wasm::WasmCode::kInterpreterEntry,
wasm::ExecutionTier::kInterpreter);
native_module->PublishCode(std::move(wasm_code));
DCHECK(native_module->IsRedirectedToInterpreter(func_index));
......
......@@ -1460,9 +1460,8 @@ void WasmInstanceObject::ImportWasmJSFunctionIntoTable(
isolate->wasm_engine(), &env, kind, sig, false);
std::unique_ptr<wasm::WasmCode> wasm_code = native_module->AddCode(
result.func_index, result.code_desc, result.frame_slot_count,
result.tagged_parameter_slots,
result.protected_instructions_data.as_vector(),
result.source_positions.as_vector(), GetCodeKind(result),
result.tagged_parameter_slots, std::move(result.protected_instructions),
std::move(result.source_positions), GetCodeKind(result),
wasm::ExecutionTier::kNone);
wasm::WasmCode* published_code =
native_module->PublishCode(std::move(wasm_code));
......
......@@ -99,17 +99,16 @@ class Reader {
return value;
}
template <typename T>
Vector<const T> ReadVector(size_t size) {
DCHECK_GE(current_size(), size);
Vector<const byte> bytes{pos_, size * sizeof(T)};
pos_ += size * sizeof(T);
void ReadVector(Vector<byte> v) {
if (v.size() > 0) {
DCHECK_GE(current_size(), v.size());
memcpy(v.begin(), current_location(), v.size());
pos_ += v.size();
}
if (FLAG_trace_wasm_serialization) {
StdoutStream{} << "read vector of " << size << " elements of size "
<< sizeof(T) << " (total size " << size * sizeof(T) << ")"
StdoutStream{} << "read vector of " << v.size() << " elements"
<< std::endl;
}
return Vector<const T>::cast(bytes);
}
void Skip(size_t size) { pos_ += size; }
......@@ -305,7 +304,8 @@ size_t NativeModuleSerializer::MeasureCode(const WasmCode* code) const {
code->kind() == WasmCode::kInterpreterEntry);
return kCodeHeaderSize + code->instructions().size() +
code->reloc_info().size() + code->source_positions().size() +
code->protected_instructions_data().size();
code->protected_instructions().size() *
sizeof(trap_handler::ProtectedInstructionData);
}
size_t NativeModuleSerializer::Measure() const {
......@@ -343,7 +343,7 @@ void NativeModuleSerializer::WriteCode(const WasmCode* code, Writer* writer) {
writer->Write(code->instructions().length());
writer->Write(code->reloc_info().length());
writer->Write(code->source_positions().length());
writer->Write(code->protected_instructions_data().length());
writer->Write(code->protected_instructions().length());
writer->Write(code->kind());
writer->Write(code->tier());
......@@ -355,7 +355,7 @@ void NativeModuleSerializer::WriteCode(const WasmCode* code, Writer* writer) {
// Write the reloc info, source positions, and protected code.
writer->WriteVector(code->reloc_info());
writer->WriteVector(code->source_positions());
writer->WriteVector(code->protected_instructions_data());
writer->WriteVector(Vector<byte>::cast(code->protected_instructions()));
#if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64 || V8_TARGET_ARCH_ARM || \
V8_TARGET_ARCH_PPC || V8_TARGET_ARCH_PPC64 || V8_TARGET_ARCH_S390X
// On platforms that don't support misaligned word stores, copy to an aligned
......@@ -516,17 +516,25 @@ bool NativeModuleDeserializer::ReadCode(int fn_index, Reader* reader) {
WasmCode::Kind kind = reader->Read<WasmCode::Kind>();
ExecutionTier tier = reader->Read<ExecutionTier>();
auto code_buffer = reader->ReadVector<byte>(code_size);
auto reloc_info = reader->ReadVector<byte>(reloc_size);
auto source_pos = reader->ReadVector<byte>(source_position_size);
Vector<const byte> code_buffer{reader->current_location(),
static_cast<size_t>(code_size)};
reader->Skip(code_size);
OwnedVector<byte> reloc_info = OwnedVector<byte>::New(reloc_size);
reader->ReadVector(reloc_info.as_vector());
OwnedVector<byte> source_pos = OwnedVector<byte>::New(source_position_size);
reader->ReadVector(source_pos.as_vector());
auto protected_instructions =
reader->ReadVector<byte>(protected_instructions_size);
OwnedVector<trap_handler::ProtectedInstructionData>::New(
protected_instructions_size);
reader->ReadVector(Vector<byte>::cast(protected_instructions.as_vector()));
WasmCode* code = native_module_->AddDeserializedCode(
fn_index, code_buffer, stack_slot_count, tagged_parameter_slots,
safepoint_table_offset, handler_table_offset, constant_pool_offset,
code_comment_offset, unpadded_binary_size, protected_instructions,
std::move(reloc_info), std::move(source_pos), kind, tier);
code_comment_offset, unpadded_binary_size,
std::move(protected_instructions), std::move(reloc_info),
std::move(source_pos), kind, tier);
// Relocate the code.
int mask = RelocInfo::ModeMask(RelocInfo::WASM_CALL) |
......
......@@ -3753,9 +3753,9 @@ TEST(Liftoff_tier_up) {
memcpy(buffer.get(), sub_code->instructions().begin(), sub_size);
desc.buffer = buffer.get();
desc.instr_size = static_cast<int>(sub_size);
std::unique_ptr<WasmCode> new_code =
native_module->AddCode(add.function_index(), desc, 0, 0, {}, {},
WasmCode::kFunction, ExecutionTier::kTurbofan);
std::unique_ptr<WasmCode> new_code = native_module->AddCode(
add.function_index(), desc, 0, 0, {}, OwnedVector<byte>(),
WasmCode::kFunction, ExecutionTier::kTurbofan);
native_module->PublishCode(std::move(new_code));
// Second run should now execute {sub}.
......
......@@ -144,9 +144,8 @@ uint32_t TestingModuleBuilder::AddFunction(const FunctionSig* sig,
sig);
std::unique_ptr<wasm::WasmCode> code = native_module_->AddCode(
index, result.code_desc, result.frame_slot_count,
result.tagged_parameter_slots,
result.protected_instructions_data.as_vector(),
result.source_positions.as_vector(), wasm::WasmCode::kInterpreterEntry,
result.tagged_parameter_slots, std::move(result.protected_instructions),
std::move(result.source_positions), wasm::WasmCode::kInterpreterEntry,
wasm::ExecutionTier::kInterpreter);
native_module_->PublishCode(std::move(code));
}
......
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