Skip to content

Commit 756d326

Browse files
lukasstadlerlukasstadler
committed
avoid unnecessary reads/writes to <return> frame slot
1 parent e9b38d2 commit 756d326

File tree

3 files changed

+56
-40
lines changed

3 files changed

+56
-40
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/grammar/TestParserTranslator.java

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@
6666
import com.oracle.graal.python.nodes.attributes.GetAttributeNode;
6767
import com.oracle.graal.python.nodes.attributes.SetAttributeNode;
6868
import com.oracle.graal.python.nodes.call.PythonCallNode;
69-
import com.oracle.graal.python.nodes.control.BlockNode;
7069
import com.oracle.graal.python.nodes.expression.AndNode;
7170
import com.oracle.graal.python.nodes.expression.BinaryArithmetic;
7271
import com.oracle.graal.python.nodes.expression.BinaryComparisonNode;
@@ -149,7 +148,7 @@ private Node unpackModuleBodyWrappers(Node n) {
149148
Node actual = n;
150149
if (n instanceof ExpressionNode.ExpressionStatementNode) {
151150
actual = n.getChildren().iterator().next();
152-
} else if (n instanceof ExpressionNode.ExpressionWithSideEffects) {
151+
} else if (n instanceof ExpressionNode.ExpressionWithSideEffects || n instanceof ExpressionNode.ExpressionWithSideEffect) {
153152
actual = n.getChildren().iterator().next();
154153
} else if (n instanceof WriteLocalVariableNode) {
155154
if (((WriteLocalVariableNode) n).getIdentifier().equals(FrameSlotIDs.RETURN_SLOT_ID)) {
@@ -316,9 +315,6 @@ public void parseDelStatement() {
316315
parseAs("del world", DeleteGlobalNode.class);
317316
parseAs("del world[0]", DeleteItemNode.class);
318317
parseAs("del world.field", DeleteAttributeNode.class);
319-
BlockNode parseAs = parseAs("del world.field, world[0]", BlockNode.class);
320-
getChild(parseAs, 0, DeleteAttributeNode.class);
321-
getChild(parseAs, 1, DeleteItemNode.class);
322318
}
323319

324320
@Test
@@ -334,11 +330,6 @@ public void parseAssignments() {
334330
parseAs = parseAs("a = 1,2", WriteGlobalNode.class);
335331
assert parseAs.getRhs() instanceof TupleLiteralNode;
336332

337-
BlockNode parseAs2 = parseAs("a = b = 1", BlockNode.class);
338-
getChild(parseAs2, 0, WriteNode.class); // write tmp
339-
getChild(parseAs2, 1, WriteNode.class); // write a
340-
getChild(parseAs2, 2, WriteNode.class); // write b
341-
342333
parseAs("a,b = 1,2", DestructuringAssignmentNode.class);
343334
parseAs("a,*b,c = 1,2", DestructuringAssignmentNode.class);
344335
parseAs("[[a],*b],c = 1,2", DestructuringAssignmentNode.class);
@@ -375,7 +366,7 @@ public void parseComparisons() {
375366

376367
AndNode parseAs = parseAs("x < y() <= z", AndNode.class);
377368
PNode leftNode = parseAs.getLeftNode();
378-
assert leftNode instanceof ExpressionNode.ExpressionWithSideEffects;
369+
assert leftNode instanceof ExpressionNode.ExpressionWithSideEffect;
379370
WriteNode tmpWrite = getChild(leftNode, 0, WriteNode.class);
380371
assert tmpWrite.getRhs() instanceof PythonCallNode;
381372
PythonCallNode rhs = (PythonCallNode) tmpWrite.getRhs();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/expression/ExpressionNode.java

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,16 @@
4141
package com.oracle.graal.python.nodes.expression;
4242

4343
import com.oracle.graal.python.nodes.PNode;
44+
import com.oracle.graal.python.nodes.control.BlockNode;
4445
import com.oracle.graal.python.nodes.statement.StatementNode;
4546
import com.oracle.truffle.api.frame.VirtualFrame;
4647
import com.oracle.truffle.api.instrumentation.GenerateWrapper;
4748
import com.oracle.truffle.api.instrumentation.ProbeNode;
4849
import com.oracle.truffle.api.instrumentation.StandardTags.ExpressionTag;
4950
import com.oracle.truffle.api.instrumentation.Tag;
51+
import com.oracle.truffle.api.nodes.ExplodeLoop;
5052
import com.oracle.truffle.api.nodes.NodeCost;
5153
import com.oracle.truffle.api.nodes.UnexpectedResultException;
52-
import com.oracle.truffle.api.source.SourceSection;
5354

5455
/**
5556
* Base class for all expressions. Expressions always return a value.
@@ -135,15 +136,14 @@ public final StatementNode asStatement() {
135136
return new ExpressionStatementNode(this);
136137
}
137138

138-
public static final class ExpressionWithSideEffects extends ExpressionNode {
139+
public static final class ExpressionWithSideEffect extends ExpressionNode {
139140
@Child private StatementNode sideEffect;
140141
@Child private ExpressionNode node;
141142

142-
private ExpressionWithSideEffects(ExpressionNode node, StatementNode sideEffect) {
143+
private ExpressionWithSideEffect(ExpressionNode node, StatementNode sideEffect) {
143144
this.node = node;
144145
this.sideEffect = sideEffect;
145-
SourceSection sourceSection = node.getSourceSection();
146-
this.assignSourceSection(sourceSection);
146+
this.assignSourceSection(node.getSourceSection());
147147
}
148148

149149
@Override
@@ -158,11 +158,40 @@ public boolean hasSideEffectAsAnExpression() {
158158
}
159159
}
160160

161+
public static final class ExpressionWithSideEffects extends ExpressionNode {
162+
@Children private final StatementNode[] sideEffects;
163+
@Child private ExpressionNode node;
164+
165+
private ExpressionWithSideEffects(ExpressionNode node, StatementNode[] sideEffects) {
166+
this.node = node;
167+
this.sideEffects = sideEffects;
168+
this.assignSourceSection(node.getSourceSection());
169+
}
170+
171+
@Override
172+
@ExplodeLoop
173+
public Object execute(VirtualFrame frame) {
174+
for (int i = 0; i < sideEffects.length; i++) {
175+
sideEffects[i].executeVoid(frame);
176+
}
177+
return node.execute(frame);
178+
}
179+
180+
@Override
181+
public boolean hasSideEffectAsAnExpression() {
182+
return true;
183+
}
184+
}
185+
161186
/**
162187
* Some expressions can have hidden side-effects such as writing to a temporary variable. These
163188
* can be wrapped together with their side effecting {@link StatementNode}.
164189
*/
165190
public final ExpressionNode withSideEffect(StatementNode sideEffect) {
166-
return new ExpressionWithSideEffects(this, sideEffect);
191+
if (sideEffect instanceof BlockNode) {
192+
return new ExpressionWithSideEffects(this, ((BlockNode) sideEffect).getStatements());
193+
} else {
194+
return new ExpressionWithSideEffect(this, sideEffect);
195+
}
167196
}
168197
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/PythonTreeTranslator.java

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1770,46 +1770,42 @@ private StatementNode asBlock(Object accept) {
17701770
List<PNode> inputList = (List<PNode>) accept;
17711771
if (inputList.size() == 0) {
17721772
return factory.createBlock();
1773+
} else if (inputList.size() == 1) {
1774+
return asBlock(inputList.get(0));
1775+
} else {
1776+
StatementNode[] statements = new StatementNode[inputList.size()];
1777+
for (int i = 0; i < statements.length; i++) {
1778+
statements[i] = asBlock(inputList.get(i));
1779+
}
1780+
return factory.createBlock(statements);
17731781
}
1774-
List<StatementNode> list = new ArrayList<>();
1775-
for (PNode node : inputList) {
1776-
list.add(asBlock(node));
1777-
}
1778-
StatementNode block = factory.createBlock(list);
1779-
return block;
17801782
} else {
1781-
throw new IllegalArgumentException();
1783+
throw new IllegalArgumentException("unexpected class: " + accept.getClass());
17821784
}
17831785
}
17841786

17851787
private ExpressionNode asExpression(Object accept) {
1786-
StatementNode moduleBlock = null;
17871788
if (accept instanceof List) {
17881789
@SuppressWarnings("unchecked")
17891790
List<PNode> list = (List<PNode>) accept;
1790-
if (list.size() > 0) {
1791+
if (list.size() == 0) {
1792+
return EmptyNode.create();
1793+
} else if (list.size() == 1) {
1794+
return asExpression(list.get(0));
1795+
} else {
17911796
ExpressionNode asExpression = asExpression(list.remove(list.size() - 1));
1792-
StatementNode writeReturnValue = factory.createWriteLocal(asExpression, environment.getReturnSlot());
1793-
writeReturnValue.assignSourceSection(asExpression.getSourceSection());
1794-
list.add(writeReturnValue);
1797+
return asExpression.withSideEffect(asBlock(list));
17951798
}
1796-
moduleBlock = asBlock(accept);
17971799
} else if (accept instanceof ExpressionNode.ExpressionStatementNode) {
1798-
moduleBlock = factory.createWriteLocal(((ExpressionNode.ExpressionStatementNode) accept).getExpression(), environment.getReturnSlot());
1799-
moduleBlock.assignSourceSection(((ExpressionNode.ExpressionStatementNode) accept).getSourceSection());
1800+
return ((ExpressionNode.ExpressionStatementNode) accept).getExpression();
18001801
} else if (accept instanceof ExpressionNode) {
1801-
moduleBlock = factory.createWriteLocal((ExpressionNode) accept, environment.getReturnSlot());
1802-
moduleBlock.assignSourceSection(((ExpressionNode) accept).getSourceSection());
1802+
return (ExpressionNode) accept;
18031803
} else if (accept instanceof StatementNode) {
1804-
moduleBlock = factory.createWriteLocal(EmptyNode.create().withSideEffect((StatementNode) accept), environment.getReturnSlot());
1804+
return EmptyNode.create().withSideEffect((StatementNode) accept);
18051805
} else if (accept == null) {
18061806
return EmptyNode.create();
1807-
}
1808-
ExpressionNode readReturn = factory.createReadLocal(environment.getReturnSlot());
1809-
if (moduleBlock != null) {
1810-
return readReturn.withSideEffect(moduleBlock);
18111807
} else {
1812-
return readReturn;
1808+
throw new IllegalArgumentException("unexpected class: " + accept.getClass());
18131809
}
18141810
}
18151811

0 commit comments

Comments
 (0)