Commit 376c7b61 authored by Jakob Gruber's avatar Jakob Gruber Committed by Commit Bot

[wasm] Update the stack check and remove WasmStackCheckMatcher

The matcher used to be needed to avoid first moving rsp to an
allocated register for LoadStackPointer. This is no longer the case
with the new stack check structure based on StackPointerGreaterThan.
This CL updates the wasm stack check and removes now-unneeded
matchers.

The generated stack check code remains unchanged from before:

// Load the stack limit through the instance then compare against rsp.
REX.W movq rcx,[rbp-0x10]
REX.W movq rcx,[rcx+0x2f]
REX.W cmpq rsp,[rcx]

// And on ia32:
mov ecx,[ebp-0x8]
mov ecx,[ecx+0x17]
cmp esp,[ecx]

Bug: v8:9534
Change-Id: I9240ad922d19d498a2661c143b12d629ac14d093
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1748733
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63165}
parent 56a6f0a1
...@@ -1311,18 +1311,6 @@ void VisitWordCompare(InstructionSelector* selector, Node* node, ...@@ -1311,18 +1311,6 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
return; return;
} }
} }
WasmStackCheckMatcher<Int32BinopMatcher, IrOpcode::kUint32LessThan> wasm_m(
node);
if (wasm_m.Matched()) {
// This is a wasm stack check. By structure, we know that we can use the
// stack pointer directly, as wasm code does not modify the stack at points
// where stack checks are performed.
Node* left = node->InputAt(0);
LocationOperand esp(InstructionOperand::EXPLICIT, LocationOperand::REGISTER,
InstructionSequence::DefaultRepresentation(),
RegisterCode::kRegCode_esp);
return VisitCompareWithMemoryOperand(selector, kIA32Cmp, left, esp, cont);
}
VisitWordCompare(selector, node, kIA32Cmp, cont); VisitWordCompare(selector, node, kIA32Cmp, cont);
} }
......
...@@ -1883,18 +1883,6 @@ void VisitWord64Compare(InstructionSelector* selector, Node* node, ...@@ -1883,18 +1883,6 @@ void VisitWord64Compare(InstructionSelector* selector, Node* node,
return; return;
} }
} }
WasmStackCheckMatcher<Int64BinopMatcher, IrOpcode::kUint64LessThan> wasm_m(
node);
if (wasm_m.Matched()) {
// This is a wasm stack check. By structure, we know that we can use the
// stack pointer directly, as wasm code does not modify the stack at points
// where stack checks are performed.
Node* left = node->InputAt(0);
LocationOperand rsp(InstructionOperand::EXPLICIT, LocationOperand::REGISTER,
InstructionSequence::DefaultRepresentation(),
RegisterCode::kRegCode_rsp);
return VisitCompareWithMemoryOperand(selector, kX64Cmp, left, rsp, cont);
}
VisitWordCompare(selector, node, kX64Cmp, cont); VisitWordCompare(selector, node, kX64Cmp, cont);
} }
......
...@@ -761,34 +761,6 @@ struct V8_EXPORT_PRIVATE DiamondMatcher ...@@ -761,34 +761,6 @@ struct V8_EXPORT_PRIVATE DiamondMatcher
Node* if_false_; Node* if_false_;
}; };
template <class BinopMatcher, IrOpcode::Value expected_opcode>
struct WasmStackCheckMatcher {
explicit WasmStackCheckMatcher(Node* compare) : compare_(compare) {}
bool Matched() {
if (compare_->opcode() != expected_opcode) return false;
BinopMatcher m(compare_);
return MatchedInternal(m.left(), m.right());
}
private:
bool MatchedInternal(const typename BinopMatcher::LeftMatcher& l,
const typename BinopMatcher::RightMatcher& r) {
// In wasm, the stack check is performed by loading the value given by
// the address of a field stored in the instance object. That object is
// passed as a parameter.
if (l.IsLoad() && r.IsLoadStackPointer()) {
LoadMatcher<LoadMatcher<NodeMatcher>> mleft(l.node());
if (mleft.object().IsLoad() && mleft.index().Is(0) &&
mleft.object().object().IsParameter()) {
return true;
}
}
return false;
}
Node* compare_;
};
template <class BinopMatcher, IrOpcode::Value expected_opcode> template <class BinopMatcher, IrOpcode::Value expected_opcode>
struct StackCheckMatcher { struct StackCheckMatcher {
StackCheckMatcher(Isolate* isolate, Node* compare) StackCheckMatcher(Isolate* isolate, Node* compare)
......
...@@ -328,10 +328,6 @@ void WasmGraphBuilder::StackCheck(wasm::WasmCodePosition position, ...@@ -328,10 +328,6 @@ void WasmGraphBuilder::StackCheck(wasm::WasmCodePosition position,
if (effect == nullptr) effect = effect_; if (effect == nullptr) effect = effect_;
if (control == nullptr) control = control_; if (control == nullptr) control = control_;
// This instruction sequence is matched in the instruction selector to
// load the stack pointer directly on some platforms. Hence, when modifying
// please also fix WasmStackCheckMatcher in node-matchers.h
Node* limit_address = graph()->NewNode( Node* limit_address = graph()->NewNode(
mcgraph()->machine()->Load(MachineType::Pointer()), instance_node_.get(), mcgraph()->machine()->Load(MachineType::Pointer()), instance_node_.get(),
mcgraph()->Int32Constant(WASM_INSTANCE_OBJECT_OFFSET(StackLimitAddress)), mcgraph()->Int32Constant(WASM_INSTANCE_OBJECT_OFFSET(StackLimitAddress)),
...@@ -340,10 +336,9 @@ void WasmGraphBuilder::StackCheck(wasm::WasmCodePosition position, ...@@ -340,10 +336,9 @@ void WasmGraphBuilder::StackCheck(wasm::WasmCodePosition position,
mcgraph()->machine()->Load(MachineType::Pointer()), limit_address, mcgraph()->machine()->Load(MachineType::Pointer()), limit_address,
mcgraph()->IntPtrConstant(0), limit_address, *control); mcgraph()->IntPtrConstant(0), limit_address, *control);
*effect = limit; *effect = limit;
Node* pointer = graph()->NewNode(mcgraph()->machine()->LoadStackPointer());
Node* check = Node* check =
graph()->NewNode(mcgraph()->machine()->UintLessThan(), limit, pointer); graph()->NewNode(mcgraph()->machine()->StackPointerGreaterThan(), limit);
Diamond stack_check(graph(), mcgraph()->common(), check, BranchHint::kTrue); Diamond stack_check(graph(), mcgraph()->common(), check, BranchHint::kTrue);
stack_check.Chain(*control); stack_check.Chain(*control);
......
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