Skip to content

Commit 8ff70a6

Browse files
zslajchrtzslajchrt
committed
[GR-14065] GetCallerFrameNode refactored.
PullRequest: fastr/1962
2 parents f42ee8e + bafaa59 commit 8ff70a6

File tree

22 files changed

+311
-88
lines changed

22 files changed

+311
-88
lines changed

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/RfEvalNode.java‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import com.oracle.truffle.api.dsl.Fallback;
3333
import com.oracle.truffle.api.dsl.Specialization;
3434
import com.oracle.truffle.api.frame.Frame;
35+
import com.oracle.truffle.api.frame.FrameInstance.FrameAccess;
36+
import com.oracle.truffle.api.frame.MaterializedFrame;
3537
import com.oracle.truffle.api.profiles.ConditionProfile;
3638
import com.oracle.truffle.api.profiles.ValueProfile;
3739
import com.oracle.truffle.r.nodes.access.variables.ReadVariableNode;
@@ -41,12 +43,13 @@
4143
import com.oracle.truffle.r.runtime.RCaller;
4244
import com.oracle.truffle.r.runtime.RError;
4345
import com.oracle.truffle.r.runtime.Utils;
46+
import com.oracle.truffle.r.runtime.VirtualEvalFrame;
4447
import com.oracle.truffle.r.runtime.context.RContext;
4548
import com.oracle.truffle.r.runtime.data.RExpression;
4649
import com.oracle.truffle.r.runtime.data.RFunction;
47-
import com.oracle.truffle.r.runtime.data.RPairList;
4850
import com.oracle.truffle.r.runtime.data.RList;
4951
import com.oracle.truffle.r.runtime.data.RNull;
52+
import com.oracle.truffle.r.runtime.data.RPairList;
5053
import com.oracle.truffle.r.runtime.data.RPromise;
5154
import com.oracle.truffle.r.runtime.data.RSymbol;
5255
import com.oracle.truffle.r.runtime.env.REnvironment;
@@ -64,7 +67,22 @@ public static RfEvalNode create() {
6467
private static RCaller createCall(REnvironment env) {
6568
// TODO: getActualCurrentFrame causes deopt
6669
Frame frame = Utils.getActualCurrentFrame();
67-
RCaller originalCaller = RArguments.getCall(env.getFrame());
70+
final MaterializedFrame envFrame = env.getFrame();
71+
RCaller originalCaller = RArguments.getCall(envFrame);
72+
if (!RCaller.isValidCaller(originalCaller) && env instanceof REnvironment.NewEnv) {
73+
// Try to find the valid original caller stored in the original frame of a
74+
// VirtualEvalFrame that is the same as envFrame
75+
RCaller validOrigCaller = Utils.iterateRFrames(FrameAccess.READ_ONLY, (f) -> {
76+
if (f instanceof VirtualEvalFrame && ((VirtualEvalFrame) f).getOriginalFrame() == envFrame) {
77+
return RArguments.getCall(f);
78+
} else {
79+
return null;
80+
}
81+
});
82+
if (validOrigCaller != null) {
83+
originalCaller = validOrigCaller;
84+
}
85+
}
6886
RCaller currentCaller = RArguments.getCall(frame);
6987
if (env == REnvironment.globalEnv(RContext.getInstance())) {
7088
return RCaller.createForPromise(originalCaller, currentCaller);

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/BrowserFunctions.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ protected RNull browser(VirtualFrame frame, Object text, Object condition, RProm
104104
if (fun != null && fun.isBuiltin() && fun.getRBuiltin().getBuiltinNodeClass() == BrowserNode.class) {
105105
if (getCallerFrame == null) {
106106
CompilerDirectives.transferToInterpreterAndInvalidate();
107-
getCallerFrame = insert(new GetCallerFrameNode());
107+
getCallerFrame = insert(GetCallerFrameNode.create());
108108
}
109109
actualFrame = getCallerFrame.execute(mFrame);
110110
caller = caller.getPrevious();

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/DoCall.java‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,10 @@ protected Object doCall(VirtualFrame frame, RFunction func, RList argsAsList, bo
131131
* GNU-R translates {@code do.call} simply to {@code eval}, which has consequences w.r.t. how
132132
* the stack should look like. The stack frame of {@code do.call} and all the frames underneath
133133
* it should be visible to {@code sys.frame}, so the caller frame passed to the callee via
134-
* arguments (see {@link com.oracle.truffle.r.nodes.function.call.CallRFunctionBaseNode}) should
135-
* be the execution frame of do.call (not the frame where the function should be evaluated given
136-
* as an argument to do.call) so that the stack walking via caller frames does not skip
137-
* {@code do.call}.
134+
* arguments (see {@link com.oracle.truffle.r.nodes.function.call.CallerFrameClosureProvider})
135+
* should be the execution frame of do.call (not the frame where the function should be
136+
* evaluated given as an argument to do.call) so that the stack walking via caller frames does
137+
* not skip {@code do.call}.
138138
*/
139139
@ImportStatic(DSLConfig.class)
140140
protected abstract static class DoCallInternal extends Node {
@@ -174,9 +174,10 @@ public Object doFastPathInEvalFrame(VirtualFrame virtualFrame, String funcName,
174174
MaterializedFrame promiseFrame = frameProfile.profile(env.getFrame(frameAccessProfile)).materialize();
175175
RArgsValuesAndNames args = getArguments(languagesClosureCache, symbolsClosureCache, promiseFrame, quote, quoteProfile, containsRSymbolProfile, argsAsList);
176176
RCaller caller = getExplicitCaller(virtualFrame, promiseFrame, env, funcName, func, args);
177-
MaterializedFrame evalFrame = getEvalFrame(virtualFrame, promiseFrame);
177+
MaterializedFrame materializedFrame = virtualFrame.materialize();
178+
MaterializedFrame evalFrame = getEvalFrame(materializedFrame, promiseFrame);
178179

179-
Object resultValue = explicitCallNode.execute(evalFrame, func, args, caller, virtualFrame.materialize());
180+
Object resultValue = explicitCallNode.execute(evalFrame, func, args, caller, materializedFrame);
180181
setVisibility(virtualFrame, getVisibilityNode.execute(evalFrame));
181182
return resultValue;
182183
}
@@ -194,9 +195,10 @@ public Object doSlowPathInEvalFrame(VirtualFrame virtualFrame, String funcName,
194195
RArgsValuesAndNames args = getArguments(null, null, promiseFrame, quote, quoteProfile,
195196
containsRSymbolProfile, argsAsList);
196197
RCaller caller = getExplicitCaller(virtualFrame, promiseFrame, env, funcName, func, args);
197-
MaterializedFrame evalFrame = getEvalFrame(virtualFrame, promiseFrame);
198+
MaterializedFrame materializedFrame = virtualFrame.materialize();
199+
MaterializedFrame evalFrame = getEvalFrame(materializedFrame, promiseFrame);
198200

199-
Object resultValue = slowPathExplicitCall.execute(evalFrame, virtualFrame.materialize(), caller, func, args);
201+
Object resultValue = slowPathExplicitCall.execute(evalFrame, materializedFrame, caller, func, args);
200202
setVisibility(virtualFrame, getVisibilitySlowPath(evalFrame));
201203
return resultValue;
202204
}
@@ -206,9 +208,12 @@ public Object doSlowPathInEvalFrame(VirtualFrame virtualFrame, String funcName,
206208
* the same time some primitives expect to see {@code do.call(foo, ...)} as the caller, so
207209
* we create a frame the fakes caller, but otherwise delegates to the frame backing the
208210
* explicitly given environment.
211+
*
212+
* @param currentFrame the current materialized frame, which is set as the caller frame
213+
* @param envFrame the frame from which the clone is made
209214
*/
210-
private static MaterializedFrame getEvalFrame(VirtualFrame virtualFrame, MaterializedFrame envFrame) {
211-
return VirtualEvalFrame.create(envFrame, RArguments.getFunction(virtualFrame), RArguments.getCall(virtualFrame));
215+
private static MaterializedFrame getEvalFrame(VirtualFrame currentFrame, MaterializedFrame envFrame) {
216+
return VirtualEvalFrame.create(envFrame, RArguments.getFunction(currentFrame), currentFrame, RArguments.getCall(currentFrame));
212217
}
213218

214219
/**

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/EnvFunctions.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ protected REnvironment asEnvironment(REnvironment env) {
118118

119119
@Specialization
120120
protected Object asEnvironmentInt(VirtualFrame frame, RAbstractIntVector pos,
121-
@Cached("new()") GetCallerFrameNode getCallerFrame) {
121+
@Cached("create()") GetCallerFrameNode getCallerFrame) {
122122
if (pos.getLength() == 0) {
123123
CompilerDirectives.transferToInterpreter();
124124
throw error(Message.INVALID_ARGUMENT, "pos");
@@ -346,7 +346,7 @@ public abstract static class Environment extends RBuiltinNode.Arg1 {
346346

347347
@Specialization
348348
protected Object environment(VirtualFrame frame, @SuppressWarnings("unused") RNull fun,
349-
@Cached("new()") GetCallerFrameNode callerFrame,
349+
@Cached("create()") GetCallerFrameNode callerFrame,
350350
@Cached("new()") PromiseDeoptimizeFrameNode deoptFrameNode) {
351351
MaterializedFrame matFrame = callerFrame.execute(frame);
352352

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/FrameFunctions.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,7 @@ private FrameHelper getFrameHelper() {
10001000
private GetCallerFrameNode getCallerFrameNode() {
10011001
if (getCaller == null) {
10021002
CompilerDirectives.transferToInterpreterAndInvalidate();
1003-
getCaller = insert(new GetCallerFrameNode());
1003+
getCaller = insert(GetCallerFrameNode.create());
10041004
}
10051005
return getCaller;
10061006
}

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/MatchFun.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ abstract static class MatchFunInternal extends RBaseNode {
9393
private final MatchFun outer;
9494

9595
private final BranchProfile needsMaterialize = BranchProfile.create();
96-
@Child private GetCallerFrameNode getCallerFrame = new GetCallerFrameNode();
96+
@Child private GetCallerFrameNode getCallerFrame = GetCallerFrameNode.create();
9797

9898
MatchFunInternal(MatchFun outer) {
9999
this.outer = outer;

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/PosToEnv.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -50,7 +50,7 @@ public static PosToEnv create() {
5050

5151
@Specialization(guards = "isMinusOne(x)")
5252
protected Object doPosToEnvMinusOne(VirtualFrame frame, @SuppressWarnings("unused") int x,
53-
@Cached("new()") GetCallerFrameNode callerFrameNode) {
53+
@Cached("create()") GetCallerFrameNode callerFrameNode) {
5454
if (REnvironment.isGlobalEnvFrame(frame)) {
5555
throw error(Message.NO_ENCLOSING_ENVIRONMENT);
5656
}

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/Recall.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -48,7 +48,7 @@ public abstract class Recall extends RBuiltinNode.Arg1 {
4848

4949
@Child private LocalReadVariableNode readArgs = LocalReadVariableNode.create(ArgumentsSignature.VARARG_NAME, false);
5050

51-
@Child private GetCallerFrameNode callerFrame = new GetCallerFrameNode();
51+
@Child private GetCallerFrameNode callerFrame = GetCallerFrameNode.create();
5252

5353
@Child private RExplicitCallNode call = RExplicitCallNode.create();
5454

‎com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/S3DispatchFunctions.java‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,12 @@ private static final class Helper extends RBaseNode {
7575
private final ConditionProfile isOpsGeneric = ConditionProfile.createBinaryProfile();
7676
@CompilationFinal private ValueProfile dotMethodClassProfile;
7777
@Child private LocalReadVariableNode rvnMethod;
78+
private final boolean nextMethod;
7879

7980
protected Helper(boolean nextMethod) {
80-
methodLookup = S3FunctionLookupNode.create(true, nextMethod);
81-
callMatcher = CallMatcherNode.create(false);
81+
this.nextMethod = nextMethod;
82+
this.methodLookup = S3FunctionLookupNode.create(true, nextMethod);
83+
this.callMatcher = CallMatcherNode.create(false);
8284
}
8385

8486
protected Object dispatch(VirtualFrame frame, RCaller parentCaller, String generic, RStringVector type, String group, MaterializedFrame callerFrame, MaterializedFrame genericDefFrame,
@@ -90,7 +92,27 @@ protected Object dispatch(VirtualFrame frame, RCaller parentCaller, String gener
9092
dotMethod = patchDotMethod(frame, lookupResult, dotMethod);
9193
}
9294
S3Args s3Args = lookupResult.createS3Args(dotMethod, callerFrame, genericDefFrame, group);
93-
Object result = callMatcher.execute(frame, parentCaller, RArguments.getCall(callerFrame), suppliedSignature, suppliedArguments, lookupResult.function, lookupResult.targetFunctionName,
95+
RCaller dispatchCaller = RArguments.getCall(callerFrame);
96+
97+
if (!RCaller.isValidCaller(dispatchCaller)) {
98+
// If callerFrame does not contain a valid caller, take the logical grand-parent of
99+
// parentCaller as the dispatch parent
100+
RCaller tmpCaller = parentCaller.getLogicalParent();
101+
tmpCaller = tmpCaller != null ? tmpCaller.getLogicalParent() : null;
102+
dispatchCaller = tmpCaller != null ? tmpCaller : dispatchCaller;
103+
}
104+
RCaller actualParentCaller = parentCaller;
105+
if (!nextMethod && actualParentCaller.getPrevious() != dispatchCaller &&
106+
RCaller.isValidCaller(dispatchCaller) && !actualParentCaller.isPromise()) {
107+
// If dispatchCaller differs from the previous caller of actualParentCaller, create
108+
// a new actualParentCaller with the dispatchCaller as the logical parent. It
109+
// guarantees that the S3 generic method and a specific method have the same logical
110+
// parents. NB: In the case of NextMethod, the logical parent of parentCaller should
111+
// be the same as dispatchCaller thanks to using
112+
// RCaller.createForGenericFunctionCall.
113+
actualParentCaller = actualParentCaller.withLogicalParent(dispatchCaller);
114+
}
115+
Object result = callMatcher.execute(frame, actualParentCaller, dispatchCaller, suppliedSignature, suppliedArguments, lookupResult.function, lookupResult.targetFunctionName,
94116
s3Args);
95117
return result;
96118
}
@@ -145,7 +167,7 @@ public abstract static class UseMethod extends RBuiltinNode.Arg2 {
145167

146168
@Child private ClassHierarchyNode classHierarchyNode = ClassHierarchyNode.createForDispatch(true);
147169
@Child private PromiseCheckHelperNode promiseCheckHelper;
148-
@Child private GetCallerFrameNode getCallerFrameNode = new GetCallerFrameNode();
170+
@Child private GetCallerFrameNode getCallerFrameNode = GetCallerFrameNode.create();
149171
@Child private Helper helper = new Helper(false);
150172

151173
private final BranchProfile firstArgMissing = BranchProfile.create();

‎com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/CallMatcherNode.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,13 @@
2121

2222
package com.oracle.truffle.r.nodes.function;
2323

24+
import static com.oracle.truffle.r.runtime.context.FastROptions.RestrictForceSplitting;
25+
2426
import java.util.function.Supplier;
2527

2628
import com.oracle.truffle.api.CompilerDirectives;
2729
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
2830
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
29-
import com.oracle.truffle.api.frame.MaterializedFrame;
3031
import com.oracle.truffle.api.frame.VirtualFrame;
3132
import com.oracle.truffle.api.nodes.ExplodeLoop;
3233
import com.oracle.truffle.api.nodes.NodeCost;
@@ -43,10 +44,8 @@
4344
import com.oracle.truffle.r.nodes.function.visibility.SetVisibilityNode;
4445
import com.oracle.truffle.r.runtime.ArgumentsSignature;
4546
import com.oracle.truffle.r.runtime.DSLConfig;
46-
import static com.oracle.truffle.r.runtime.context.FastROptions.RestrictForceSplitting;
4747
import com.oracle.truffle.r.runtime.RArguments;
4848
import com.oracle.truffle.r.runtime.RArguments.DispatchArgs;
49-
import com.oracle.truffle.r.runtime.RArguments.S3Args;
5049
import com.oracle.truffle.r.runtime.RCaller;
5150
import com.oracle.truffle.r.runtime.RInternalError;
5251
import com.oracle.truffle.r.runtime.RVisibility;
@@ -291,9 +290,8 @@ public Object execute(VirtualFrame frame, RCaller parentCaller, RCaller dispatch
291290
} else {
292291
caller = RCaller.createForGenericFunctionCall(parent, argsSupplier, parentCaller != null ? parentCaller : RArguments.getCall(frame));
293292
}
294-
MaterializedFrame callerFrame = dispatchArgs instanceof S3Args ? ((S3Args) dispatchArgs).callEnv : null;
295293
try {
296-
return call.execute(frame, cachedFunction, caller, callerFrame, reorderedArgs, matchedArgs.getSignature(), cachedFunction.getEnclosingFrame(), dispatchArgs);
294+
return call.execute(frame, cachedFunction, caller, null, reorderedArgs, matchedArgs.getSignature(), cachedFunction.getEnclosingFrame(), dispatchArgs);
297295
} finally {
298296
visibility.executeAfterCall(frame, caller);
299297
}
@@ -381,11 +379,10 @@ public Object execute(VirtualFrame frame, RCaller parentCaller, RCaller dispatch
381379
caller = RCaller.createForGenericFunctionCall(parent, argsSupplier, parentCaller != null ? parentCaller : RArguments.getCall(frame));
382380
}
383381

384-
MaterializedFrame callerFrame = (dispatchArgs instanceof S3Args) ? ((S3Args) dispatchArgs).callEnv : null;
385382
if (function.isBuiltin()) {
386383
return callRBuiltin.execute(frame, function, reorderedArgs.getArguments());
387384
} else {
388-
return call.execute(frame, function, caller, callerFrame, reorderedArgs.getArguments(), reorderedArgs.getSignature(), function.getEnclosingFrame(), dispatchArgs);
385+
return call.execute(frame, function, caller, null, reorderedArgs.getArguments(), reorderedArgs.getSignature(), function.getEnclosingFrame(), dispatchArgs);
389386
}
390387
}
391388

0 commit comments

Comments
 (0)