AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[wasm] Cache instance node in wrappers

Browse Source 
Since wrappers do not get optimized,
https://chromium-review.googlesource.com/c/v8/v8/+/2739976 introduced
some performance regression by not caching nodes in the
WasmGraphBuilder. Therefore, we reintroduce caching of the instance
node. We do it in Start() to ensure the effect chain is correct.
Additional changes:
- Change signature of Start() to void.
- Initialize effect and control in Start().
- Rename BuildLoadInstance() -> GetInstance().

Bug: chromium:1189100
Change-Id: I9147f738e67b4f4b822c845e7d33d9fd4ceb65fa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2804679
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73824}
This commit is contained in:
Manos Koukoutos 2021-04-07 10:48:59 +00:00 committed by Commit Bot
parent 997d88e64f
commit eafdc074fb
3 changed files with 32 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
src/compiler/wasm-compiler.cc
View File

@ -81,7 +81,7 @@ MachineType assert_size(int expected_size, MachineType type) {
#define LOAD_MUTABLE_INSTANCE_FIELD(name, type) \ #define LOAD_MUTABLE_INSTANCE_FIELD(name, type) \
gasm_->LoadFromObject( \ gasm_->LoadFromObject( \
assert_size(WASM_INSTANCE_OBJECT_SIZE(name), type), BuildLoadInstance(), \ assert_size(WASM_INSTANCE_OBJECT_SIZE(name), type), GetInstance(), \
wasm::ObjectAccess::ToTagged(WasmInstanceObject::k##name##Offset)) wasm::ObjectAccess::ToTagged(WasmInstanceObject::k##name##Offset))
// TODO(11510): Using LoadImmutable for tagged values causes registers to be // TODO(11510): Using LoadImmutable for tagged values causes registers to be
@ -94,7 +94,7 @@ MachineType assert_size(int expected_size, MachineType type) {
? LOAD_MUTABLE_INSTANCE_FIELD(name, type) \ ? LOAD_MUTABLE_INSTANCE_FIELD(name, type) \
: gasm_->LoadImmutable( \ : gasm_->LoadImmutable( \
assert_size(WASM_INSTANCE_OBJECT_SIZE(name), type), \ assert_size(WASM_INSTANCE_OBJECT_SIZE(name), type), \
BuildLoadInstance(), \ GetInstance(), \
wasm::ObjectAccess::ToTagged( \ wasm::ObjectAccess::ToTagged( \
WasmInstanceObject::k##name##Offset))) WasmInstanceObject::k##name##Offset)))
@ -487,14 +487,22 @@ WasmGraphBuilder::WasmGraphBuilder(
// available. // available.
WasmGraphBuilder::~WasmGraphBuilder() = default; WasmGraphBuilder::~WasmGraphBuilder() = default;
Node* WasmGraphBuilder::Start(unsigned params) { void WasmGraphBuilder::Start(unsigned params) {
Node* start = graph()->NewNode(mcgraph()->common()->Start(params)); Node* start = graph()->NewNode(mcgraph()->common()->Start(params));
graph()->SetStart(start); graph()->SetStart(start);
SetEffectControl(start);
// Initialize parameter nodes.
parameters_ = zone_->NewArray<Node*>(params); parameters_ = zone_->NewArray<Node*>(params);
for (unsigned i = 0; i < params; i++) { for (unsigned i = 0; i < params; i++) {
parameters_[i] = nullptr; parameters_[i] = nullptr;
} }
return start; // Initialize instance node.
instance_node_ =
use_js_isolate_and_params()
? gasm_->LoadExportedFunctionInstance(
gasm_->LoadFunctionDataFromJSFunction(
Param(Linkage::kJSCallClosureParamIndex, "%closure")))
: Param(wasm::kWasmInstanceParameterIndex);
} }
Node* WasmGraphBuilder::Param(int index, const char* debug_name) { Node* WasmGraphBuilder::Param(int index, const char* debug_name) {
@ -620,15 +628,7 @@ Node* WasmGraphBuilder::NoContextConstant() {
return mcgraph()->IntPtrConstant(0); return mcgraph()->IntPtrConstant(0);
} }
Node* WasmGraphBuilder::BuildLoadInstance() { Node* WasmGraphBuilder::GetInstance() { return instance_node_.get(); }
if (use_js_isolate_and_params()) {
Node* js_closure = Param(Linkage::kJSCallClosureParamIndex, "%closure");
return gasm_->LoadExportedFunctionInstance(
gasm_->LoadFunctionDataFromJSFunction(js_closure));
} else {
return Param(wasm::kWasmInstanceParameterIndex);
}
}
Node* WasmGraphBuilder::BuildLoadIsolateRoot() { Node* WasmGraphBuilder::BuildLoadIsolateRoot() {
if (use_js_isolate_and_params()) { if (use_js_isolate_and_params()) {
@ -2845,7 +2845,7 @@ Node* WasmGraphBuilder::BuildCallNode(const wasm::FunctionSig* sig,
Node* instance_node, const Operator* op, Node* instance_node, const Operator* op,
Node* frame_state) { Node* frame_state) {
if (instance_node == nullptr) { if (instance_node == nullptr) {
instance_node = BuildLoadInstance(); instance_node = GetInstance();
} }
needs_stack_check_ = true; needs_stack_check_ = true;
const size_t params = sig->parameter_count(); const size_t params = sig->parameter_count();
@ -3234,7 +3234,7 @@ Node* WasmGraphBuilder::BuildCallRef(uint32_t sig_index, Vector<Node*> args,
// every call. // every call.
Node* function_instance_node = Node* function_instance_node =
gasm_->CallBuiltin(Builtins::kWasmAllocatePair, Operator::kEliminatable, gasm_->CallBuiltin(Builtins::kWasmAllocatePair, Operator::kEliminatable,
BuildLoadInstance(), callable); GetInstance(), callable);
gasm_->Goto(&end_label, call_target, function_instance_node); gasm_->Goto(&end_label, call_target, function_instance_node);
} }
@ -5444,7 +5444,7 @@ void WasmGraphBuilder::MemoryInit(uint32_t data_segment_index, Node* dst,
gasm_->ExternalConstant(ExternalReference::wasm_memory_init()); gasm_->ExternalConstant(ExternalReference::wasm_memory_init());
Node* stack_slot = StoreArgsInStackSlot( Node* stack_slot = StoreArgsInStackSlot(
{{MachineType::PointerRepresentation(), BuildLoadInstance()}, {{MachineType::PointerRepresentation(), GetInstance()},
{MachineRepresentation::kWord32, dst}, {MachineRepresentation::kWord32, dst},
{MachineRepresentation::kWord32, src}, {MachineRepresentation::kWord32, src},
{MachineRepresentation::kWord32, {MachineRepresentation::kWord32,
@ -5496,7 +5496,7 @@ void WasmGraphBuilder::MemoryCopy(Node* dst, Node* src, Node* size,
gasm_->ExternalConstant(ExternalReference::wasm_memory_copy()); gasm_->ExternalConstant(ExternalReference::wasm_memory_copy());
Node* stack_slot = StoreArgsInStackSlot( Node* stack_slot = StoreArgsInStackSlot(
{{MachineType::PointerRepresentation(), BuildLoadInstance()}, {{MachineType::PointerRepresentation(), GetInstance()},
{MachineRepresentation::kWord32, dst}, {MachineRepresentation::kWord32, dst},
{MachineRepresentation::kWord32, src}, {MachineRepresentation::kWord32, src},
{MachineRepresentation::kWord32, size}}); {MachineRepresentation::kWord32, size}});
@ -5513,7 +5513,7 @@ void WasmGraphBuilder::MemoryFill(Node* dst, Node* value, Node* size,
gasm_->ExternalConstant(ExternalReference::wasm_memory_fill()); gasm_->ExternalConstant(ExternalReference::wasm_memory_fill());
Node* stack_slot = StoreArgsInStackSlot( Node* stack_slot = StoreArgsInStackSlot(
{{MachineType::PointerRepresentation(), BuildLoadInstance()}, {{MachineType::PointerRepresentation(), GetInstance()},
{MachineRepresentation::kWord32, dst}, {MachineRepresentation::kWord32, dst},
{MachineRepresentation::kWord32, value}, {MachineRepresentation::kWord32, value},
{MachineRepresentation::kWord32, size}}); {MachineRepresentation::kWord32, size}});
@ -6364,7 +6364,7 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
wasm::ValueType type) { wasm::ValueType type) {
// Make sure ValueType fits in a Smi. // Make sure ValueType fits in a Smi.
STATIC_ASSERT(wasm::ValueType::kLastUsedBit + 1 <= kSmiValueSize); STATIC_ASSERT(wasm::ValueType::kLastUsedBit + 1 <= kSmiValueSize);
Node* inputs[] = {BuildLoadInstance(), input, Node* inputs[] = {GetInstance(), input,
mcgraph()->IntPtrConstant( mcgraph()->IntPtrConstant(
IntToSmi(static_cast<int>(type.raw_bit_field())))}; IntToSmi(static_cast<int>(type.raw_bit_field())))};
@ -6721,7 +6721,7 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
const int wasm_param_count = static_cast<int>(sig_->parameter_count()); const int wasm_param_count = static_cast<int>(sig_->parameter_count());
// Build the start and the JS parameter nodes. // Build the start and the JS parameter nodes.
SetEffectControl(Start(wasm_param_count + 5)); Start(wasm_param_count + 5);
// Create the js_closure and js_context parameters. // Create the js_closure and js_context parameters.
Node* js_closure = Param(Linkage::kJSCallClosureParamIndex, "%closure"); Node* js_closure = Param(Linkage::kJSCallClosureParamIndex, "%closure");
@ -6840,7 +6840,7 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
int wasm_count = static_cast<int>(sig_->parameter_count()); int wasm_count = static_cast<int>(sig_->parameter_count());
// Build the start and the parameter nodes. // Build the start and the parameter nodes.
SetEffectControl(Start(wasm_count + 4)); Start(wasm_count + 4);
Node* native_context = Node* native_context =
LOAD_INSTANCE_FIELD(NativeContext, MachineType::TaggedPointer()); LOAD_INSTANCE_FIELD(NativeContext, MachineType::TaggedPointer());
@ -7096,7 +7096,7 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
// Build the start and the parameter nodes. // Build the start and the parameter nodes.
int param_count = 1 /* closure */ + 1 /* receiver */ + wasm_count + int param_count = 1 /* closure */ + 1 /* receiver */ + wasm_count +
1 /* new.target */ + 1 /* #arg */ + 1 /* context */; 1 /* new.target */ + 1 /* #arg */ + 1 /* context */;
SetEffectControl(Start(param_count)); Start(param_count);
Node* closure = Param(Linkage::kJSCallClosureParamIndex); Node* closure = Param(Linkage::kJSCallClosureParamIndex);
Node* context = Param(Linkage::GetJSCallContextParamIndex(wasm_count + 1)); Node* context = Param(Linkage::GetJSCallContextParamIndex(wasm_count + 1));
@ -7166,7 +7166,7 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
void BuildCWasmEntry() { void BuildCWasmEntry() {
// +1 offset for first parameter index being -1. // +1 offset for first parameter index being -1.
SetEffectControl(Start(CWasmEntryParameters::kNumParameters + 1)); Start(CWasmEntryParameters::kNumParameters + 1);
Node* code_entry = Param(CWasmEntryParameters::kCodeEntry); Node* code_entry = Param(CWasmEntryParameters::kCodeEntry);
Node* object_ref = Param(CWasmEntryParameters::kObjectRef); Node* object_ref = Param(CWasmEntryParameters::kObjectRef);
@ -7504,8 +7504,7 @@ wasm::WasmCompilationResult CompileWasmMathIntrinsic(
source_positions); source_positions);
// Set up the graph start. // Set up the graph start.
Node* start = builder.Start(static_cast<int>(sig->parameter_count() + 1 + 1)); builder.Start(static_cast<int>(sig->parameter_count() + 1 + 1));
builder.SetEffectControl(start);
// Generate either a unop or a binop. // Generate either a unop or a binop.
Node* node = nullptr; Node* node = nullptr;
@ -7623,8 +7622,7 @@ wasm::WasmCode* CompileWasmCapiCallWrapper(wasm::WasmEngine* wasm_engine,
int param_count = static_cast<int>(sig->parameter_count()) + int param_count = static_cast<int>(sig->parameter_count()) +
1 /* offset for first parameter index being -1 */ + 1 /* offset for first parameter index being -1 */ +
1 /* Wasm instance */ + 1 /* kExtraCallableParam */; 1 /* Wasm instance */ + 1 /* kExtraCallableParam */;
Node* start = builder.Start(param_count); builder.Start(param_count);
builder.SetEffectControl(start);
builder.BuildCapiCallWrapper(address); builder.BuildCapiCallWrapper(address);
// Run the compiler pipeline to generate machine code. // Run the compiler pipeline to generate machine code.

5
src/compiler/wasm-compiler.h
View File

@ -229,7 +229,7 @@ class WasmGraphBuilder {
//----------------------------------------------------------------------- //-----------------------------------------------------------------------
// Operations independent of {control} or {effect}. // Operations independent of {control} or {effect}.
//----------------------------------------------------------------------- //-----------------------------------------------------------------------
Node* Start(unsigned params); void Start(unsigned params);
Node* Param(int index, const char* debug_name = nullptr); Node* Param(int index, const char* debug_name = nullptr);
Node* Loop(Node* entry); Node* Loop(Node* entry);
void TerminateLoop(Node* effect, Node* control); void TerminateLoop(Node* effect, Node* control);
@ -518,7 +518,7 @@ class WasmGraphBuilder {
Node* NoContextConstant(); Node* NoContextConstant();
Node* BuildLoadInstance(); Node* GetInstance();
Node* BuildLoadIsolateRoot(); Node* BuildLoadIsolateRoot();
// MemBuffer is only called with valid offsets (after bounds checking), so the // MemBuffer is only called with valid offsets (after bounds checking), so the
@ -748,6 +748,7 @@ class WasmGraphBuilder {
compiler::SourcePositionTable* const source_position_table_ = nullptr; compiler::SourcePositionTable* const source_position_table_ = nullptr;
Isolate* const isolate_; Isolate* const isolate_;
SetOncePointer<Node> instance_node_;
std::unique_ptr<Int64LoweringSpecialCase> lowering_special_case_; std::unique_ptr<Int64LoweringSpecialCase> lowering_special_case_;
CallDescriptor* i32_atomic_wait_descriptor_ = nullptr; CallDescriptor* i32_atomic_wait_descriptor_ = nullptr;

5
src/wasm/graph-builder-interface.cc
View File

@ -116,11 +116,10 @@ class WasmGraphBuildingInterface {
void StartFunction(FullDecoder* decoder) { void StartFunction(FullDecoder* decoder) {
// The first '+ 1' is needed by TF Start node, the second '+ 1' is for the // The first '+ 1' is needed by TF Start node, the second '+ 1' is for the
// instance parameter. // instance parameter.
TFNode* start = builder_->Start( builder_->Start(static_cast<int>(decoder->sig_->parameter_count() + 1 + 1));
static_cast<int>(decoder->sig_->parameter_count() + 1 + 1));
uint32_t num_locals = decoder->num_locals(); uint32_t num_locals = decoder->num_locals();
SsaEnv* ssa_env = decoder->zone()->New<SsaEnv>( SsaEnv* ssa_env = decoder->zone()->New<SsaEnv>(
decoder->zone(), SsaEnv::kReached, start, start, num_locals); decoder->zone(), SsaEnv::kReached, effect(), control(), num_locals);
SetEnv(ssa_env); SetEnv(ssa_env);
// Initialize local variables. Parameters are shifted by 1 because of the // Initialize local variables. Parameters are shifted by 1 because of the