[compiler] Inline GeneratorPrototypeNext in Turbofan [v8/v8 : main]
0 views
Skip to first unread message
Victor Gomes (Gerrit)
Mar 10, 2026, 9:45:18 AMMar 10
to Darius Mercadier, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Darius Mercadier
Victor Gomes voted and added 1 comment![]( "Open in Gerrit")
Votes added by Victor Gomes
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Victor Gomes . resolved
Small percentage, but slowly going up! PTAL!
Related details
Attention is currently required from:
- Darius Mercadier
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Darius Mercadier (Gerrit)
Mar 10, 2026, 10:11:03 AMMar 10
to Victor Gomes, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Victor Gomes
Darius Mercadier voted and added 5 comments![]( "Open in Gerrit")
Votes added by Darius Mercadier
| Code-Review | +1 |
5 comments
Patchset-level comments
Darius Mercadier . resolved
LGTM % comments.
This is just horrible to read, good thing that we're getting rid of SoN :D
Commit Message
File src/compiler/js-call-reducer.cc
Line 6764, Patchset 2 (Latest):
// Turboshaft GraphBuilder explicitly drops the exception edge for
// JSStackCheck (which is part of the inlined generator initialization) as
// it's not expected to throw under optimized flat frames on 64-bit
// platforms. However, on ia32 with dynamic argument pushes, this triggers an
// uncatchable StackOverflow crash when the stack guard is hit inside the
// inline. We disable the optimization here to prevent inlining JSStackCheck.Darius Mercadier . unresolved
I don't understand. Which JSStackCheck is this about exactly? How can it throw exactly?
Line 6842, Patchset 2 (Latest):
Node* resume_node = e_not_closed = graph()->NewNode(
common()->Call(descriptor),
jsgraph()->HeapConstantNoHole(callable.code()), value, receiver, context,
n.frame_state(), e_not_closed, if_not_closed);
Darius Mercadier . unresolved
Don't you need to check for exceptions? The builtin does.
Line 6761, Patchset 2 (Latest):
// https://tc39.es/ecma262/#sec-generator.prototype.next
Reduction JSCallReducer::ReduceGeneratorPrototypeNext(Node* node) {
#ifdef V8_TARGET_ARCH_IA32
// Turboshaft GraphBuilder explicitly drops the exception edge for
// JSStackCheck (which is part of the inlined generator initialization) as
// it's not expected to throw under optimized flat frames on 64-bit
// platforms. However, on ia32 with dynamic argument pushes, this triggers an
// uncatchable StackOverflow crash when the stack guard is hit inside the
// inline. We disable the optimization here to prevent inlining JSStackCheck.
return NoChange();
#else
JSCallNode n(node);
CallParameters const& p = n.Parameters();
Node* receiver = n.receiver();
Node* value = n.ArgumentOrUndefined(0, jsgraph());
Node* context = n.context();
Effect effect = n.effect();
Control control = n.control();
if (p.speculation_mode() != SpeculationMode::kAllowSpeculation) {
return NoChange();
}
MapInference inference(broker(), receiver, effect);
if (!inference.HaveMaps() ||
!inference.AllOfInstanceTypesAre(JS_GENERATOR_OBJECT_TYPE)) {
return NoChange();
}
inference.RelyOnMapsPreferStability(dependencies(), jsgraph(), &effect,
control, p.feedback());
// Check if the {receiver} is running or already closed.
Node* receiver_continuation = effect =
graph()->NewNode(simplified()->LoadField(
AccessBuilder::ForJSGeneratorObjectContinuation()),
receiver, effect, control);
Node* closed_constant =
jsgraph()->ConstantNoHole(JSGeneratorObject::kGeneratorClosed);
Node* check_closed = graph()->NewNode(simplified()->NumberEqual(),
receiver_continuation, closed_constant);
Node* branch_closed = graph()->NewNode(common()->Branch(BranchHint::kFalse),
check_closed, control);
Node* if_closed = graph()->NewNode(common()->IfTrue(), branch_closed);
Node* if_not_closed = graph()->NewNode(common()->IfFalse(), branch_closed);
// If closed: return {value: undefined, done: true}
Node* e_closed = effect;
Node* v_closed = e_closed = graph()->NewNode(
javascript()->CreateIterResultObject(), jsgraph()->UndefinedConstant(),
jsgraph()->TrueConstant(), context, e_closed);
// If not closed, check if executing.
Node* executing_constant =
jsgraph()->ConstantNoHole(JSGeneratorObject::kGeneratorExecuting);
Node* check_executing = graph()->NewNode(
simplified()->NumberEqual(), receiver_continuation, executing_constant);
// A throwing/deopting check is better here, since executing generators are
// extremely rare. We can just deoptimize if it's executing.
Node* e_not_closed = graph()->NewNode(
simplified()->CheckIf(DeoptimizeReason::kWrongCallTarget, p.feedback()),
graph()->NewNode(simplified()->BooleanNot(), check_executing), effect,
if_not_closed);
// Set resume_mode to kNext.
e_not_closed = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForJSGeneratorObjectResumeMode()),
receiver, jsgraph()->ConstantNoHole(JSGeneratorObject::kNext),
e_not_closed, if_not_closed);
// Emit Call to ResumeGeneratorTrampoline
Callable callable =
Builtins::CallableFor(isolate(), Builtin::kResumeGeneratorTrampoline);
CallDescriptor* descriptor = Linkage::GetStubCallDescriptor(
graph()->zone(), callable.descriptor(),
callable.descriptor().GetStackParameterCount(),
CallDescriptor::kNeedsFrameState);
Node* resume_node = e_not_closed = graph()->NewNode(
common()->Call(descriptor),
jsgraph()->HeapConstantNoHole(callable.code()), value, receiver, context,
n.frame_state(), e_not_closed, if_not_closed);
// The generator could have returned or yielded. JSResumeGenerator returns the
// yielded value. Check the generator's state again to see if it's executing
// (meaning it naturally returned).
Node* new_continuation = e_not_closed =
graph()->NewNode(simplified()->LoadField(
AccessBuilder::ForJSGeneratorObjectContinuation()),
receiver, e_not_closed, if_not_closed);
Node* check_returned = graph()->NewNode(simplified()->NumberEqual(),
new_continuation, executing_constant);
Node* branch_returned = graph()->NewNode(common()->Branch(BranchHint::kFalse),
check_returned, if_not_closed);
Node* if_returned = graph()->NewNode(common()->IfTrue(), branch_returned);
Node* if_yielded = graph()->NewNode(common()->IfFalse(), branch_returned);
// If it returned, close it.
Node* e_returned =
graph()->NewNode(simplified()->StoreField(
AccessBuilder::ForJSGeneratorObjectContinuation()),
receiver, closed_constant, e_not_closed, if_returned);
// Wrap the returned value in an IterResult object: {value: resume_node, done:
// true}
Node* v_returned = e_returned =
graph()->NewNode(javascript()->CreateIterResultObject(), resume_node,
jsgraph()->TrueConstant(), context, e_returned);
// If it yielded, the value is already wrapped by the generator.
Node* v_yielded = resume_node;
Node* e_yielded = e_not_closed;
// Merge the returned and yielded paths.
Node* control_after_resume =
graph()->NewNode(common()->Merge(2), if_returned, if_yielded);
Node* e_after_resume = graph()->NewNode(common()->EffectPhi(2), e_returned,
e_yielded, control_after_resume);
Node* v_after_resume =
graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
v_returned, v_yielded, control_after_resume);
// Merge the originally closed path and the running path.
control =
graph()->NewNode(common()->Merge(2), if_closed, control_after_resume);
effect = graph()->NewNode(common()->EffectPhi(2), e_closed, e_after_resume,
control);
Node* final_value =
graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
v_closed, v_after_resume, control);
ReplaceWithValue(node, final_value, effect, control);
return Replace(final_value);
#endif // V8_TARGET_ARCH_IA32
}Darius Mercadier . resolved
It's a bit unfortunate that variable names and comments don't match the builtin, it makes it harder to follow... no need to change anything though, just a thought for next time.
Related details
Attention is currently required from:
- Victor Gomes
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Victor Gomes (Gerrit)
Mar 10, 2026, 11:56:38 AMMar 10
to Darius Mercadier, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Victor Gomes voted and added 5 comments![]( "Open in Gerrit")
Votes added by Victor Gomes
| Commit-Queue | +2 |
5 comments
Patchset-level comments
Commit Message
Line 12, Patchset 2:
Darius Mercadier . resolved
Victor GomesBug: 488242428
I'm using an specific one for lazy-collections.
File src/compiler/js-call-reducer.cc
Line 6764, Patchset 2:
// Turboshaft GraphBuilder explicitly drops the exception edge for
// JSStackCheck (which is part of the inlined generator initialization) as
// it's not expected to throw under optimized flat frames on 64-bit
// platforms. However, on ia32 with dynamic argument pushes, this triggers an
// uncatchable StackOverflow crash when the stack guard is hit inside the
// inline. We disable the optimization here to prevent inlining JSStackCheck.Darius Mercadier . resolved
I don't understand. Which JSStackCheck is this about exactly? How can it throw exactly?
Victor Gomes
Sorry, I was on acid here. You caught the issue! It was fixed with the IfException node.
Line 6842, Patchset 2:
Node* resume_node = e_not_closed = graph()->NewNode(
common()->Call(descriptor),
jsgraph()->HeapConstantNoHole(callable.code()), value, receiver, context,
n.frame_state(), e_not_closed, if_not_closed);
Darius Mercadier . resolved
Don't you need to check for exceptions? The builtin does.
Victor Gomes
We do! Thanks for catching this!
Victor Gomes
I tried to update some of the names to match the builtin.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
Mar 10, 2026, 12:21:32 PMMar 10
to Victor Gomes, Darius Mercadier, V8 LUCI CQ, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Victor Gomes
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m4-mini-perf/jetstream-main.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/115a0a70090000
Related details
Attention is currently required from:
- Victor Gomes
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
Mar 10, 2026, 12:45:02 PMMar 10
to Victor Gomes, Darius Mercadier, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, v8-re...@googlegroups.com
V8 LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/compiler/js-call-reducer.cc
Insertions: 67, Deletions: 50.
@@ -6760,15 +6760,6 @@
// https://tc39.es/ecma262/#sec-generator.prototype.next
Reduction JSCallReducer::ReduceGeneratorPrototypeNext(Node* node) {
-#ifdef V8_TARGET_ARCH_IA32
- // Turboshaft GraphBuilder explicitly drops the exception edge for
- // JSStackCheck (which is part of the inlined generator initialization) as
- // it's not expected to throw under optimized flat frames on 64-bit
- // platforms. However, on ia32 with dynamic argument pushes, this triggers an
- // uncatchable StackOverflow crash when the stack guard is hit inside the
- // inline. We disable the optimization here to prevent inlining JSStackCheck.
- return NoChange();
-#else
JSCallNode n(node);
CallParameters const& p = n.Parameters();
Node* receiver = n.receiver();
@@ -6796,40 +6787,41 @@
AccessBuilder::ForJSGeneratorObjectContinuation()),
receiver, effect, control);
- Node* closed_constant =
- jsgraph()->ConstantNoHole(JSGeneratorObject::kGeneratorClosed);
+ Node* closed = jsgraph()->ConstantNoHole(JSGeneratorObject::kGeneratorClosed);
Node* check_closed = graph()->NewNode(simplified()->NumberEqual(),
- receiver_continuation, closed_constant);
+ receiver_continuation, closed);
Node* branch_closed = graph()->NewNode(common()->Branch(BranchHint::kFalse),
check_closed, control);
- Node* if_closed = graph()->NewNode(common()->IfTrue(), branch_closed);
- Node* if_not_closed = graph()->NewNode(common()->IfFalse(), branch_closed);
+ Node* if_receiverisclosed =
+ graph()->NewNode(common()->IfTrue(), branch_closed);
+ Node* if_receiverisrunning =
+ graph()->NewNode(common()->IfFalse(), branch_closed);
// If closed: return {value: undefined, done: true}
- Node* e_closed = effect;
- Node* v_closed = e_closed = graph()->NewNode(
+ Node* e_receiverisclosed = effect;
+ Node* v_receiverisclosed = e_receiverisclosed = graph()->NewNode(
javascript()->CreateIterResultObject(), jsgraph()->UndefinedConstant(),
- jsgraph()->TrueConstant(), context, e_closed);
+ jsgraph()->TrueConstant(), context, e_receiverisclosed);
// If not closed, check if executing.
- Node* executing_constant =
+ Node* executing =
jsgraph()->ConstantNoHole(JSGeneratorObject::kGeneratorExecuting);
- Node* check_executing = graph()->NewNode(
- simplified()->NumberEqual(), receiver_continuation, executing_constant);
+ Node* check_executing = graph()->NewNode(simplified()->NumberEqual(),
+ receiver_continuation, executing);
// A throwing/deopting check is better here, since executing generators are
// extremely rare. We can just deoptimize if it's executing.
- Node* e_not_closed = graph()->NewNode(
+ Node* e_receiverisrunning = graph()->NewNode(
simplified()->CheckIf(DeoptimizeReason::kWrongCallTarget, p.feedback()),
graph()->NewNode(simplified()->BooleanNot(), check_executing), effect,
- if_not_closed);
+ if_receiverisrunning);
// Set resume_mode to kNext.
- e_not_closed = graph()->NewNode(
+ e_receiverisrunning = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForJSGeneratorObjectResumeMode()),
receiver, jsgraph()->ConstantNoHole(JSGeneratorObject::kNext),
- e_not_closed, if_not_closed);
+ e_receiverisrunning, if_receiverisrunning);
// Emit Call to ResumeGeneratorTrampoline
Callable callable =
@@ -6839,64 +6831,89 @@
callable.descriptor().GetStackParameterCount(),
CallDescriptor::kNeedsFrameState);
- Node* resume_node = e_not_closed = graph()->NewNode(
+ Node* result = e_receiverisrunning = if_receiverisrunning = graph()->NewNode(
common()->Call(descriptor),
jsgraph()->HeapConstantNoHole(callable.code()), value, receiver, context,
- n.frame_state(), e_not_closed, if_not_closed);
+ n.frame_state(), e_receiverisrunning, if_receiverisrunning);
+
+ // Close the generator if there was an exception.
+ Node* if_exception = graph()->NewNode(
+ common()->IfException(), e_receiverisrunning, if_receiverisrunning);
+ Node* e_exception = if_exception;
+ if_receiverisrunning =
+ graph()->NewNode(common()->IfSuccess(), if_receiverisrunning);
+
+ e_exception =
+ graph()->NewNode(simplified()->StoreField(
+ AccessBuilder::ForJSGeneratorObjectContinuation()),
+ receiver, closed, e_exception, if_exception);
+
+ Node* on_exception = nullptr;
+ if (NodeProperties::IsExceptionalCall(node, &on_exception)) {
+ // Replace the original exception handler with our new exception path.
+ ReplaceWithValue(on_exception, if_exception, e_exception, if_exception);
+ } else {
+ // We must physically rethrow the exception.
+ Node* rethrow = e_exception = graph()->NewNode(
+ javascript()->CallRuntime(Runtime::kReThrow, 1), if_exception, context,
+ n.frame_state(), e_exception, if_exception);
+ Node* throw_node = graph()->NewNode(common()->Throw(), rethrow, rethrow);
+ MergeControlToEnd(graph(), common(), throw_node);
+ }
// The generator could have returned or yielded. JSResumeGenerator returns the
// yielded value. Check the generator's state again to see if it's executing
// (meaning it naturally returned).
- Node* new_continuation = e_not_closed =
+ Node* result_continuation = e_receiverisrunning =
graph()->NewNode(simplified()->LoadField(
AccessBuilder::ForJSGeneratorObjectContinuation()),
- receiver, e_not_closed, if_not_closed);
+ receiver, e_receiverisrunning, if_receiverisrunning);
Node* check_returned = graph()->NewNode(simplified()->NumberEqual(),
- new_continuation, executing_constant);
+ result_continuation, executing);
- Node* branch_returned = graph()->NewNode(common()->Branch(BranchHint::kFalse),
- check_returned, if_not_closed);
- Node* if_returned = graph()->NewNode(common()->IfTrue(), branch_returned);
+ Node* branch_returned =
+ graph()->NewNode(common()->Branch(BranchHint::kFalse), check_returned,
+ if_receiverisrunning);
+ Node* if_final_return = graph()->NewNode(common()->IfTrue(), branch_returned);
Node* if_yielded = graph()->NewNode(common()->IfFalse(), branch_returned);
// If it returned, close it.
- Node* e_returned =
+ Node* e_final_return =
graph()->NewNode(simplified()->StoreField(
AccessBuilder::ForJSGeneratorObjectContinuation()),
- receiver, closed_constant, e_not_closed, if_returned);
+ receiver, closed, e_receiverisrunning, if_final_return);
- // Wrap the returned value in an IterResult object: {value: resume_node, done:
+ // Wrap the returned value in an IterResult object: {value: result, done:
// true}
- Node* v_returned = e_returned =
- graph()->NewNode(javascript()->CreateIterResultObject(), resume_node,
- jsgraph()->TrueConstant(), context, e_returned);
+ Node* v_final_return = e_final_return =
+ graph()->NewNode(javascript()->CreateIterResultObject(), result,
+ jsgraph()->TrueConstant(), context, e_final_return);
// If it yielded, the value is already wrapped by the generator.
- Node* v_yielded = resume_node;
- Node* e_yielded = e_not_closed;
+ Node* v_yielded = result;
+ Node* e_yielded = e_receiverisrunning;
// Merge the returned and yielded paths.
Node* control_after_resume =
- graph()->NewNode(common()->Merge(2), if_returned, if_yielded);
- Node* e_after_resume = graph()->NewNode(common()->EffectPhi(2), e_returned,
- e_yielded, control_after_resume);
+ graph()->NewNode(common()->Merge(2), if_final_return, if_yielded);
+ Node* e_after_resume = graph()->NewNode(
+ common()->EffectPhi(2), e_final_return, e_yielded, control_after_resume);
Node* v_after_resume =
graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
- v_returned, v_yielded, control_after_resume);
+ v_final_return, v_yielded, control_after_resume);
// Merge the originally closed path and the running path.
- control =
- graph()->NewNode(common()->Merge(2), if_closed, control_after_resume);
- effect = graph()->NewNode(common()->EffectPhi(2), e_closed, e_after_resume,
- control);
+ control = graph()->NewNode(common()->Merge(2), if_receiverisclosed,
+ control_after_resume);
+ effect = graph()->NewNode(common()->EffectPhi(2), e_receiverisclosed,
+ e_after_resume, control);
Node* final_value =
graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
- v_closed, v_after_resume, control);
+ v_receiverisclosed, v_after_resume, control);
ReplaceWithValue(node, final_value, effect, control);
return Replace(final_value);
-#endif // V8_TARGET_ARCH_IA32
}
// https://tc39.es/ecma262/#sec-%arrayiteratorprototype%.next
```
Change information
Commit message:
[compiler] Inline GeneratorPrototypeNext in Turbofan
Inlines the GeneratorPrototypeNext builtin in JSCallReducer, converting
it down to a JSResumeGenerator node to eliminate builtin overhead and
begin paving the way for further escape analysis optimizations.
Bug: 491263706
Change-Id: Id5a681e2a66a66834090cb93fcfd281ae198be39
Commit-Queue: Victor Gomes <victo...@chromium.org>
Reviewed-by: Darius Mercadier <dmerc...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105709}
Files:
- M src/compiler/js-call-reducer.cc
- M src/compiler/js-call-reducer.h
Change size: M
Delta: 2 files changed, 161 insertions(+), 0 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Darius Mercadier
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages