From b327896c1e2415bd7785a16d7ab1794c5f14f634 Mon Sep 17 00:00:00 2001 From: stepan Date: Fri, 4 Sep 2026 13:49:04 +0200 Subject: [PATCH] Optimize two-element destructuring assignments --- .../bytecode_dsl/RootNodeCompiler.java | 84 +++++++++++++++---- .../bytecode_dsl/PBytecodeDSLRootNode.java | 61 ++++++++++++++ 2 files changed, 131 insertions(+), 14 deletions(-) diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java index 61025e8f14..23d562ea86 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java @@ -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(); @@ -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)); } } @@ -3590,9 +3590,15 @@ public void visitTypeParams(TypeParamTy[] typeParams) { public class StoreVisitor implements BaseBytecodeDSLVisitor { 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 @@ -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, @@ -3692,7 +3712,7 @@ private void visitIterableAssign(ExprTy[] nodes) { target.accept(new StoreVisitor(() -> { b.emitLoadLocal(targets[index]); - })); + }, allowFastLocalLookup)); endTemporaryLocal(targets[index]); } @@ -3894,8 +3914,35 @@ 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); })); @@ -3903,17 +3950,26 @@ private void emitAssignment(ExprTy[] targets, ExprTy value) { 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()) { @@ -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 void emitAsyncFor(ExprTy iterOrNull, ExprTy target, StmtTy[] orElse, boolean isComprehension, T arg, BiConsumer body) { assert !isComprehension || orElse == null; @@ -3987,7 +4043,7 @@ private 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) { @@ -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); diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java index 223dd6f753..83e79d0c5e 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java @@ -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.