Skip to content

[GR-79401] Optimize two-element destructuring assignments. #1132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graalvmbot merged 1 commit into from
Sep 8, 2026
Merged

[GR-79401] Optimize two-element destructuring assignments. #1132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 70 additions & 14 deletions ...acle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1428,7 +1428,7 @@ private void emitComprehension(ComprehensionTy[] generators, int index, Builder

b.beginBlock();

statementCompiler.storeTemporaryLocalToTarget(localValue, comp.target, b);
statementCompiler.storeTemporaryLocalToTarget(localValue, comp.target, b, false);
emitComprehensionBody(generators, index, type, collection, accumulateProducer, statementCompiler);

b.endBlock();
Expand Down Expand Up @@ -2055,17 +2055,17 @@ private BytecodeLocal beginTemporaryLocal() {
/**
* If the target expression is simple local variable, we can often just directly store into it.
* Otherwise, we create temporary local. {@link BytecodeLocal} instances returned by this should
* be passed to {@link #storeTemporaryLocalToTarget(BytecodeLocal, ExprTy, Builder)}.
* be passed to {@link #storeTemporaryLocalToTarget(BytecodeLocal, ExprTy, Builder, boolean)}.
*/
public BytecodeLocal beginTemporaryLocalOrGetLocal(ExprTy target, Builder b) {
return RootNodeCompiler.this.beginTemporaryLocalOrGetLocal(target, b);
}

private void storeTemporaryLocalToTarget(BytecodeLocal temporaryLocal, ExprTy target, Builder b) {
private void storeTemporaryLocalToTarget(BytecodeLocal temporaryLocal, ExprTy target, Builder b, boolean allowFastLocalLookup) {
if (RootNodeCompiler.this.isTemporaryLocal(temporaryLocal)) {
target.accept(new StoreVisitor(() -> {
b.emitLoadLocal(temporaryLocal);
}));
}, allowFastLocalLookup));
}
}

Expand Down Expand Up @@ -3590,9 +3590,15 @@ public void visitTypeParams(TypeParamTy[] typeParams) {
public class StoreVisitor implements BaseBytecodeDSLVisitor<Void> {
private final Builder b = StatementCompiler.this.b;
private final Runnable generateValue;
private final boolean allowFastLocalLookup;

StoreVisitor(Runnable generateValue) {
this(generateValue, true);
}

StoreVisitor(Runnable generateValue, boolean allowFastLocalLookup) {
this.generateValue = generateValue;
this.allowFastLocalLookup = allowFastLocalLookup;
}

@Override
Expand Down Expand Up @@ -3645,6 +3651,20 @@ public Void visit(ExprTy.Starred node) {
private void visitIterableAssign(ExprTy[] nodes) {
b.beginBlock();

if (nodes.length == 2 && containsNoStarred(nodes)) {
BytecodeLocal target1 = allowFastLocalLookup ? beginTemporaryLocalOrGetLocal(nodes[0], b) : beginTemporaryLocal();
BytecodeLocal target2 = allowFastLocalLookup ? beginTemporaryLocalOrGetLocal(nodes[1], b) : beginTemporaryLocal();
b.beginUnpackToLocals2(target1, target2);
generateValue.run();
b.endUnpackToLocals2();
storeTemporaryLocalToTarget(target1, nodes[0], b, allowFastLocalLookup);
storeTemporaryLocalToTarget(target2, nodes[1], b, allowFastLocalLookup);
endTemporaryLocal(target1);
endTemporaryLocal(target2);
b.endBlock();
return;
}

/*
* The rhs should be fully evaluated and unpacked into the expected number of
* elements before storing values into the lhs (e.g., if an lhs element is f().attr,
Expand Down Expand Up @@ -3692,7 +3712,7 @@ private void visitIterableAssign(ExprTy[] nodes) {

target.accept(new StoreVisitor(() -> {
b.emitLoadLocal(targets[index]);
}));
}, allowFastLocalLookup));
endTemporaryLocal(targets[index]);
}

Expand Down Expand Up @@ -3894,26 +3914,62 @@ public Void visit(StmtTy.Assign node) {
return null;
}

private ExprTy[] getElementsOfTupleOrList(ExprTy value) {
if (value instanceof Tuple tupleExpr) {
return tupleExpr.elements;
} else if (value instanceof ExprTy.List listExpr) {
return listExpr.elements;
}
return null;
}

private void emitAssignment(ExprTy[] targets, ExprTy value) {
if (targets.length == 1) {
ExprTy[] valueElements = getElementsOfTupleOrList(value);
if (targets.length == 1 && targets[0] instanceof ExprTy.Tuple targetTuple && valueElements != null &&
targetTuple.elements.length == valueElements.length &&
containsNoStarred(targetTuple.elements) && containsNoStarred(valueElements)) {
b.beginBlock();
StackValue[] values = new StackValue[valueElements.length];
for (int i = 0; i < values.length; i++) {
b.beginBindStackValue();
valueElements[i].accept(this);
values[i] = b.endBindStackValue();
}
for (int i = 0; i < values.length; i++) {
int index = i;
targetTuple.elements[i].accept(new StoreVisitor(() -> {
b.emitLoadStackValue(values[index]);
}));
}
b.endBlock();
} else if (targets.length == 1) {
targets[0].accept(new StoreVisitor(() -> {
value.accept(this);
}));
} else {
b.beginBlock();
b.beginBindStackValue();
value.accept(this);
StackValue tmp = b.endBindStackValue();
StackValue values = b.endBindStackValue();

for (ExprTy target : targets) {
target.accept(new StoreVisitor(() -> {
b.emitLoadStackValue(tmp);
b.emitLoadStackValue(values);
}));
}
b.endBlock();
}
}

private static boolean containsNoStarred(ExprTy[] nodes) {
for (ExprTy node : nodes) {
if (node instanceof ExprTy.Starred) {
return false;
}
}
return true;
}

@Override
public Void visit(StmtTy.AsyncFor node) {
if (!scope.isFunction()) {
Expand All @@ -3931,10 +3987,10 @@ public Void visit(StmtTy.AsyncFor node) {
}

/**
* @param iterOrNull If {@code null}, then it assumes that the first argument holds the
* iterator, i.e., it won't call {@code __aiter__} on it and just use it as is.
* This is the calling convention for async comprehensions.
*/
* @param iterOrNull If {@code null}, then it assumes that the first argument holds the
* iterator, i.e., it won't call {@code __aiter__} on it and just use it as is.
* This is the calling convention for async comprehensions.
*/
private <T> void emitAsyncFor(ExprTy iterOrNull, ExprTy target, StmtTy[] orElse, boolean isComprehension,
T arg, BiConsumer<StatementCompiler, T> body) {
assert !isComprehension || orElse == null;
Expand Down Expand Up @@ -3987,7 +4043,7 @@ private <T> void emitAsyncFor(ExprTy iterOrNull, ExprTy target, StmtTy[] orElse,
b.endTryCatch();
b.emitLoadLocal(result);
b.endBlock();
}));
}, !isComprehension));
// TODO: GR-71890, we should clear result, or create a temporary local for each iteration
body.accept(this, arg);
if (!isComprehension) {
Expand Down Expand Up @@ -4408,7 +4464,7 @@ public Void visit(StmtTy.For node) {
// body
b.beginBlock();
continueLabel = b.createLabel();
storeTemporaryLocalToTarget(value, node.target, b);
storeTemporaryLocalToTarget(value, node.target, b, true);

visitSequence(node.body);
b.emitLabel(continueLabel);
Expand Down
61 changes: 61 additions & 0 deletions ...cle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2665,6 +2665,67 @@ public static void doIt(VirtualFrame frame, Object value, Object primary, Object
}
}

@Operation(storeBytecodeIndex = true)
@ConstantOperand(type = LocalAccessor.class)
@ConstantOperand(type = LocalAccessor.class)
@ImportStatic({PGuards.class})
public static final class UnpackToLocals2 {
@ExplodeLoop
@Specialization(guards = "isBuiltinSequence(sequence)")
public static void doUnpackSequence(VirtualFrame localFrame, LocalAccessor target1, LocalAccessor target2, PSequence sequence,
@Bind Node inliningTarget,
@Bind BytecodeNode bytecode,
@Cached SequenceNodes.GetSequenceStorageNode getSequenceStorageNode,
@Cached SequenceStorageNodes.GetItemScalarNode getItemNode,
@Exclusive @Cached PRaiseNode raiseNode) {
SequenceStorage storage = getSequenceStorageNode.execute(inliningTarget, sequence);
int len = storage.length();
if (len != 2) {
throw UnpackToLocals.raiseError(inliningTarget, raiseNode, len, 2);
}
Object value1 = getItemNode.execute(inliningTarget, storage, 0);
Object value2 = getItemNode.execute(inliningTarget, storage, 1);
target1.setObject(bytecode, localFrame, value1);
target2.setObject(bytecode, localFrame, value2);
}

@Specialization
@InliningCutoff
public static void doUnpackIterable(VirtualFrame virtualFrame, LocalAccessor target1, LocalAccessor target2, Object collection,
@Bind Node inliningTarget,
@Bind BytecodeNode bytecode,
@Cached PyObjectGetIter getIter,
@Cached PyIterNextNode getNextNode,
@Cached IsBuiltinObjectProfile notIterableProfile,
@Exclusive @Cached PRaiseNode raiseNode) {
Object iterator;
try {
iterator = getIter.execute(virtualFrame, inliningTarget, collection);
} catch (PException e) {
e.expectTypeError(inliningTarget, notIterableProfile);
throw UnpackToLocals.raiseNotIterableError(collection, inliningTarget, raiseNode);
}
Object value1 = extractItem(virtualFrame, inliningTarget, 0, iterator, getNextNode, raiseNode);
Object value2 = extractItem(virtualFrame, inliningTarget, 1, iterator, getNextNode, raiseNode);
try {
getNextNode.execute(virtualFrame, inliningTarget, iterator);
} catch (IteratorExhausted e) {
target1.setObject(bytecode, virtualFrame, value1);
target2.setObject(bytecode, virtualFrame, value2);
return;
}
throw UnpackToLocals.raiseTooManyValues(inliningTarget, raiseNode, 2);
}

private static Object extractItem(VirtualFrame virtualFrame, Node inliningTarget, int index, Object iterator, PyIterNextNode getNextNode, PRaiseNode raiseNode) {
try {
return getNextNode.execute(virtualFrame, inliningTarget, iterator);
} catch (IteratorExhausted e) {
throw UnpackToLocals.raiseNotEnoughValues(inliningTarget, raiseNode, 2, index);
}
}
}

/**
* This operation is used to implement destructing assignment where the rhs should be fully
* evaluated and unpacked into temporary variables and then assigned to the targets.
Expand Down
Loading