Commit 6f5f8e6c authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ

Revert "[wasm] Introduce CallInfo in WasmGraphBuildingInterface"

This reverts commit db95e20b.

Reason for revert: UBSan failures https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20UBSan/18300/overview

Original change's description:
> [wasm] Introduce CallInfo in WasmGraphBuildingInterface
>
> The DoCall and DoReturnCall functions implement function calls in
> WasmGraphBuilderInterface. These functions need different arguments
> based on if the call is direct, indirect or call_ref. Right now, these
> arguments are misnamed in some cases, and callers have to pass default
> values for unused arguments.
> This CL tidies up the arguments of these functions by introducing a
> CallInfo class which provides different constructors based on the type
> of the call, where only the required arguments need to be passed.
>
> Change-Id: Ie03de6d3cf253a9baa0369f569589bb91d0b1866
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3162606
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#76910}

Change-Id: Ie0b288b3cbb66de4858fb7fbf1bc992518e637d0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3168284
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#76911}
parent db95e20b
...@@ -3155,7 +3155,7 @@ Node* WasmGraphBuilder::BuildLoadCallTargetFromExportedFunctionData( ...@@ -3155,7 +3155,7 @@ Node* WasmGraphBuilder::BuildLoadCallTargetFromExportedFunctionData(
} }
// TODO(9495): Support CAPI function refs. // TODO(9495): Support CAPI function refs.
Node* WasmGraphBuilder::BuildCallRef(const wasm::FunctionSig* sig, Node* WasmGraphBuilder::BuildCallRef(uint32_t sig_index,
base::Vector<Node*> args, base::Vector<Node*> args,
base::Vector<Node*> rets, base::Vector<Node*> rets,
CheckForNull null_check, CheckForNull null_check,
...@@ -3166,6 +3166,8 @@ Node* WasmGraphBuilder::BuildCallRef(const wasm::FunctionSig* sig, ...@@ -3166,6 +3166,8 @@ Node* WasmGraphBuilder::BuildCallRef(const wasm::FunctionSig* sig,
position); position);
} }
const wasm::FunctionSig* sig = env_->module->signature(sig_index);
Node* function_data = gasm_->LoadFunctionDataFromJSFunction(args[0]); Node* function_data = gasm_->LoadFunctionDataFromJSFunction(args[0]);
auto load_target = gasm_->MakeLabel(); auto load_target = gasm_->MakeLabel();
...@@ -3241,21 +3243,20 @@ void WasmGraphBuilder::CompareToExternalFunctionAtIndex( ...@@ -3241,21 +3243,20 @@ void WasmGraphBuilder::CompareToExternalFunctionAtIndex(
failure_control, BranchHint::kTrue); failure_control, BranchHint::kTrue);
} }
Node* WasmGraphBuilder::CallRef(const wasm::FunctionSig* sig, Node* WasmGraphBuilder::CallRef(uint32_t sig_index, base::Vector<Node*> args,
base::Vector<Node*> args,
base::Vector<Node*> rets, base::Vector<Node*> rets,
WasmGraphBuilder::CheckForNull null_check, WasmGraphBuilder::CheckForNull null_check,
wasm::WasmCodePosition position) { wasm::WasmCodePosition position) {
return BuildCallRef(sig, args, rets, null_check, IsReturnCall::kCallContinues, return BuildCallRef(sig_index, args, rets, null_check,
position); IsReturnCall::kCallContinues, position);
} }
Node* WasmGraphBuilder::ReturnCallRef(const wasm::FunctionSig* sig, Node* WasmGraphBuilder::ReturnCallRef(uint32_t sig_index,
base::Vector<Node*> args, base::Vector<Node*> args,
WasmGraphBuilder::CheckForNull null_check, WasmGraphBuilder::CheckForNull null_check,
wasm::WasmCodePosition position) { wasm::WasmCodePosition position) {
return BuildCallRef(sig, args, {}, null_check, IsReturnCall::kReturnCall, return BuildCallRef(sig_index, args, {}, null_check,
position); IsReturnCall::kReturnCall, position);
} }
Node* WasmGraphBuilder::ReturnCall(uint32_t index, base::Vector<Node*> args, Node* WasmGraphBuilder::ReturnCall(uint32_t index, base::Vector<Node*> args,
......
...@@ -325,7 +325,7 @@ class WasmGraphBuilder { ...@@ -325,7 +325,7 @@ class WasmGraphBuilder {
Node* CallIndirect(uint32_t table_index, uint32_t sig_index, Node* CallIndirect(uint32_t table_index, uint32_t sig_index,
base::Vector<Node*> args, base::Vector<Node*> rets, base::Vector<Node*> args, base::Vector<Node*> rets,
wasm::WasmCodePosition position); wasm::WasmCodePosition position);
Node* CallRef(const wasm::FunctionSig* sig, base::Vector<Node*> args, Node* CallRef(uint32_t sig_index, base::Vector<Node*> args,
base::Vector<Node*> rets, CheckForNull null_check, base::Vector<Node*> rets, CheckForNull null_check,
wasm::WasmCodePosition position); wasm::WasmCodePosition position);
void CompareToExternalFunctionAtIndex(Node* func_ref, uint32_t function_index, void CompareToExternalFunctionAtIndex(Node* func_ref, uint32_t function_index,
...@@ -337,7 +337,7 @@ class WasmGraphBuilder { ...@@ -337,7 +337,7 @@ class WasmGraphBuilder {
Node* ReturnCallIndirect(uint32_t table_index, uint32_t sig_index, Node* ReturnCallIndirect(uint32_t table_index, uint32_t sig_index,
base::Vector<Node*> args, base::Vector<Node*> args,
wasm::WasmCodePosition position); wasm::WasmCodePosition position);
Node* ReturnCallRef(const wasm::FunctionSig* sig, base::Vector<Node*> args, Node* ReturnCallRef(uint32_t sig_index, base::Vector<Node*> args,
CheckForNull null_check, wasm::WasmCodePosition position); CheckForNull null_check, wasm::WasmCodePosition position);
void BrOnNull(Node* ref_object, Node** non_null_node, Node** null_node); void BrOnNull(Node* ref_object, Node** non_null_node, Node** null_node);
...@@ -589,7 +589,7 @@ class WasmGraphBuilder { ...@@ -589,7 +589,7 @@ class WasmGraphBuilder {
base::Vector<Node*> rets, base::Vector<Node*> rets,
wasm::WasmCodePosition position, Node* func_index, wasm::WasmCodePosition position, Node* func_index,
IsReturnCall continuation); IsReturnCall continuation);
Node* BuildCallRef(const wasm::FunctionSig* sig, base::Vector<Node*> args, Node* BuildCallRef(uint32_t sig_index, base::Vector<Node*> args,
base::Vector<Node*> rets, CheckForNull null_check, base::Vector<Node*> rets, CheckForNull null_check,
IsReturnCall continuation, IsReturnCall continuation,
wasm::WasmCodePosition position); wasm::WasmCodePosition position);
......
...@@ -625,42 +625,44 @@ class WasmGraphBuildingInterface { ...@@ -625,42 +625,44 @@ class WasmGraphBuildingInterface {
LoadContextIntoSsa(ssa_env_); LoadContextIntoSsa(ssa_env_);
} }
enum CallMode { kCallDirect, kCallIndirect, kCallRef };
void CallDirect(FullDecoder* decoder, void CallDirect(FullDecoder* decoder,
const CallFunctionImmediate<validate>& imm, const CallFunctionImmediate<validate>& imm,
const Value args[], Value returns[]) { const Value args[], Value returns[]) {
DoCall(decoder, CallInfo::CallDirect(imm.index), imm.sig, args, returns); DoCall(decoder, kCallDirect, 0, CheckForNull::kWithoutNullCheck, nullptr,
imm.sig, imm.index, args, returns);
} }
void ReturnCall(FullDecoder* decoder, void ReturnCall(FullDecoder* decoder,
const CallFunctionImmediate<validate>& imm, const CallFunctionImmediate<validate>& imm,
const Value args[]) { const Value args[]) {
DoReturnCall(decoder, CallInfo::CallDirect(imm.index), imm.sig, args); DoReturnCall(decoder, kCallDirect, 0, CheckForNull::kWithoutNullCheck,
Value{nullptr, kWasmBottom}, imm.sig, imm.index, args);
} }
void CallIndirect(FullDecoder* decoder, const Value& index, void CallIndirect(FullDecoder* decoder, const Value& index,
const CallIndirectImmediate<validate>& imm, const CallIndirectImmediate<validate>& imm,
const Value args[], Value returns[]) { const Value args[], Value returns[]) {
DoCall( DoCall(decoder, kCallIndirect, imm.table_imm.index,
decoder, CheckForNull::kWithoutNullCheck, index.node, imm.sig,
CallInfo::CallIndirect(index, imm.table_imm.index, imm.sig_imm.index), imm.sig_imm.index, args, returns);
imm.sig, args, returns);
} }
void ReturnCallIndirect(FullDecoder* decoder, const Value& index, void ReturnCallIndirect(FullDecoder* decoder, const Value& index,
const CallIndirectImmediate<validate>& imm, const CallIndirectImmediate<validate>& imm,
const Value args[]) { const Value args[]) {
DoReturnCall( DoReturnCall(decoder, kCallIndirect, imm.table_imm.index,
decoder, CheckForNull::kWithoutNullCheck, index, imm.sig,
CallInfo::CallIndirect(index, imm.table_imm.index, imm.sig_imm.index), imm.sig_imm.index, args);
imm.sig, args);
} }
void CallRef(FullDecoder* decoder, const Value& func_ref, void CallRef(FullDecoder* decoder, const Value& func_ref,
const FunctionSig* sig, uint32_t sig_index, const Value args[], const FunctionSig* sig, uint32_t sig_index, const Value args[],
Value returns[]) { Value returns[]) {
if (!FLAG_wasm_inlining) { if (!FLAG_wasm_inlining) {
DoCall(decoder, CallInfo::CallRef(func_ref, NullCheckFor(func_ref.type)), DoCall(decoder, kCallRef, 0, NullCheckFor(func_ref.type), func_ref.node,
sig, args, returns); sig, sig_index, args, returns);
return; return;
} }
...@@ -681,8 +683,10 @@ class WasmGraphBuildingInterface { ...@@ -681,8 +683,10 @@ class WasmGraphBuildingInterface {
ssa_env_->control = success_control; ssa_env_->control = success_control;
Value* returns_direct = Value* returns_direct =
decoder->zone()->NewArray<Value>(sig->return_count()); decoder->zone()->NewArray<Value>(sig->return_count());
DoCall(decoder, CallInfo::CallDirect(expected_function_index), DoCall(decoder, kCallDirect, expected_function_index,
decoder->module_->signature(sig_index), args, returns_direct); CheckForNull::kWithoutNullCheck, nullptr,
decoder->module_->signature(sig_index), sig_index, args,
returns_direct);
TFNode* control_direct = control(); TFNode* control_direct = control();
TFNode* effect_direct = effect(); TFNode* effect_direct = effect();
...@@ -690,9 +694,8 @@ class WasmGraphBuildingInterface { ...@@ -690,9 +694,8 @@ class WasmGraphBuildingInterface {
ssa_env_->effect = initial_effect; ssa_env_->effect = initial_effect;
ssa_env_->control = failure_control; ssa_env_->control = failure_control;
Value* returns_ref = decoder->zone()->NewArray<Value>(sig->return_count()); Value* returns_ref = decoder->zone()->NewArray<Value>(sig->return_count());
DoCall(decoder, CallInfo::CallRef(func_ref, NullCheckFor(func_ref.type)), DoCall(decoder, kCallRef, 0, NullCheckFor(func_ref.type), func_ref.node,
sig, args, returns_ref); sig, sig_index, args, returns_ref);
TFNode* control_ref = control(); TFNode* control_ref = control();
TFNode* effect_ref = effect(); TFNode* effect_ref = effect();
...@@ -717,9 +720,8 @@ class WasmGraphBuildingInterface { ...@@ -717,9 +720,8 @@ class WasmGraphBuildingInterface {
const FunctionSig* sig, uint32_t sig_index, const FunctionSig* sig, uint32_t sig_index,
const Value args[]) { const Value args[]) {
if (!FLAG_wasm_inlining) { if (!FLAG_wasm_inlining) {
DoReturnCall(decoder, DoReturnCall(decoder, kCallRef, 0, NullCheckFor(func_ref.type), func_ref,
CallInfo::CallRef(func_ref, NullCheckFor(func_ref.type)), sig, sig_index, args);
sig, args);
return; return;
} }
...@@ -738,15 +740,15 @@ class WasmGraphBuildingInterface { ...@@ -738,15 +740,15 @@ class WasmGraphBuildingInterface {
builder_->SetControl(success_control); builder_->SetControl(success_control);
ssa_env_->control = success_control; ssa_env_->control = success_control;
DoReturnCall(decoder, CallInfo::CallDirect(expected_function_index), sig, DoReturnCall(decoder, kCallDirect, 0, CheckForNull::kWithoutNullCheck,
Value{nullptr, kWasmBottom}, sig, expected_function_index,
args); args);
builder_->SetEffectControl(initial_effect, failure_control); builder_->SetEffectControl(initial_effect, failure_control);
ssa_env_->effect = initial_effect; ssa_env_->effect = initial_effect;
ssa_env_->control = failure_control; ssa_env_->control = failure_control;
DoReturnCall(decoder, DoReturnCall(decoder, kCallRef, 0, NullCheckFor(func_ref.type), func_ref,
CallInfo::CallRef(func_ref, NullCheckFor(func_ref.type)), sig, sig, sig_index, args);
args);
} }
void BrOnNull(FullDecoder* decoder, const Value& ref_object, uint32_t depth) { void BrOnNull(FullDecoder* decoder, const Value& ref_object, uint32_t depth) {
...@@ -1515,102 +1517,36 @@ class WasmGraphBuildingInterface { ...@@ -1515,102 +1517,36 @@ class WasmGraphBuildingInterface {
return result; return result;
} }
class CallInfo { void DoCall(FullDecoder* decoder, CallMode call_mode, uint32_t table_index,
public: CheckForNull null_check, TFNode* caller_node,
enum CallMode { kCallDirect, kCallIndirect, kCallRef }; const FunctionSig* sig, uint32_t sig_index, const Value args[],
Value returns[]) {
static CallInfo CallDirect(uint32_t callee_index) {
return {kCallDirect, callee_index, nullptr, 0,
CheckForNull::kWithoutNullCheck};
}
static CallInfo CallIndirect(const Value& index_value, uint32_t table_index,
uint32_t sig_index) {
return {kCallIndirect, sig_index, &index_value, table_index,
CheckForNull::kWithoutNullCheck};
}
static CallInfo CallRef(const Value& funcref_value,
CheckForNull null_check) {
return {kCallRef, 0, &funcref_value, 0, null_check};
}
CallMode call_mode() { return call_mode_; }
uint32_t sig_index() {
DCHECK_EQ(call_mode_, kCallIndirect);
return callee_or_sig_index_;
}
uint32_t callee_index() {
DCHECK_EQ(call_mode_, kCallDirect);
return callee_or_sig_index_;
}
CheckForNull null_check() {
DCHECK_EQ(call_mode_, kCallRef);
return null_check_;
}
const Value* index_or_callee_value() {
DCHECK_NE(call_mode_, kCallDirect);
return index_or_callee_value_;
}
uint32_t table_index() {
DCHECK_EQ(call_mode_, kCallIndirect);
return table_index_;
}
private:
CallInfo(CallMode call_mode, uint32_t callee_or_sig_index,
const Value* index_or_callee_value, uint32_t table_index,
CheckForNull null_check)
: call_mode_(call_mode),
callee_or_sig_index_(callee_or_sig_index),
index_or_callee_value_(index_or_callee_value),
table_index_(table_index),
null_check_(null_check) {}
CallMode call_mode_;
uint32_t callee_or_sig_index_;
const Value* index_or_callee_value_;
uint32_t table_index_;
CheckForNull null_check_;
};
void DoCall(FullDecoder* decoder, CallInfo call_info, const FunctionSig* sig,
const Value args[], Value returns[]) {
size_t param_count = sig->parameter_count(); size_t param_count = sig->parameter_count();
size_t return_count = sig->return_count(); size_t return_count = sig->return_count();
NodeVector arg_nodes(param_count + 1); NodeVector arg_nodes(param_count + 1);
base::SmallVector<TFNode*, 1> return_nodes(return_count); base::SmallVector<TFNode*, 1> return_nodes(return_count);
arg_nodes[0] = (call_info.call_mode() == CallInfo::kCallDirect) arg_nodes[0] = caller_node;
? nullptr
: call_info.index_or_callee_value()->node;
for (size_t i = 0; i < param_count; ++i) { for (size_t i = 0; i < param_count; ++i) {
arg_nodes[i + 1] = args[i].node; arg_nodes[i + 1] = args[i].node;
} }
switch (call_info.call_mode()) { switch (call_mode) {
case CallInfo::kCallIndirect: case kCallIndirect:
CheckForException( CheckForException(
decoder, builder_->CallIndirect( decoder, builder_->CallIndirect(
call_info.table_index(), call_info.sig_index(), table_index, sig_index, base::VectorOf(arg_nodes),
base::VectorOf(arg_nodes),
base::VectorOf(return_nodes), decoder->position())); base::VectorOf(return_nodes), decoder->position()));
break; break;
case CallInfo::kCallDirect: case kCallDirect:
CheckForException( CheckForException(
decoder, builder_->CallDirect( decoder, builder_->CallDirect(sig_index, base::VectorOf(arg_nodes),
call_info.callee_index(), base::VectorOf(arg_nodes), base::VectorOf(return_nodes),
base::VectorOf(return_nodes), decoder->position())); decoder->position()));
break; break;
case CallInfo::kCallRef: case kCallRef:
CheckForException( CheckForException(
decoder, decoder, builder_->CallRef(sig_index, base::VectorOf(arg_nodes),
builder_->CallRef(sig, base::VectorOf(arg_nodes), base::VectorOf(return_nodes), null_check,
base::VectorOf(return_nodes), decoder->position()));
call_info.null_check(), decoder->position()));
break; break;
} }
for (size_t i = 0; i < return_count; ++i) { for (size_t i = 0; i < return_count; ++i) {
...@@ -1621,20 +1557,17 @@ class WasmGraphBuildingInterface { ...@@ -1621,20 +1557,17 @@ class WasmGraphBuildingInterface {
LoadContextIntoSsa(ssa_env_); LoadContextIntoSsa(ssa_env_);
} }
void DoReturnCall(FullDecoder* decoder, CallInfo call_info, void DoReturnCall(FullDecoder* decoder, CallMode call_mode,
const FunctionSig* sig, const Value args[]) { uint32_t table_index, CheckForNull null_check,
Value index_or_caller_value, const FunctionSig* sig,
uint32_t sig_index, const Value args[]) {
size_t arg_count = sig->parameter_count(); size_t arg_count = sig->parameter_count();
ValueVector arg_values(arg_count + 1); ValueVector arg_values(arg_count + 1);
if (call_info.call_mode() == CallInfo::kCallDirect) { arg_values[0] = index_or_caller_value;
arg_values[0].node = nullptr; for (uint32_t i = 0; i < arg_count; i++) {
} else { arg_values[i + 1] = args[i];
arg_values[0] = *call_info.index_or_callee_value();
// This is not done by copy assignment.
arg_values[0].node = call_info.index_or_callee_value()->node;
} }
std::memcpy(arg_values.data() + 1, args, arg_count * sizeof(Value));
if (FLAG_wasm_loop_unrolling) { if (FLAG_wasm_loop_unrolling) {
BuildNestedLoopExits(decoder, decoder->control_depth(), false, BuildNestedLoopExits(decoder, decoder->control_depth(), false,
arg_values); arg_values);
...@@ -1643,25 +1576,23 @@ class WasmGraphBuildingInterface { ...@@ -1643,25 +1576,23 @@ class WasmGraphBuildingInterface {
NodeVector arg_nodes(arg_count + 1); NodeVector arg_nodes(arg_count + 1);
GetNodes(arg_nodes.data(), base::VectorOf(arg_values)); GetNodes(arg_nodes.data(), base::VectorOf(arg_values));
switch (call_info.call_mode()) { switch (call_mode) {
case CallInfo::kCallIndirect: case kCallIndirect:
CheckForException(decoder, CheckForException(
builder_->ReturnCallIndirect( decoder, builder_->ReturnCallIndirect(table_index, sig_index,
call_info.table_index(), call_info.sig_index(),
base::VectorOf(arg_nodes), decoder->position()));
break;
case CallInfo::kCallDirect:
CheckForException(decoder,
builder_->ReturnCall(call_info.callee_index(),
base::VectorOf(arg_nodes), base::VectorOf(arg_nodes),
decoder->position())); decoder->position()));
break; break;
case CallInfo::kCallRef: case kCallDirect:
CheckForException( CheckForException(
decoder, builder_->ReturnCallRef(sig, base::VectorOf(arg_nodes), decoder, builder_->ReturnCall(sig_index, base::VectorOf(arg_nodes),
call_info.null_check(),
decoder->position())); decoder->position()));
break; break;
case kCallRef:
CheckForException(decoder, builder_->ReturnCallRef(
sig_index, base::VectorOf(arg_nodes),
null_check, decoder->position()));
break;
} }
} }
......
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