Commit 8bba185f authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ

[maglev] Initialize loop merge to uninitialized predecessor

Explicitly initialize the loop merge's back-edge predecessor to a
specfic "uninitialized" value, distinct from nullptr (which marks dead
loops) and done in both debug and release modes.

Bug: v8:7700
Change-Id: I6a845cc4dbd7da75954f78607e69a5d4e2ec1ec7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3645114Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80525}
parent dd74a023
...@@ -85,8 +85,8 @@ class MaglevGraphBuilder { ...@@ -85,8 +85,8 @@ class MaglevGraphBuilder {
BasicBlockRef* old_jump_targets = jump_targets_[offset].Reset(); BasicBlockRef* old_jump_targets = jump_targets_[offset].Reset();
while (old_jump_targets != nullptr) { while (old_jump_targets != nullptr) {
BasicBlock* predecessor = merge_state.predecessor_at(predecessor_index); BasicBlock* predecessor = merge_state.predecessor_at(predecessor_index);
if (predecessor == nullptr) { if (predecessor == MergePointInterpreterFrameState::kDeadPredecessor) {
// We can have null predecessors if the predecessor is dead. // We might have dead predecessors.
predecessor_index--; predecessor_index--;
continue; continue;
} }
......
...@@ -398,7 +398,8 @@ void MaglevPrintingVisitor::Process(Phi* phi, const ProcessingState& state) { ...@@ -398,7 +398,8 @@ void MaglevPrintingVisitor::Process(Phi* phi, const ProcessingState& state) {
// moves). // moves).
for (int i = 0; i < phi->input_count(); ++i) { for (int i = 0; i < phi->input_count(); ++i) {
if (i > 0) os_ << ", "; if (i > 0) os_ << ", ";
if (state.block()->predecessor_at(i) == nullptr) { if (state.block()->predecessor_at(i) ==
MergePointInterpreterFrameState::kDeadPredecessor) {
os_ << "<dead>"; os_ << "<dead>";
} else { } else {
os_ << PrintNodeLabel(graph_labeller, phi->input(i).node()); os_ << PrintNodeLabel(graph_labeller, phi->input(i).node());
......
...@@ -268,9 +268,7 @@ class MergePointInterpreterFrameState { ...@@ -268,9 +268,7 @@ class MergePointInterpreterFrameState {
}); });
DCHECK(!frame_state_.liveness()->AccumulatorIsLive()); DCHECK(!frame_state_.liveness()->AccumulatorIsLive());
#ifdef DEBUG predecessors_[0] = uninitialized_predecessor();
predecessors_[0] = nullptr;
#endif
} }
// Merges an unmerged framestate with a possibly merged framestate into |this| // Merges an unmerged framestate with a possibly merged framestate into |this|
...@@ -299,7 +297,7 @@ class MergePointInterpreterFrameState { ...@@ -299,7 +297,7 @@ class MergePointInterpreterFrameState {
const InterpreterFrameState& loop_end_state, const InterpreterFrameState& loop_end_state,
BasicBlock* loop_end_block, int merge_offset) { BasicBlock* loop_end_block, int merge_offset) {
DCHECK_EQ(predecessors_so_far_, predecessor_count_); DCHECK_EQ(predecessors_so_far_, predecessor_count_);
DCHECK_NULL(predecessors_[0]); DCHECK_EQ(predecessors_[0], uninitialized_predecessor());
predecessors_[0] = loop_end_block; predecessors_[0] = loop_end_block;
frame_state_.ForEachValue( frame_state_.ForEachValue(
...@@ -331,7 +329,7 @@ class MergePointInterpreterFrameState { ...@@ -331,7 +329,7 @@ class MergePointInterpreterFrameState {
// JumpLoop has been early terminated with a deopt). // JumpLoop has been early terminated with a deopt).
void MergeDeadLoop() { void MergeDeadLoop() {
DCHECK_EQ(predecessors_so_far_, predecessor_count_); DCHECK_EQ(predecessors_so_far_, predecessor_count_);
DCHECK_NULL(predecessors_[0]); DCHECK_EQ(predecessors_[0], uninitialized_predecessor());
predecessors_[0] = kDeadPredecessor; predecessors_[0] = kDeadPredecessor;
} }
...@@ -361,7 +359,8 @@ class MergePointInterpreterFrameState { ...@@ -361,7 +359,8 @@ class MergePointInterpreterFrameState {
DCHECK_EQ(predecessors_so_far_, predecessor_count_); DCHECK_EQ(predecessors_so_far_, predecessor_count_);
// If there is only one predecessor, and it's not set, then this is a loop // If there is only one predecessor, and it's not set, then this is a loop
// merge with no forward control flow entering it. // merge with no forward control flow entering it.
return predecessor_count_ == 1 && predecessors_[0] == nullptr; return predecessor_count_ == 1 &&
predecessors_[0] == uninitialized_predecessor();
} }
private: private:
...@@ -369,6 +368,13 @@ class MergePointInterpreterFrameState { ...@@ -369,6 +368,13 @@ class MergePointInterpreterFrameState {
const MaglevCompilationUnit& info, const MaglevCompilationUnit& info,
const MergePointInterpreterFrameState& state); const MergePointInterpreterFrameState& state);
// Create an uninitialized value sentinel for loop merges. This is distinct
// from kDeadPredecessor, which is for loops that were reached, but were dead.
// TODO(leszeks): Do this in a way that isn't UB.
static BasicBlock* uninitialized_predecessor() {
return reinterpret_cast<BasicBlock*>(0x100b);
}
ValueNode* FromInt32ToTagged(MaglevCompilationUnit& compilation_unit, ValueNode* FromInt32ToTagged(MaglevCompilationUnit& compilation_unit,
ValueNode* value) { ValueNode* value) {
DCHECK_EQ(value->properties().value_representation(), DCHECK_EQ(value->properties().value_representation(),
...@@ -440,7 +446,7 @@ class MergePointInterpreterFrameState { ...@@ -440,7 +446,7 @@ class MergePointInterpreterFrameState {
// If the merged node is null, this is a pre-created loop header merge // If the merged node is null, this is a pre-created loop header merge
// frame will null values for anything that isn't a loop Phi. // frame will null values for anything that isn't a loop Phi.
if (merged == nullptr) { if (merged == nullptr) {
DCHECK_NULL(predecessors_[0]); DCHECK_EQ(predecessors_[0], uninitialized_predecessor());
DCHECK_EQ(predecessors_so_far_, 1); DCHECK_EQ(predecessors_so_far_, 1);
return unmerged; return unmerged;
} }
...@@ -489,7 +495,7 @@ class MergePointInterpreterFrameState { ...@@ -489,7 +495,7 @@ class MergePointInterpreterFrameState {
// If the merged node is null, this is a pre-created loop header merge // If the merged node is null, this is a pre-created loop header merge
// frame with null values for anything that isn't a loop Phi. // frame with null values for anything that isn't a loop Phi.
if (merged == nullptr) { if (merged == nullptr) {
DCHECK_NULL(predecessors_[0]); DCHECK_EQ(predecessors_[0], uninitialized_predecessor());
DCHECK_EQ(predecessors_so_far_, 1); DCHECK_EQ(predecessors_so_far_, 1);
return; return;
} }
......
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