Skip to content

Commit 0a0a8a0

Browse files
msimacekmsimacek
committed
Move transfer to generator frame to YIELD_VALUE
1 parent dbcabf4 commit 0a0a8a0

File tree

2 files changed

+378
-351
lines changed

2 files changed

+378
-351
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeGeneratorRootNode.java

Lines changed: 54 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -81,99 +81,81 @@ public PBytecodeGeneratorRootNode(PythonLanguage language, PBytecodeRootNode roo
8181
this.rootNode = rootNode;
8282
this.resumeBci = resumeBci;
8383
this.resumeStackTop = resumeStackTop;
84-
frameSlotTypes = new FrameSlotType[resumeStackTop + 1];
84+
frameSlotTypes = new FrameSlotType[resumeStackTop - rootNode.stackoffset + 1];
8585
}
8686

8787
@ExplodeLoop
88-
private void copyFrameSlotsIntoVirtualFrame(MaterializedFrame generatorFrame, VirtualFrame virtualFrame) {
88+
private void copyStackSlotsIntoVirtualFrame(MaterializedFrame generatorFrame, VirtualFrame virtualFrame) {
89+
int offset = rootNode.stackoffset;
8990
for (int i = 0; i < frameSlotTypes.length; i++) {
91+
int frameIndex = i + offset;
9092
switch (frameSlotTypes[i]) {
9193
case Object:
92-
if (generatorFrame.isObject(i)) {
93-
virtualFrame.setObject(i, generatorFrame.getObject(i));
94+
if (generatorFrame.isObject(frameIndex)) {
95+
virtualFrame.setObject(frameIndex, generatorFrame.getObject(frameIndex));
9496
continue;
9597
}
9698
break;
9799
case Int:
98-
if (generatorFrame.isInt(i)) {
99-
virtualFrame.setInt(i, generatorFrame.getInt(i));
100+
if (generatorFrame.isInt(frameIndex)) {
101+
virtualFrame.setInt(frameIndex, generatorFrame.getInt(frameIndex));
100102
continue;
101103
}
102104
break;
103105
case Long:
104-
if (generatorFrame.isLong(i)) {
105-
virtualFrame.setLong(i, generatorFrame.getLong(i));
106+
if (generatorFrame.isLong(frameIndex)) {
107+
virtualFrame.setLong(frameIndex, generatorFrame.getLong(frameIndex));
106108
continue;
107109
}
108110
break;
109111
case Double:
110-
if (generatorFrame.isDouble(i)) {
111-
virtualFrame.setDouble(i, generatorFrame.getDouble(i));
112+
if (generatorFrame.isDouble(frameIndex)) {
113+
virtualFrame.setDouble(frameIndex, generatorFrame.getDouble(frameIndex));
112114
continue;
113115
}
114116
break;
115117
case Boolean:
116-
if (generatorFrame.isBoolean(i)) {
117-
virtualFrame.setBoolean(i, generatorFrame.getBoolean(i));
118+
if (generatorFrame.isBoolean(frameIndex)) {
119+
virtualFrame.setBoolean(frameIndex, generatorFrame.getBoolean(frameIndex));
118120
continue;
119121
}
120122
break;
121123
}
122124
CompilerDirectives.transferToInterpreterAndInvalidate();
123-
if (generatorFrame.isObject(i)) {
124-
virtualFrame.setObject(i, generatorFrame.getObject(i));
125+
if (generatorFrame.isObject(frameIndex)) {
126+
virtualFrame.setObject(frameIndex, generatorFrame.getObject(frameIndex));
125127
frameSlotTypes[i] = FrameSlotType.Object;
126-
} else if (generatorFrame.isInt(i)) {
127-
virtualFrame.setInt(i, generatorFrame.getInt(i));
128+
} else if (generatorFrame.isInt(frameIndex)) {
129+
virtualFrame.setInt(frameIndex, generatorFrame.getInt(frameIndex));
128130
frameSlotTypes[i] = FrameSlotType.Int;
129-
} else if (generatorFrame.isLong(i)) {
130-
virtualFrame.setLong(i, generatorFrame.getLong(i));
131+
} else if (generatorFrame.isLong(frameIndex)) {
132+
virtualFrame.setLong(frameIndex, generatorFrame.getLong(frameIndex));
131133
frameSlotTypes[i] = FrameSlotType.Long;
132-
} else if (generatorFrame.isDouble(i)) {
133-
virtualFrame.setDouble(i, generatorFrame.getDouble(i));
134+
} else if (generatorFrame.isDouble(frameIndex)) {
135+
virtualFrame.setDouble(frameIndex, generatorFrame.getDouble(frameIndex));
134136
frameSlotTypes[i] = FrameSlotType.Double;
135-
} else if (generatorFrame.isBoolean(i)) {
136-
virtualFrame.setBoolean(i, generatorFrame.getBoolean(i));
137+
} else if (generatorFrame.isBoolean(frameIndex)) {
138+
virtualFrame.setBoolean(frameIndex, generatorFrame.getBoolean(frameIndex));
137139
frameSlotTypes[i] = FrameSlotType.Boolean;
138140
} else {
139141
throw new IllegalStateException("unexpected frame slot type");
140142
}
141143
}
142144
}
143145

144-
@ExplodeLoop
145-
private void copyFrameSlotsToGeneratorFrame(VirtualFrame virtualFrame, MaterializedFrame generatorFrame) {
146-
int stackTop = getFrameDescriptor().getNumberOfSlots();
147-
CompilerAsserts.partialEvaluationConstant(stackTop);
148-
for (int i = 0; i < stackTop; i++) {
149-
if (virtualFrame.isObject(i)) {
150-
generatorFrame.setObject(i, virtualFrame.getObject(i));
151-
} else if (virtualFrame.isInt(i)) {
152-
generatorFrame.setInt(i, virtualFrame.getInt(i));
153-
} else if (virtualFrame.isLong(i)) {
154-
generatorFrame.setLong(i, virtualFrame.getLong(i));
155-
} else if (virtualFrame.isDouble(i)) {
156-
generatorFrame.setDouble(i, virtualFrame.getDouble(i));
157-
} else if (virtualFrame.isBoolean(i)) {
158-
generatorFrame.setBoolean(i, virtualFrame.getBoolean(i));
159-
} else {
160-
throw CompilerDirectives.shouldNotReachHere("unexpected frame slot type");
161-
}
162-
}
163-
}
164-
165146
private void profileFrameSlots(MaterializedFrame generatorFrame) {
166147
CompilerAsserts.neverPartOfCompilation();
148+
int offset = rootNode.stackoffset;
167149
for (int i = 0; i < frameSlotTypes.length; i++) {
168-
if (generatorFrame.isObject(i)) {
150+
if (generatorFrame.isObject(offset + i)) {
169151
frameSlotTypes[i] = FrameSlotType.Object;
170-
} else if (generatorFrame.isInt(i)) {
152+
} else if (generatorFrame.isInt(offset + i)) {
171153
frameSlotTypes[i] = FrameSlotType.Int;
172-
} else if (generatorFrame.isLong(i)) {
154+
} else if (generatorFrame.isLong(offset + i)) {
173155
frameSlotTypes[i] = FrameSlotType.Long;
174-
} else if (generatorFrame.isDouble(i)) {
156+
} else if (generatorFrame.isDouble(offset + i)) {
175157
frameSlotTypes[i] = FrameSlotType.Double;
176-
} else if (generatorFrame.isBoolean(i)) {
158+
} else if (generatorFrame.isBoolean(offset + i)) {
177159
frameSlotTypes[i] = FrameSlotType.Boolean;
178160
} else {
179161
throw new IllegalStateException("unexpected frame slot type");
@@ -185,21 +167,17 @@ private void profileFrameSlots(MaterializedFrame generatorFrame) {
185167
public Object executeOSR(VirtualFrame osrFrame, int target, Object interpreterState) {
186168
Integer osrStackTop = (Integer) interpreterState;
187169
MaterializedFrame generatorFrame = PArguments.getGeneratorFrame(osrFrame);
188-
copyFrameSlotsIntoVirtualFrame(generatorFrame, osrFrame);
170+
copyStackSlotsIntoVirtualFrame(generatorFrame, osrFrame);
189171
copyOSRStackRemainderIntoVirtualFrame(generatorFrame, osrFrame, osrStackTop);
190-
try {
191-
return rootNode.executeFromBci(osrFrame, osrFrame, this, target, osrStackTop);
192-
} finally {
193-
copyFrameSlotsToGeneratorFrame(osrFrame, generatorFrame);
194-
}
172+
return rootNode.executeFromBci(osrFrame, generatorFrame, osrFrame, this, target, osrStackTop);
195173
}
196174

197175
@ExplodeLoop
198176
private void copyOSRStackRemainderIntoVirtualFrame(MaterializedFrame generatorFrame, VirtualFrame osrFrame, int stackTop) {
199177
/*
200-
* In addition to local variables and stack slots present at resume, OSR needs to also
201-
* revirtualize stack items that have been pushed since resume. Stack slots at a back edge
202-
* should never be primitives.
178+
* In addition to stack slots present at resume, OSR needs to also re-virtualize stack items
179+
* that have been pushed since resume. Stack slots at a back edge should never be
180+
* primitives.
203181
*/
204182
for (int i = resumeStackTop; i <= stackTop; i++) {
205183
osrFrame.setObject(i, generatorFrame.getObject(i));
@@ -213,26 +191,34 @@ public Object execute(VirtualFrame frame) {
213191
/*
214192
* This copying of exceptions is necessary because we need to remember the exception state
215193
* in the generator, but we don't want to remember the state that is "inherited" from the
216-
* outer frame as that can change with each invocation.
194+
* outer frame as that can change with each invocation. The values are copied back in
195+
* YIELD_VALUE (because the stack pointer is still PE-constant there, so we can explode the
196+
* loop). In interpreter, we use the materialized frame directly, but we still profile the
197+
* incoming types.
217198
*/
218199
PException localException = PArguments.getException(generatorFrame);
219200
PException outerException = PArguments.getException(frame);
220201
PArguments.setException(frame, localException == null ? outerException : localException);
221-
Frame localFrame;
222-
boolean usingMaterializedFrame = CompilerDirectives.inInterpreter();
223-
if (usingMaterializedFrame) {
202+
Frame stackFrame;
203+
/*
204+
* Using the materialized frame as stack would be bad for compiled performance, so we copy
205+
* the stack slots back to the virtual frame and use that as the stack in compiled code. The
206+
* values are copied back in yield node.
207+
*
208+
* TODO we could try to re-virtualize the locals too, but we would need to profile the loads
209+
* and stores to only copy what is actually used, otherwise copying everything makes things
210+
* worse.
211+
*/
212+
if (CompilerDirectives.inInterpreter()) {
224213
profileFrameSlots(generatorFrame);
225-
localFrame = generatorFrame;
214+
stackFrame = generatorFrame;
226215
} else {
227-
copyFrameSlotsIntoVirtualFrame(generatorFrame, frame);
228-
localFrame = frame;
216+
copyStackSlotsIntoVirtualFrame(generatorFrame, frame);
217+
stackFrame = frame;
229218
}
230219
try {
231-
return rootNode.executeFromBci(frame, generatorFrame, this, resumeBci, resumeStackTop);
220+
return rootNode.executeFromBci(frame, generatorFrame, stackFrame, this, resumeBci, resumeStackTop);
232221
} finally {
233-
if (!usingMaterializedFrame) {
234-
copyFrameSlotsToGeneratorFrame(frame, generatorFrame);
235-
}
236222
calleeContext.exit(frame, this);
237223
PException exception = PArguments.getException(frame);
238224
if (exception != outerException && exception != PException.NO_EXCEPTION) {

0 commit comments

Comments
 (0)