Skip to content

Commit 20e0ab1

Browse files
steve-ssteve-s
committed
[GR-71260] MaterializeFrameNode: fix PE error, minor footprint improvements, test_indirect_call works if assertions are disabled.
PullRequest: graalpython/4091
2 parents 3b09630 + a58aada commit 20e0ab1

File tree

3 files changed

+45
-48
lines changed

3 files changed

+45
-48
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_indirect_call.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,17 @@
5656
STABILIZES_AT = 1 if sys.implementation.name != "graalpy" or __graalpython__.truffle_runtime == 'Interpreted' else 10
5757
STABILIZES_AT = int(os.environ.get('GRAALPY_TEST_INDIRECT_CALL_STABILIZES_AT', STABILIZES_AT))
5858

59+
has_stack_walk_check = False
60+
if sys.implementation.name == "graalpy":
61+
result = __graalpython__.was_stack_walk(False)
62+
if result is None:
63+
print("NOTE: assertions are not enabled; test_indirect_call cannot check for "
64+
"repeated stack walks and will perform only basic sanity checks")
65+
else:
66+
has_stack_walk_check = True
67+
5968
def was_stack_walk(new_value):
60-
if sys.implementation.name == "graalpy":
69+
if has_stack_walk_check:
6170
return __graalpython__.was_stack_walk(new_value)
6271
return False
6372

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/GraalPythonModuleBuiltins.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,8 +637,9 @@ public Object doBoundary(Object value) {
637637
boolean assertionsEnabled = false;
638638
assert assertionsEnabled = true;
639639
if (!assertionsEnabled) {
640-
// It does not make sense to run with if assertions are not enabled
641-
throw PRaiseNode.raiseStatic(this, PythonBuiltinClassType.SystemError);
640+
// None indicates that assertions are disabled and this functionality is not
641+
// available
642+
return PNone.NONE;
642643
}
643644

644645
boolean prev = PythonContext.get(this).wasStackWalk;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java‎

Lines changed: 32 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@
4747
import com.oracle.graal.python.nodes.PRootNode;
4848
import com.oracle.graal.python.nodes.bytecode.BytecodeFrameInfo;
4949
import com.oracle.graal.python.nodes.bytecode.FrameInfo;
50-
import com.oracle.graal.python.nodes.bytecode_dsl.BytecodeDSLFrameInfo;
5150
import com.oracle.graal.python.nodes.bytecode_dsl.PBytecodeDSLRootNode;
5251
import com.oracle.graal.python.runtime.PythonOptions;
5352
import com.oracle.graal.python.runtime.object.PFactory;
5453
import com.oracle.graal.python.util.PythonUtils;
54+
import com.oracle.truffle.api.CompilerAsserts;
5555
import com.oracle.truffle.api.Truffle;
5656
import com.oracle.truffle.api.bytecode.BytecodeNode;
5757
import com.oracle.truffle.api.dsl.Bind;
@@ -67,10 +67,10 @@
6767
import com.oracle.truffle.api.frame.Frame;
6868
import com.oracle.truffle.api.frame.FrameDescriptor;
6969
import com.oracle.truffle.api.frame.MaterializedFrame;
70-
import com.oracle.truffle.api.nodes.ExplodeLoop;
7170
import com.oracle.truffle.api.nodes.Node;
72-
import com.oracle.truffle.api.nodes.RootNode;
7371
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
72+
import com.oracle.truffle.api.profiles.InlinedIntValueProfile;
73+
import com.oracle.truffle.api.profiles.ValueProfile;
7474

7575
/**
7676
* This node makes sure that the current frame has a filled-in PFrame object with a backref
@@ -125,14 +125,17 @@ public final PFrame executeOnStack(Frame frameToMaterialize, PRootNode location,
125125
* root nodes, we fetch the root node from the frame descriptor.
126126
*/
127127
public final PFrame executeOnStack(boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize) {
128-
Node location;
129-
RootNode rootNode = PArguments.getCurrentFrameInfo(frameToMaterialize).getRootNode();
130-
if (PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER && PGenerator.unwrapContinuationRoot(rootNode) instanceof PBytecodeDSLRootNode) {
131-
assert this.isAdoptable();
132-
location = BytecodeNode.get(this);
133-
assert location != null;
128+
Node location = this;
129+
if (!PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER) {
130+
if (!location.isAdoptable()) {
131+
// This should only happen in uncached manual bytecode interpreter, we are fine with
132+
// root node in such case
133+
location = PArguments.getCurrentFrameInfo(frameToMaterialize).getRootNode();
134+
}
134135
} else {
135-
location = rootNode;
136+
// We will need EncapsulatingNodeReference or thread the BytecodeNode as argument for
137+
// BytecodeDSL uncached execution
138+
assert this.isAdoptable();
136139
}
137140
return execute(location, markAsEscaped, forceSync, frameToMaterialize);
138141
}
@@ -155,29 +158,17 @@ public final PFrame execute(Node location, boolean markAsEscaped, boolean forceS
155158

156159
abstract PFrame executeImpl(Node location, boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize);
157160

158-
@Specialization(guards = {
159-
"cachedFD == frameToMaterialize.getFrameDescriptor()", //
160-
"getPFrame(frameToMaterialize) == null", //
161-
"!isGeneratorFrame(frameToMaterialize)", //
162-
"!hasCustomLocals(frameToMaterialize)"}, limit = "1")
161+
@Specialization(guards = {"getPFrame(frameToMaterialize) == null", "!isGeneratorFrame(frameToMaterialize)", "!hasCustomLocals(frameToMaterialize)"})
163162
static PFrame freshPFrameCachedFD(Node location, boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize,
164-
@Cached(value = "frameToMaterialize.getFrameDescriptor()") FrameDescriptor cachedFD,
165163
@Bind PythonLanguage language,
164+
@Cached(inline = false) ValueProfile cachedFDProfile,
166165
@Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode) {
166+
FrameDescriptor cachedFD = cachedFDProfile.profile(frameToMaterialize.getFrameDescriptor());
167167
MaterializedFrame locals = createLocalsFrame(cachedFD);
168168
PFrame escapedFrame = PFactory.createPFrame(language, PArguments.getCurrentFrameInfo(frameToMaterialize), location, locals);
169169
return doEscapeFrame(frameToMaterialize, escapedFrame, markAsEscaped, forceSync, location, syncValuesNode);
170170
}
171171

172-
@Specialization(guards = {"getPFrame(frameToMaterialize) == null", "!isGeneratorFrame(frameToMaterialize)", "!hasCustomLocals(frameToMaterialize)"}, replaces = "freshPFrameCachedFD")
173-
static PFrame freshPFrame(Node location, boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize,
174-
@Bind PythonLanguage language,
175-
@Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode) {
176-
MaterializedFrame locals = createLocalsFrame(frameToMaterialize.getFrameDescriptor());
177-
PFrame escapedFrame = PFactory.createPFrame(language, PArguments.getCurrentFrameInfo(frameToMaterialize), location, locals);
178-
return doEscapeFrame(frameToMaterialize, escapedFrame, markAsEscaped, forceSync, location, syncValuesNode);
179-
}
180-
181172
@Specialization(guards = {"getPFrame(frameToMaterialize) == null", "!isGeneratorFrame(frameToMaterialize)", "hasCustomLocals(frameToMaterialize)"})
182173
static PFrame freshPFrameCusstomLocals(Node location, boolean markAsEscaped, @SuppressWarnings("unused") boolean forceSync,
183174
Frame frameToMaterialize,
@@ -203,7 +194,7 @@ static PFrame alreadyEscapedFrame(@SuppressWarnings("unused") Node location, boo
203194
@Cached InlinedConditionProfile syncProfile) {
204195
PFrame pyFrame = getPFrame(frameToMaterialize);
205196
if (syncProfile.profile(inliningTarget, forceSync && !PGenerator.isGeneratorFrame(frameToMaterialize))) {
206-
syncValuesNode.execute(pyFrame, frameToMaterialize);
197+
syncValuesNode.execute(pyFrame, frameToMaterialize, location);
207198
}
208199
if (markAsEscaped) {
209200
pyFrame.getRef().markAsEscaped();
@@ -231,6 +222,7 @@ private static void processBytecodeFrame(Frame frameToMaterialize, PFrame pyFram
231222
if (PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER) {
232223
BytecodeNode bytecodeNode;
233224
assert !(location instanceof PBytecodeDSLRootNode); // we need BytecodeNode or its child
225+
CompilerAsserts.partialEvaluationConstant(location);
234226
bytecodeNode = BytecodeNode.get(location);
235227
if (bytecodeNode != null) {
236228
pyFrame.setBci(bytecodeNode.getBytecodeIndex(frameToMaterialize));
@@ -255,7 +247,7 @@ private static PFrame doEscapeFrame(Frame frameToMaterialize, PFrame escapedFram
255247
// on a freshly created PFrame, we do always sync the arguments
256248
PArguments.synchronizeArgs(frameToMaterialize, escapedFrame);
257249
if (forceSync) {
258-
syncValuesNode.execute(escapedFrame, frameToMaterialize);
250+
syncValuesNode.execute(escapedFrame, frameToMaterialize, location);
259251
}
260252
if (markAsEscaped) {
261253
topFrameRef.markAsEscaped();
@@ -282,37 +274,32 @@ protected static PFrame getPFrame(Frame frame) {
282274
@GenerateUncached
283275
public abstract static class SyncFrameValuesNode extends Node {
284276

285-
public abstract void execute(PFrame pyFrame, Frame frameToSync);
277+
public abstract void execute(PFrame pyFrame, Frame frameToSync, Node location);
286278

287-
@Specialization(guards = {"!pyFrame.hasCustomLocals()",
288-
"frameToSync.getFrameDescriptor() == cachedFd"}, limit = "1")
289-
@ExplodeLoop
290-
static void doSyncCached(PFrame pyFrame, Frame frameToSync,
291-
@Cached(value = "frameToSync.getFrameDescriptor()") FrameDescriptor cachedFd) {
279+
@Specialization(guards = "!pyFrame.hasCustomLocals()")
280+
static void doSync(PFrame pyFrame, Frame frameToSync, Node location,
281+
@Bind Node inliningTarget,
282+
@Cached(inline = false) ValueProfile frameDescriptorProfile,
283+
@Cached InlinedIntValueProfile slotCountProfile) {
284+
FrameDescriptor cachedFd = frameDescriptorProfile.profile(frameToSync.getFrameDescriptor());
292285
MaterializedFrame target = pyFrame.getLocals();
293286
assert cachedFd == target.getFrameDescriptor();
294-
int slotCount = variableSlotCount(cachedFd);
287+
int slotCount = slotCountProfile.profile(inliningTarget, variableSlotCount(cachedFd));
295288

296289
if (PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER) {
297-
FrameInfo info = (FrameInfo) cachedFd.getInfo();
298-
if (info instanceof BytecodeDSLFrameInfo bytecodeDSLFrameInfo) {
299-
PBytecodeDSLRootNode rootNode = bytecodeDSLFrameInfo.getRootNode();
300-
rootNode.getBytecodeNode().copyLocalValues(0, frameToSync, target, 0, slotCount);
290+
CompilerAsserts.partialEvaluationConstant(location);
291+
BytecodeNode bytecodeNode = BytecodeNode.get(location);
292+
if (bytecodeNode != null) {
293+
bytecodeNode.copyLocalValues(0, frameToSync, target, 0, slotCount);
301294
}
302295
} else {
303296
frameToSync.copyTo(0, target, 0, slotCount);
304297
}
305298
}
306299

307-
@Specialization(guards = "!pyFrame.hasCustomLocals()", replaces = "doSyncCached")
308-
@ExplodeLoop
309-
static void doSync(PFrame pyFrame, Frame frameToSync) {
310-
doSyncCached(pyFrame, frameToSync, frameToSync.getFrameDescriptor());
311-
}
312-
313300
@Specialization(guards = "pyFrame.hasCustomLocals()")
314301
@SuppressWarnings("unused")
315-
static void doCustomLocals(PFrame pyFrame, Frame frameToSync) {
302+
static void doCustomLocals(PFrame pyFrame, Frame frameToSync, Node location) {
316303
// nothing to do
317304
}
318305

0 commit comments

Comments
 (0)