[vm,dyn_modules] Source position cleanups and fewer temp uses.

Reduces the usage of temps in the following cases to remove unneeded
StoreLocal/Push instructions:

Refactors visitVariableSet so that a temporary is used only if the
variable is captured and the result is not ignored. (That is,
sets of uncaptured late final fields no longer need a temporary.)

Only allocate a temp for FunctionDeclarations and FunctionExpressions
if there are function type arguments to capture or if the function
is generic (in which case the delayed type arguments field must be
empty-initialized, not null initialized).

-----

Clean up how source positions are emitted to both be more consistent
and to also move some source position emissions closer to the actual
instruction that performs the operation:

Move emission of debugging information for FunctionDeclaration,
and VariableSet to when the variable is actually set. The emission
in VariableDeclaration stays before the initializer as that's when
debugger tests expect to pause at it.

Change the implementation of _genConditionAndJumpIf to use if/else
chaining instead of early returns so that the condition's source
position can be recorded once before the if/then chain and then the
previous source position restored afterward the chain. This means
that _emitDebuggerInformation can be used.

_generateNonLocalControlTransfer is only called during visit methods
where the parent call to _generateNode has already recorded the from
node's position, so there's no need to record it during that call.
Instead, move the emission of the source position to the jump within the
passed continuation.

Remove _emitLocalSourcePosition, since it is no longer used.

Cq-Include-Trybots: luci.dart.try:vm-dyn-linux-debug-x64-try,vm-dyn-mac-debug-arm64-try
Change-Id: Id7722146769c57713ae466d67401258c010cee37
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/473380
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This commit is contained in:
Tess Strickland
2026-01-19 04:04:43 -08:00
committed by Commit Queue
parent 2bfdb03c4a
commit a02596b28f
5 changed files with 104 additions and 119 deletions
+61 -78
View File
@@ -1533,25 +1533,23 @@ class BytecodeGenerator extends RecursiveVisitor {
}
void _genConditionAndJumpIf(Expression condition, bool value, Label dest) {
final savedSourcePosition = asm.currentSourcePosition;
_recordSourcePosition(condition.fileOffset);
final bool? constantValue = _constantConditionValue(condition);
if (constantValue != null) {
if (constantValue == value) {
_emitLocalSourcePosition(condition.fileOffset);
_emitSourcePosition();
asm.emitJump(dest);
}
return;
}
if (condition is EqualsNull) {
} else if (condition is EqualsNull) {
_generateNode(condition.expression);
_emitLocalSourcePosition(condition.fileOffset);
_emitSourcePosition();
if (value) {
asm.emitJumpIfNull(dest);
} else {
asm.emitJumpIfNotNull(dest);
}
return;
}
if (condition is Not) {
} else if (condition is Not) {
_genConditionAndJumpIf(condition.operand, !value, dest);
} else if (condition is LogicalExpression) {
final isOR = (condition.operatorEnum == LogicalExpressionOperator.OR);
@@ -1573,13 +1571,14 @@ class BytecodeGenerator extends RecursiveVisitor {
if (negated) {
value = !value;
}
_emitLocalSourcePosition(condition.fileOffset);
_emitSourcePosition();
if (value) {
asm.emitJumpIfTrue(dest);
} else {
asm.emitJumpIfFalse(dest);
}
}
asm.currentSourcePosition = savedSourcePosition;
}
int _getDefaultParamConstIndex(VariableDeclaration param) {
@@ -2637,20 +2636,25 @@ class BytecodeGenerator extends RecursiveVisitor {
_genPushInstantiatorTypeArguments();
asm.emitAllocateClosure();
final int temp = locals.tempIndexInFrame(node);
asm.emitStoreLocal(temp);
final bool storeFunctionTAV = locals.hasFunctionTypeArgsVar;
final bool setEmptyDelayedTAV = function.typeParameters.isNotEmpty;
if (locals.hasFunctionTypeArgsVar) {
asm.emitPush(temp);
_genPushFunctionTypeArguments();
asm.emitStoreFieldTOS(cp.addInstanceField(closureFunctionTypeArguments));
}
if (storeFunctionTAV || setEmptyDelayedTAV) {
final int temp = locals.tempIndexInFrame(node);
asm.emitStoreLocal(temp);
// Delayed type arguments are only used by generic closures.
if (function.typeParameters.isNotEmpty) {
asm.emitPush(temp);
asm.emitPushConstant(cp.addEmptyTypeArguments());
asm.emitStoreFieldTOS(cp.addInstanceField(closureDelayedTypeArguments));
if (storeFunctionTAV) {
asm.emitPush(temp);
_genPushFunctionTypeArguments();
asm.emitStoreFieldTOS(
cp.addInstanceField(closureFunctionTypeArguments));
}
if (setEmptyDelayedTAV) {
asm.emitPush(temp);
asm.emitPushConstant(cp.addEmptyTypeArguments());
asm.emitStoreFieldTOS(cp.addInstanceField(closureDelayedTypeArguments));
}
}
}
@@ -2770,27 +2774,13 @@ class BytecodeGenerator extends RecursiveVisitor {
// Emits a source position entry and/or debugger stop as appropriate.
void _emitSourcePosition() {
if (options.emitSourcePositions) {
asm.emitSourcePosition();
}
asm.emitSourcePosition();
if (options.emitDebuggerStops &&
(asm.currentSourcePositionFlags & SourcePositions.syntheticFlag) == 0) {
asm.emitDebugCheck();
}
}
// Records the given file offset as the current source position and
// emits a source position entry and/or debugger stop as appropriate,
// restoring the current source position afterwards.
void _emitLocalSourcePosition(int fileOffset) {
if (fileOffset != TreeNode.noOffset) {
final savedSourcePosition = asm.currentSourcePosition;
_recordSourcePosition(fileOffset);
_emitSourcePosition();
asm.currentSourcePosition = savedSourcePosition;
}
}
/// Generates non-local transfer from inner node [from] into the outer
/// node, executing finally blocks on the way out. [to] can be null,
/// in such case all enclosing finally blocks are executed.
@@ -2798,7 +2788,6 @@ class BytecodeGenerator extends RecursiveVisitor {
/// the last finally block.
void _generateNonLocalControlTransfer(
TreeNode from, TreeNode to, GenerateContinuation continuation) {
_emitLocalSourcePosition(from.fileOffset);
List<TryFinally> tryFinallyBlocks = _getEnclosingTryFinallyBlocks(from, to);
_addFinallyBlocks(tryFinallyBlocks, continuation);
}
@@ -3737,31 +3726,44 @@ class BytecodeGenerator extends RecursiveVisitor {
@override
void visitVariableSet(VariableSet node) {
final v = node.variable;
final bool hasResult = !isExpressionWithoutResult(node);
final bool isLateFinal = v.isLate && v.isFinal;
if (!isLateFinal) {
_genPushContextIfCaptured(v);
}
_genPushContextIfCaptured(v);
_generateNode(node.value);
// Wrap the set in an already initialized check for late final variables.
final bool isLateFinal = v.isLate && v.isFinal;
final Label error = new Label();
if (isLateFinal) {
_genLoadVar(v);
asm.emitJumpIfInitialized(error);
}
// _genStoreVar pops the stored value off the stack. If the result isn't
// used, this is fine. If it is used but the variable isn't captured, then
// the generator uses StoreLocal instead of calling _genStoreVar. Otherwise,
// a temporary must be used to save and restore the value (as there is no
// keep-on-stack equivalent of StoreContextVar).
final bool hasResult = !isExpressionWithoutResult(node);
final bool isCaptured = locals.isCaptured(v);
final bool storeResultInTemp = hasResult && isCaptured;
if (storeResultInTemp) {
asm.emitStoreLocal(locals.tempIndexInFrame(node));
}
if (!v.isSynthesized) {
_emitSourcePosition();
}
if (hasResult && !isCaptured) {
asm.emitStoreLocal(locals.getVarIndexInFrame(v));
} else {
_genStoreVar(v);
}
if (storeResultInTemp) {
asm.emitPush(locals.tempIndexInFrame(node));
}
if (isLateFinal) {
final int temp = locals.tempIndexInFrame(node);
asm.emitPopLocal(temp);
final Label error = new Label();
final Label done = new Label();
_genLoadVar(v);
asm.emitJumpIfInitialized(error);
_genPushContextIfCaptured(v);
asm.emitPush(temp);
_genStoreVar(v);
asm.emitJump(done);
asm.bind(error);
@@ -3771,28 +3773,6 @@ class BytecodeGenerator extends RecursiveVisitor {
asm.emitDrop1();
asm.bind(done);
if (hasResult) {
asm.emitPush(temp);
}
} else if (locals.isCaptured(v)) {
final int temp = locals.tempIndexInFrame(node);
if (hasResult) {
asm.emitStoreLocal(temp);
}
_genStoreVar(v);
if (hasResult) {
asm.emitPush(temp);
}
} else {
final int localIndex = locals.getVarIndexInFrame(v);
if (hasResult) {
asm.emitStoreLocal(localIndex);
} else {
asm.emitPopLocal(localIndex);
}
}
}
@@ -3887,6 +3867,7 @@ class BytecodeGenerator extends RecursiveVisitor {
_generateNonLocalControlTransfer(node, node.target, () {
_genUnwindContext(targetContextLevel);
_emitSourcePosition();
asm.emitJump(targetLabel);
});
}
@@ -3899,6 +3880,7 @@ class BytecodeGenerator extends RecursiveVisitor {
_generateNonLocalControlTransfer(node, node.target.parent!, () {
_genUnwindContext(targetContextLevel);
_emitSourcePosition();
asm.emitJump(targetLabel);
});
}
@@ -3991,9 +3973,9 @@ class BytecodeGenerator extends RecursiveVisitor {
@override
void visitFunctionDeclaration(ast.FunctionDeclaration node) {
_emitSourcePosition();
_genPushContextIfCaptured(node.variable);
_genClosure(node, node.variable.name!, node.function);
_emitSourcePosition();
_genStoreVar(node.variable);
}
@@ -4394,8 +4376,9 @@ class BytecodeGenerator extends RecursiveVisitor {
asm.currentSourcePositionFlags |= SourcePositions.syntheticFlag;
}
if (emitStore) {
// Record the source position at the start of the bytecode generated
// for storing the variable.
// Record the source position of the declaration at the start of
// the generated bytecode since debugger tests expect to pause
// at the declaration prior to running the initializer (if any).
if (initializer != null) {
_recordSourcePosition(node.fileEqualsOffset);
}
+24 -6
View File
@@ -1003,19 +1003,37 @@ class _Allocator extends RecursiveVisitor {
_visitFunction(node);
}
// A temporary is only needed for function declarations or expressions when:
// * There are function type arguments to capture.
// * The function is generic and so the delayed type arguments field of the
// closure must be empty-initialized, not null-initialized.
bool _closureAllocationNeedsTemp(FunctionNode function) =>
_currentFrame.functionTypeArgsVar != null ||
function.typeParameters.isNotEmpty;
@override
void visitFunctionDeclaration(FunctionDeclaration node) {
_allocateVariable(node.variable);
_allocateTemp(node);
final needsTemp = _closureAllocationNeedsTemp(node.function);
if (needsTemp) {
_allocateTemp(node);
}
_visitFunction(node);
_freeTemp(node);
if (needsTemp) {
_freeTemp(node);
}
}
@override
void visitFunctionExpression(FunctionExpression node) {
_allocateTemp(node);
final needsTemp = _closureAllocationNeedsTemp(node.function);
if (needsTemp) {
_allocateTemp(node);
}
_visitFunction(node);
_freeTemp(node);
if (needsTemp) {
_freeTemp(node);
}
}
@override
@@ -1178,8 +1196,8 @@ class _Allocator extends RecursiveVisitor {
@override
void visitVariableSet(VariableSet node) {
final v = node.variable;
final bool needsTemp = locals.isCaptured(v) || v.isLate && v.isFinal;
final bool needsTemp =
node.parent is! ExpressionStatement && locals.isCaptured(node.variable);
_visit(node, temps: needsTemp ? 1 : 0);
}
@@ -9,13 +9,12 @@ Class '', script = 'DART_SDK/pkg/dart2bytecode/testcases/async.dart'
Field 'asyncInFieldInitializer', type = FunctionType (dart:async::Future < dart:core::int >) -> dart:async::Future < Null >, getter = 'get:asyncInFieldInitializer', reflectable, static, is-late, has-initializer
initializer
Bytecode {
Entry 3
Entry 2
CheckStack 0
PushConstant CP#0
Push r0
PushNull
AllocateClosure
StoreLocal r2
ReturnTOS
}
ConstantPool {
@@ -530,7 +529,7 @@ Function 'closure', static, reflectable, debuggable
return-type dynamic
Bytecode {
Entry 4
Entry 3
CheckStack 0
AllocateContext 0, 2
PopLocal r0
@@ -544,7 +543,6 @@ Bytecode {
Push r0
PushNull
AllocateClosure
StoreLocal r3
PopLocal r2
Push r2
ReturnTOS
@@ -23,7 +23,6 @@ Bytecode {
Push r0
PushNull
AllocateClosure
StoreLocal r3
PopLocal r2
Push r2
StoreLocal r3
@@ -47,7 +46,7 @@ ConstantPool {
}
Closure DART_SDK/pkg/dart2bytecode/testcases/closures.dart::simpleClosure::'<anonymous closure>' (dart:core::int y) -> Null
ClosureCode {
Entry 3
Entry 2
Push FP[-6]
LoadFieldTOS CP#1
PopLocal r0
@@ -780,7 +779,6 @@ Bytecode {
Push r0
PushNull
AllocateClosure
StoreLocal r4
PopLocal r3
Push r3
StoreLocal r4
@@ -812,7 +810,6 @@ Bytecode {
Push r0
PushNull
AllocateClosure
StoreLocal r3
PopLocal r2
Push r2
StoreLocal r3
@@ -888,10 +885,9 @@ L1:
Push r0
PushNull
AllocateClosure
StoreLocal r2
PopLocal r3
Push r3
Push r3
PopLocal r2
Push r2
Push r2
UncheckedClosureCall CP#10, 1
Drop1
Push r0
@@ -905,7 +901,7 @@ L2:
Closure DART_SDK/pkg/dart2bytecode/testcases/closures.dart::B::topLevel::Closure/0::'closure2' () -> void
ClosureCode {
Entry 3
Entry 2
Push FP[-5]
LoadFieldTOS CP#1
PopLocal r0
@@ -1012,7 +1008,6 @@ L2:
Push r0
PushNull
AllocateClosure
StoreLocal r4
InstantiatedInterfaceCall CP#7, 2
Drop1
Push r3
@@ -1020,7 +1015,6 @@ L2:
Push r0
PushNull
AllocateClosure
StoreLocal r4
InstantiatedInterfaceCall CP#7, 2
Drop1
Push r0
@@ -1081,7 +1075,7 @@ ClosureCode {
Closure DART_SDK/pkg/dart2bytecode/testcases/closures.dart::C::testForLoop::'<anonymous closure>' (dart:core::int ii) -> Null
ClosureCode {
Entry 3
Entry 2
Push FP[-6]
LoadFieldTOS CP#4
PopLocal r0
@@ -1132,7 +1126,6 @@ L2:
Push r0
PushNull
AllocateClosure
StoreLocal r4
PopLocal r3
Push r3
StoreLocal r4
@@ -1168,7 +1161,7 @@ ConstantPool {
}
Closure DART_SDK/pkg/dart2bytecode/testcases/closures.dart::C::testForInLoop::'<anonymous closure>' () -> Null
ClosureCode {
Entry 3
Entry 2
Push FP[-5]
LoadFieldTOS CP#7
PopLocal r0
@@ -1212,7 +1205,7 @@ Function 'foo', reflectable, debuggable
return-type dynamic
Bytecode {
Entry 3
Entry 2
CheckStack 0
AllocateContext 0, 1
PopLocal r0
@@ -1234,7 +1227,6 @@ L1:
Push FP[-6]
LoadTypeArgumentsField CP#1
AllocateClosure
StoreLocal r2
ReturnTOS
}
Parameter flags: [2]
@@ -1266,7 +1258,7 @@ Function 'bar', reflectable, debuggable
return-type dynamic
Bytecode {
Entry 3
Entry 2
CheckStack 0
AllocateContext 0, 1
PopLocal r0
@@ -1278,7 +1270,6 @@ Bytecode {
Push FP[-5]
LoadTypeArgumentsField CP#5
AllocateClosure
StoreLocal r2
ReturnTOS
}
ConstantPool {
@@ -1304,7 +1295,6 @@ ClosureCode {
LoadContextVar 0, 0
LoadTypeArgumentsField CP#5
AllocateClosure
StoreLocal r3
PopLocal r2
Push r2
Push r2
@@ -219,7 +219,6 @@ Try #0 start:
Push r0
PushNull
AllocateClosure
StoreLocal r5
PopLocal r4
Push r4
Push r4
@@ -270,7 +269,6 @@ Try #0 handler:
Push r0
PushNull
AllocateClosure
StoreLocal r5
PopLocal r6
Push r6
ReturnTOS
@@ -282,7 +280,7 @@ L1:
ReturnTOS
}
ExceptionsTable {
try-index 0, outer -1, start 20, end 58, handler 58, needs-stack-trace, types [CP#6]
try-index 0, outer -1, start 20, end 56, handler 56, needs-stack-trace, types [CP#6]
}
ConstantPool {
[0] = ClosureFunction 0
@@ -309,7 +307,7 @@ ConstantPool {
}
Closure DART_SDK/pkg/dart2bytecode/testcases/try_blocks.dart::testTryCatch3::'foo' () -> void
ClosureCode {
Entry 6
Entry 5
Push FP[-5]
LoadFieldTOS CP#1
PopLocal r0
@@ -323,7 +321,7 @@ Try #0 start:
Jump L1
Try #0 end:
Try #0 handler:
SetFrame 6
SetFrame 5
Push r2
PopLocal r0
MoveSpecial exception, r2
@@ -594,7 +592,6 @@ Try #1 start:
Push r0
PushNull
AllocateClosure
StoreLocal r8
PopLocal r7
Push r7
Push r7
@@ -651,8 +648,8 @@ L3:
ReturnTOS
}
ExceptionsTable {
try-index 0, outer -1, start 53, end 136, handler 136, needs-stack-trace, synthetic, types [CP#11]
try-index 1, outer 0, start 70, end 98, handler 98, needs-stack-trace, synthetic, types [CP#11]
try-index 0, outer -1, start 53, end 134, handler 134, needs-stack-trace, synthetic, types [CP#11]
try-index 1, outer 0, start 70, end 96, handler 96, needs-stack-trace, synthetic, types [CP#11]
}
ConstantPool {
[0] = InterfaceCall 'dart:core::Object::==', ArgDesc num-args 2, num-type-args 0, names []
@@ -697,7 +694,7 @@ Function 'testTryFinally3', static, reflectable, debuggable
return-type dynamic
Bytecode {
Entry 6
Entry 5
CheckStack 0
AllocateContext 0, 1
PopLocal r0
@@ -713,12 +710,11 @@ Try #0 start:
Push r0
PushNull
AllocateClosure
StoreLocal r5
PopLocal r2
Jump L1
Try #0 end:
Try #0 handler:
SetFrame 6
SetFrame 5
Push r3
PopLocal r0
MoveSpecial exception, r3
@@ -750,7 +746,7 @@ L1:
ReturnTOS
}
ExceptionsTable {
try-index 0, outer -1, start 23, end 37, handler 37, needs-stack-trace, synthetic, types [CP#6]
try-index 0, outer -1, start 23, end 35, handler 35, needs-stack-trace, synthetic, types [CP#6]
}
ConstantPool {
[0] = ClosureFunction 0