Commit 7cdd1ed3 authored by Victor Gomes's avatar Victor Gomes Committed by V8 LUCI CQ

[maglev] Workaround for generator resume middle loop issue

The current abort will crash if the generator is created by the
interpreter and resumed by the maglevved code.

This current workaround is not ideal since it can introduce
a deopt-reopt loop.

Bug: v8:7700, v8:13109
Change-Id: I7db71a896711255d866ace98eddde85538aa2903
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3879228
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Reviewed-by: 's avatarJakob Linke <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83054}
parent 854a3728
...@@ -55,6 +55,7 @@ namespace internal { ...@@ -55,6 +55,7 @@ namespace internal {
V(OutOfBounds, "out of bounds") \ V(OutOfBounds, "out of bounds") \
V(Overflow, "overflow") \ V(Overflow, "overflow") \
V(Smi, "Smi") \ V(Smi, "Smi") \
V(SuspendGeneratorIsDead, "SuspendGenerator is in a dead branch") \
V(TransitionedToMonomorphicIC, "IC transitioned to monomorphic") \ V(TransitionedToMonomorphicIC, "IC transitioned to monomorphic") \
V(TransitionedToMegamorphicIC, "IC transitioned to megamorphic") \ V(TransitionedToMegamorphicIC, "IC transitioned to megamorphic") \
V(Unknown, "(unknown)") \ V(Unknown, "(unknown)") \
......
...@@ -209,11 +209,8 @@ class MaglevGraphBuilder { ...@@ -209,11 +209,8 @@ class MaglevGraphBuilder {
// live again. // live again.
// //
// So, manually advance the iterator to the resume, go through the motions // So, manually advance the iterator to the resume, go through the motions
// of processing the merge state, but immediately emit an abort (which // of processing the merge state, but immediately emit an unconditional
// also kills the resume). // deopt (which also kills the resume).
//
// TODO(leszeks): Instead of emitting an Abort, we could shrink the
// generator switch, removing this resume as an option.
iterator_.Advance(); iterator_.Advance();
DCHECK_EQ(iterator_.current_bytecode(), DCHECK_EQ(iterator_.current_bytecode(),
interpreter::Bytecode::kResumeGenerator); interpreter::Bytecode::kResumeGenerator);
...@@ -221,7 +218,12 @@ class MaglevGraphBuilder { ...@@ -221,7 +218,12 @@ class MaglevGraphBuilder {
DCHECK_EQ(NumPredecessors(resume_offset), 1); DCHECK_EQ(NumPredecessors(resume_offset), 1);
ProcessMergePoint(resume_offset); ProcessMergePoint(resume_offset);
StartNewBlock(resume_offset); StartNewBlock(resume_offset);
BuildAbort(AbortReason::kInvalidParametersAndRegistersInGenerator); // TODO(v8:7700): This approach is not ideal. We can create a deopt-reopt
// loop: the interpreted code runs, creates a generator while feedback is
// still not yet allocated, then suspends the generator, tiers up to
// maglev, and reaches this deopt. We then deopt, but since the generator
// is never created again, we re-opt without the suspend part and we loop!
EmitUnconditionalDeopt(DeoptimizeReason::kSuspendGeneratorIsDead);
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