Commit 448af14a authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ

[maglev] Fix uninitialised next_use in deopts

Deopt InputLocation next_use fields are not initialised, so if a deopt
is the last use of a node we won't release it. Fix this by initialising
the input location array. Also add a DCHECK to verify that register
assignments match what registers a node thinks it's in.

Bug: v8:7700
Change-Id: I4003a027489cf8eeef7c4e60fa64f72cebd2c4e8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3657438Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80690}
parent 7926e5d2
...@@ -386,14 +386,11 @@ DeoptInfo::DeoptInfo(Zone* zone, const MaglevCompilationUnit& compilation_unit, ...@@ -386,14 +386,11 @@ DeoptInfo::DeoptInfo(Zone* zone, const MaglevCompilationUnit& compilation_unit,
state(state), state(state),
input_locations(zone->NewArray<InputLocation>( input_locations(zone->NewArray<InputLocation>(
GetInputLocationsArraySize(compilation_unit, state))) { GetInputLocationsArraySize(compilation_unit, state))) {
// Default initialise if we're printing the graph, to avoid printing junk // Initialise InputLocations so that they correctly don't have a next use id.
// values.
if (FLAG_print_maglev_graph) {
for (size_t i = 0; i < GetInputLocationsArraySize(compilation_unit, state); for (size_t i = 0; i < GetInputLocationsArraySize(compilation_unit, state);
++i) { ++i) {
new (&input_locations[i]) InputLocation(); new (&input_locations[i]) InputLocation();
} }
}
} }
// --- // ---
......
...@@ -806,6 +806,7 @@ class ValueNode : public Node { ...@@ -806,6 +806,7 @@ class ValueNode : public Node {
end_id_ = id; end_id_ = id;
*last_uses_next_use_id_ = id; *last_uses_next_use_id_ = id;
last_uses_next_use_id_ = input_location->get_next_use_id_address(); last_uses_next_use_id_ = input_location->get_next_use_id_address();
DCHECK_EQ(*last_uses_next_use_id_, kInvalidNodeId);
} }
struct LiveRange { struct LiveRange {
...@@ -873,6 +874,14 @@ class ValueNode : public Node { ...@@ -873,6 +874,14 @@ class ValueNode : public Node {
} }
return registers_with_result_ != kEmptyRegList; return registers_with_result_ != kEmptyRegList;
} }
bool is_in_register(Register reg) const {
DCHECK(!use_double_register());
return registers_with_result_.has(reg);
}
bool is_in_register(DoubleRegister reg) const {
DCHECK(use_double_register());
return double_registers_with_result_.has(reg);
}
compiler::InstructionOperand allocation() const { compiler::InstructionOperand allocation() const {
if (has_register()) { if (has_register()) {
......
...@@ -446,6 +446,8 @@ void StraightForwardRegisterAllocator::AllocateNode(Node* node) { ...@@ -446,6 +446,8 @@ void StraightForwardRegisterAllocator::AllocateNode(Node* node) {
PrintLiveRegs(); PrintLiveRegs();
printing_visitor_->os() << "\n"; printing_visitor_->os() << "\n";
} }
VerifyRegisterState();
} }
void StraightForwardRegisterAllocator::AllocateNodeResult(ValueNode* node) { void StraightForwardRegisterAllocator::AllocateNodeResult(ValueNode* node) {
...@@ -637,6 +639,8 @@ void StraightForwardRegisterAllocator::AllocateControlNode(ControlNode* node, ...@@ -637,6 +639,8 @@ void StraightForwardRegisterAllocator::AllocateControlNode(ControlNode* node,
printing_visitor_->Process(node, printing_visitor_->Process(node,
ProcessingState(compilation_info_, block_it_)); ProcessingState(compilation_info_, block_it_));
} }
VerifyRegisterState();
} }
void StraightForwardRegisterAllocator::TryAllocateToInput(Phi* phi) { void StraightForwardRegisterAllocator::TryAllocateToInput(Phi* phi) {
...@@ -806,6 +810,22 @@ void StraightForwardRegisterAllocator::VerifyInputs(NodeBase* node) { ...@@ -806,6 +810,22 @@ void StraightForwardRegisterAllocator::VerifyInputs(NodeBase* node) {
#endif #endif
} }
void StraightForwardRegisterAllocator::VerifyRegisterState() {
#ifdef DEBUG
for (Register reg : general_registers_.used()) {
ValueNode* node = general_registers_.GetValue(reg);
DCHECK(node->is_in_register(reg));
}
for (DoubleRegister reg : double_registers_.used()) {
ValueNode* node = double_registers_.GetValue(reg);
if (!node->is_in_register(reg)) {
FATAL("Node n%d doesn't think it is in register %s",
graph_labeller()->NodeId(node), RegisterName(reg));
}
}
#endif
}
void StraightForwardRegisterAllocator::SpillRegisters() { void StraightForwardRegisterAllocator::SpillRegisters() {
auto spill = [&](auto reg, ValueNode* node) { Spill(node); }; auto spill = [&](auto reg, ValueNode* node) { Spill(node); };
general_registers_.ForEachUsedRegister(spill); general_registers_.ForEachUsedRegister(spill);
......
...@@ -136,6 +136,7 @@ class StraightForwardRegisterAllocator { ...@@ -136,6 +136,7 @@ class StraightForwardRegisterAllocator {
void TryAllocateToInput(Phi* phi); void TryAllocateToInput(Phi* phi);
void VerifyInputs(NodeBase* node); void VerifyInputs(NodeBase* node);
void VerifyRegisterState();
void AddMoveBeforeCurrentNode(ValueNode* node, void AddMoveBeforeCurrentNode(ValueNode* node,
compiler::InstructionOperand source, compiler::InstructionOperand source,
......
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