Commit 93c24065 authored by Manos Koukoutos's avatar Manos Koukoutos Committed by Commit Bot

[wasm][turbofan] Impose single appearance of each parameter

The register allocator sometimes fails if a parameter node appears
twice. This seems to be an issue in the register allocator rather than
a global assumption of Turbofan. This CL ensures duplication does not
happen in wasm code until the issue is resolved.

Changes:
- Cache parameter nodes in wasm-compiler.
- Use Dead() over Parameter() as placeholder in lowering stages.

Change-Id: I7afb5de45dd169819309fea3d3c1a7cfe68af62c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2756529
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73427}
parent 32828e61
......@@ -35,8 +35,7 @@ Int64Lowering::Int64Lowering(
stack_(zone),
replacements_(nullptr),
signature_(signature),
placeholder_(
graph->NewNode(common->Parameter(-2, "placeholder"), graph->start())),
placeholder_(graph->NewNode(common->Dead())),
special_case_(std::move(special_case)) {
DCHECK_NOT_NULL(graph);
DCHECK_NOT_NULL(graph->end());
......
......@@ -68,8 +68,7 @@ SimdScalarLowering::SimdScalarLowering(
stack_(mcgraph_->zone()),
replacements_(nullptr),
signature_(signature),
placeholder_(graph()->NewNode(common()->Parameter(-2, "placeholder"),
graph()->start())),
placeholder_(graph()->NewNode(common()->Dead())),
parameter_count_after_lowering_(-1) {
DCHECK_NOT_NULL(graph());
DCHECK_NOT_NULL(graph()->end());
......
......@@ -468,11 +468,24 @@ WasmGraphBuilder::~WasmGraphBuilder() = default;
Node* WasmGraphBuilder::Start(unsigned params) {
Node* start = graph()->NewNode(mcgraph()->common()->Start(params));
graph()->SetStart(start);
parameters_ = zone_->NewArray<Node*>(params);
for (unsigned i = 0; i < params; i++) {
parameters_[i] = nullptr;
}
return start;
}
Node* WasmGraphBuilder::Param(unsigned index) {
return gasm_->Parameter(index);
Node* WasmGraphBuilder::Param(int index, const char* debug_name) {
DCHECK_NOT_NULL(graph()->start());
// Turbofan allows negative parameter indices.
static constexpr int kMinParameterIndex = -1;
DCHECK_GE(index, kMinParameterIndex);
int array_index = index - kMinParameterIndex;
if (parameters_[array_index] == nullptr) {
parameters_[array_index] = graph()->NewNode(
mcgraph()->common()->Parameter(index, debug_name), graph()->start());
}
return parameters_[array_index];
}
Node* WasmGraphBuilder::Loop(Node* entry) {
......@@ -6678,20 +6691,9 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
SetEffectControl(Start(wasm_param_count + 5));
// Create the js_closure and js_context parameters.
Node* js_closure =
graph()->NewNode(mcgraph()->common()->Parameter(
Linkage::kJSCallClosureParamIndex, "%closure"),
graph()->start());
Node* js_context = graph()->NewNode(
mcgraph()->common()->Parameter(
Linkage::GetJSCallContextParamIndex(wasm_param_count + 1),
"%context"),
graph()->start());
// Create the instance_node node to pass as parameter. It is loaded from
// an actual reference to an instance or a placeholder reference,
// called {WasmExportedFunction} via the {WasmExportedFunctionData}
// structure.
Node* js_closure = Param(Linkage::kJSCallClosureParamIndex, "%closure");
Node* js_context = Param(
Linkage::GetJSCallContextParamIndex(wasm_param_count + 1), "%context");
Node* function_data = gasm_->LoadFunctionDataFromJSFunction(js_closure);
instance_node_.set(gasm_->LoadExportedFunctionInstance(function_data));
......
......@@ -229,7 +229,7 @@ class WasmGraphBuilder {
// Operations independent of {control} or {effect}.
//-----------------------------------------------------------------------
Node* Start(unsigned params);
Node* Param(unsigned index);
Node* Param(int index, const char* debug_name = nullptr);
Node* Loop(Node* entry);
Node* TerminateLoop(Node* effect, Node* control);
Node* LoopExit(Node* loop_node);
......@@ -727,6 +727,8 @@ class WasmGraphBuilder {
MachineGraph* const mcgraph_;
wasm::CompilationEnv* const env_;
Node** parameters_;
WasmInstanceCacheNodes* instance_cache_ = nullptr;
SetOncePointer<Node> instance_node_;
......
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