Skip to content

Commit d867023

Browse files
steve-ssteve-s
committed
Refactor ExternalFunctionInvokeNode
Into bare ExternalFunctionInvokeNode that doesn't do any result conversions and convenience helper ExternalFunctionWrapperInvokeNode that converts the result according to the provided PExternalFunctionWrapper.
1 parent 9b83dbd commit d867023

File tree

6 files changed

+90
-38
lines changed

6 files changed

+90
-38
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/ExternalFunctionNodes.java

Lines changed: 78 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.ReleaseNativeWrapperNodeGen;
7373
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.CreateArgsTupleNodeGen;
7474
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.DefaultCheckFunctionResultNodeGen;
75+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.ExternalFunctionInvokeNodeGen;
7576
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.MaterializePrimitiveNodeGen;
7677
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.ReleaseNativeSequenceStorageNodeGen;
7778
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper;
@@ -143,6 +144,7 @@
143144
import com.oracle.truffle.api.dsl.Cached.Exclusive;
144145
import com.oracle.truffle.api.dsl.Cached.Shared;
145146
import com.oracle.truffle.api.dsl.Fallback;
147+
import com.oracle.truffle.api.dsl.GenerateCached;
146148
import com.oracle.truffle.api.dsl.GenerateInline;
147149
import com.oracle.truffle.api.dsl.GenerateUncached;
148150
import com.oracle.truffle.api.dsl.ImportStatic;
@@ -820,46 +822,29 @@ public static MethDirectRoot create(PythonLanguage lang, TruffleString name, PEx
820822
* Like {@link com.oracle.graal.python.nodes.call.FunctionInvokeNode} but invokes a C function.
821823
*/
822824
@GenerateUncached
823-
@GenerateInline(false)
825+
@GenerateCached(false)
826+
@GenerateInline
824827
public abstract static class ExternalFunctionInvokeNode extends PNodeWithContext {
825-
public abstract Object execute(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments);
828+
abstract Object execute(VirtualFrame frame, Node inliningTarget, PythonContext ctx, PythonThreadState threadState, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments);
826829

827-
static CheckFunctionResultNode createCheckResultNode(PExternalFunctionWrapper provider) {
828-
CheckFunctionResultNode node = provider.createCheckFunctionResultNode();
829-
return node != null ? node : DefaultCheckFunctionResultNodeGen.create();
830+
public final Object call(VirtualFrame frame, Node inliningTarget, PythonContext ctx, PythonThreadState threadState, CApiTiming timing, TruffleString name, Object callable,
831+
Object... cArguments) {
832+
return execute(frame, inliningTarget, ctx, threadState, timing, name, callable, cArguments);
830833
}
831834

832-
static CheckFunctionResultNode getUncachedCheckResultNode(PExternalFunctionWrapper provider) {
833-
CheckFunctionResultNode node = provider.getUncachedCheckFunctionResultNode();
834-
return node != null ? node : DefaultCheckFunctionResultNodeGen.getUncached();
835-
}
836-
837-
@Specialization(guards = {"provider == cachedProvider"}, limit = "1")
838-
static Object invoke(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments,
839-
@Bind("this") Node inliningTarget,
840-
@Cached("provider") PExternalFunctionWrapper cachedProvider,
841-
@Cached(value = "createCheckResultNode(provider)", uncached = "getUncachedCheckResultNode(provider)") CheckFunctionResultNode checkResultNode,
842-
@Cached(value = "provider.createConvertRetNode()", uncached = "provider.getUncachedConvertRetNode()") CExtToJavaNode convertReturnValue,
843-
@Cached PForeignToPTypeNode fromForeign,
844-
@Cached GetThreadStateNode getThreadStateNode,
845-
@Cached GilNode gilNode,
835+
@Specialization
836+
static Object invoke(VirtualFrame frame, Node inliningTarget, PythonContext ctx, PythonThreadState threadState, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments,
837+
@Cached(inline = false) GilNode gilNode,
846838
@Cached(value = "createFor(this)", uncached = "getUncached()") IndirectCallData indirectCallData,
847839
@CachedLibrary(limit = "2") InteropLibrary lib) {
848-
PythonContext ctx = PythonContext.get(inliningTarget);
849-
PythonThreadState threadState = getThreadStateNode.execute(inliningTarget, ctx);
850840

851841
// If any code requested the caught exception (i.e. used 'sys.exc_info()'), we store
852842
// it to the context since we cannot propagate it through the native frames.
853843
Object state = IndirectCallContext.enter(frame, threadState, indirectCallData);
854844

855845
CApiTiming.enter();
856846
try {
857-
Object result = lib.execute(callable, cArguments);
858-
result = checkResultNode.execute(threadState, name, result);
859-
if (convertReturnValue != null) {
860-
result = convertReturnValue.execute(result);
861-
}
862-
return fromForeign.executeConvert(result);
847+
return lib.execute(callable, cArguments);
863848
} catch (UnsupportedTypeException | UnsupportedMessageException e) {
864849
CompilerDirectives.transferToInterpreterAndInvalidate();
865850
throw PRaiseNode.raiseUncached(inliningTarget, TypeError, ErrorMessages.CALLING_NATIVE_FUNC_FAILED, name, e);
@@ -873,7 +858,7 @@ static Object invoke(VirtualFrame frame, PExternalFunctionWrapper provider, CApi
873858
* extensions are releasing the GIL and if then an LLVM exception occurs, C code
874859
* wouldn't re-acquire it (unexpectedly).
875860
*/
876-
gilNode.acquire();
861+
gilNode.acquire(ctx);
877862

878863
/*
879864
* Special case after calling a C function: transfer caught exception back to frame
@@ -885,14 +870,73 @@ static Object invoke(VirtualFrame frame, PExternalFunctionWrapper provider, CApi
885870
IndirectCallContext.exit(frame, threadState, state);
886871
}
887872
}
873+
}
874+
875+
/**
876+
* Wraps {@link ExternalFunctionInvokeNode} with result checking and conversion according to the
877+
* passed {@link PExternalFunctionWrapper}. This node assumes that the provider argument is in
878+
* the cached case a PE constant.
879+
*/
880+
@GenerateInline(false)
881+
public abstract static class ExternalFunctionWrapperInvokeNode extends PNodeWithContext {
882+
public abstract Object execute(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments);
883+
884+
@NeverDefault
885+
static CheckFunctionResultNode createCheckResultNode(PExternalFunctionWrapper provider) {
886+
CheckFunctionResultNode node = provider.createCheckFunctionResultNode();
887+
return node != null ? node : DefaultCheckFunctionResultNodeGen.create();
888+
}
889+
890+
static CheckFunctionResultNode getUncachedCheckResultNode(PExternalFunctionWrapper provider) {
891+
CheckFunctionResultNode node = provider.getUncachedCheckFunctionResultNode();
892+
return node != null ? node : DefaultCheckFunctionResultNodeGen.getUncached();
893+
}
894+
895+
@Specialization
896+
static Object invokeCached(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments,
897+
@Bind("this") Node inliningTarget,
898+
@Cached("createCheckResultNode(provider)") CheckFunctionResultNode checkResultNode,
899+
@SuppressWarnings("truffle-neverdefault") @Cached("provider.createConvertRetNode()") CExtToJavaNode convertReturnValue,
900+
@Cached PForeignToPTypeNode fromForeign,
901+
@Cached GetThreadStateNode getThreadStateNode,
902+
@Cached ExternalFunctionInvokeNode invokeNode) {
903+
CompilerAsserts.partialEvaluationConstant(provider);
904+
PythonContext ctx = PythonContext.get(inliningTarget);
905+
return invoke(frame, ctx, timing, name, callable, cArguments, inliningTarget, checkResultNode, convertReturnValue, fromForeign, getThreadStateNode, invokeNode);
906+
}
907+
908+
private static Object invoke(VirtualFrame frame, PythonContext ctx, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments, Node inliningTarget,
909+
CheckFunctionResultNode checkResultNode, CExtToJavaNode convertReturnValue, PForeignToPTypeNode fromForeign, GetThreadStateNode getThreadStateNode,
910+
ExternalFunctionInvokeNode invokeNode) {
911+
PythonThreadState threadState = getThreadStateNode.execute(inliningTarget, ctx);
912+
Object result = invokeNode.execute(frame, inliningTarget, ctx, threadState, timing, name, callable, cArguments);
913+
result = checkResultNode.execute(threadState, name, result);
914+
if (convertReturnValue != null) {
915+
result = convertReturnValue.execute(result);
916+
}
917+
return fromForeign.executeConvert(result);
918+
}
919+
920+
@GenerateCached(false)
921+
private static final class ExternalFunctionWrapperInvokeNodeUncached extends ExternalFunctionWrapperInvokeNode {
922+
private static final ExternalFunctionWrapperInvokeNodeUncached INSTANCE = new ExternalFunctionWrapperInvokeNodeUncached();
923+
924+
@Override
925+
public Object execute(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments) {
926+
CompilerDirectives.transferToInterpreterAndInvalidate();
927+
PythonContext ctx = PythonContext.get(null);
928+
return invoke(frame, ctx, timing, name, callable, cArguments, null, getUncachedCheckResultNode(provider), provider.getUncachedConvertRetNode(), PForeignToPTypeNode.getUncached(),
929+
GetThreadStateNode.getUncached(), ExternalFunctionInvokeNodeGen.getUncached());
930+
}
931+
}
888932

889933
@NeverDefault
890-
public static ExternalFunctionInvokeNode create() {
891-
return ExternalFunctionNodesFactory.ExternalFunctionInvokeNodeGen.create();
934+
public static ExternalFunctionWrapperInvokeNode create() {
935+
return ExternalFunctionNodesFactory.ExternalFunctionWrapperInvokeNodeGen.create();
892936
}
893937

894-
public static ExternalFunctionInvokeNode getUncached() {
895-
return ExternalFunctionNodesFactory.ExternalFunctionInvokeNodeGen.getUncached();
938+
public static ExternalFunctionWrapperInvokeNode getUncached() {
939+
return ExternalFunctionWrapperInvokeNodeUncached.INSTANCE;
896940
}
897941
}
898942

@@ -901,7 +945,7 @@ abstract static class MethodDescriptorRoot extends PRootNode {
901945
private final CApiTiming timing;
902946
@Child private CalleeContext calleeContext = CalleeContext.create();
903947
@Child private CallVarargsMethodNode invokeNode;
904-
@Child private ExternalFunctionInvokeNode externalInvokeNode;
948+
@Child private ExternalFunctionWrapperInvokeNode externalInvokeNode;
905949
@Child private ReadIndexedArgumentNode readSelfNode;
906950
@Child private ReadIndexedArgumentNode readCallableNode;
907951
@Child private ReleaseNativeWrapperNode releaseNativeWrapperNode;
@@ -920,7 +964,7 @@ abstract static class MethodDescriptorRoot extends PRootNode {
920964
this.timing = CApiTiming.create(true, name);
921965
this.provider = provider;
922966
if (provider != null) {
923-
this.externalInvokeNode = ExternalFunctionInvokeNode.create();
967+
this.externalInvokeNode = ExternalFunctionWrapperInvokeNode.create();
924968
this.convertArgs = provider.createConvertArgNodes();
925969
} else {
926970
this.invokeNode = CallVarargsMethodNode.create();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PySequenceDelItemNode.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
4848
import com.oracle.graal.python.builtins.objects.cext.capi.CApiContext;
4949
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes;
50+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.ExternalFunctionWrapperInvokeNode;
5051
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
5152
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
5253
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
@@ -110,7 +111,7 @@ static Object doGenericManaged(VirtualFrame frame, Object object, int index,
110111
static Object doNative(VirtualFrame frame, PythonAbstractNativeObject object, int index,
111112
@Bind("this") Node inliningTarget,
112113
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
113-
@Cached ExternalFunctionNodes.ExternalFunctionInvokeNode invokeNode) {
114+
@Cached ExternalFunctionWrapperInvokeNode invokeNode) {
114115
Object executable = CApiContext.getNativeSymbol(inliningTarget, SYMBOL);
115116
return invokeNode.execute(frame, DELITEM, C_API_TIMING, SYMBOL.getTsName(), executable, new Object[]{toNativeNode.execute(object), index});
116117
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PySequenceGetItemNode.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
4848
import com.oracle.graal.python.builtins.objects.cext.capi.CApiContext;
4949
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes;
50+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.ExternalFunctionWrapperInvokeNode;
5051
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
5152
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
5253
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
@@ -110,7 +111,7 @@ static Object doGenericManaged(VirtualFrame frame, Object object, int index,
110111
static Object doNative(VirtualFrame frame, PythonAbstractNativeObject object, int index,
111112
@Bind("this") Node inliningTarget,
112113
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
113-
@Cached ExternalFunctionNodes.ExternalFunctionInvokeNode invokeNode) {
114+
@Cached ExternalFunctionWrapperInvokeNode invokeNode) {
114115
Object executable = CApiContext.getNativeSymbol(inliningTarget, SYMBOL);
115116
return invokeNode.execute(frame, GETITEM, C_API_TIMING, SYMBOL.getTsName(), executable, new Object[]{toNativeNode.execute(object), index});
116117
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PySequenceSetItemNode.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
4848
import com.oracle.graal.python.builtins.objects.cext.capi.CApiContext;
4949
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes;
50+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.ExternalFunctionWrapperInvokeNode;
5051
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
5152
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
5253
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
@@ -110,7 +111,7 @@ static Object doGenericManaged(VirtualFrame frame, Object object, int index, Obj
110111
static Object doNative(VirtualFrame frame, PythonAbstractNativeObject object, int index, Object value,
111112
@Bind("this") Node inliningTarget,
112113
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
113-
@Cached ExternalFunctionNodes.ExternalFunctionInvokeNode invokeNode) {
114+
@Cached ExternalFunctionWrapperInvokeNode invokeNode) {
114115
Object executable = CApiContext.getNativeSymbol(inliningTarget, SYMBOL);
115116
return invokeNode.execute(frame, SETITEM, C_API_TIMING, SYMBOL.getTsName(), executable, new Object[]{toNativeNode.execute(object), index, toNativeNode.execute(value)});
116117
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PySequenceSizeNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ static long doGenericManaged(VirtualFrame frame, Object object,
125125
static long doNative(VirtualFrame frame, PythonAbstractNativeObject object,
126126
@Bind("this") Node inliningTarget,
127127
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
128-
@Cached ExternalFunctionNodes.ExternalFunctionInvokeNode invokeNode) {
128+
@Cached ExternalFunctionNodes.ExternalFunctionWrapperInvokeNode invokeNode) {
129129
Object executable = CApiContext.getNativeSymbol(inliningTarget, SYMBOL);
130130
Object size = invokeNode.execute(frame, LENFUNC, C_API_TIMING, SYMBOL.getTsName(), executable, new Object[]{toNativeNode.execute(object)});
131131
assert PGuards.isInteger(size);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@
159159
import com.oracle.graal.python.nodes.statement.AbstractImportNode;
160160
import com.oracle.graal.python.nodes.util.CastToJavaIntLossyNode;
161161
import com.oracle.graal.python.runtime.AsyncHandler.AsyncAction;
162+
import com.oracle.graal.python.runtime.PythonContextFactory.GetThreadStateNodeGen;
162163
import com.oracle.graal.python.runtime.exception.ExceptionUtils;
163164
import com.oracle.graal.python.runtime.exception.PException;
164165
import com.oracle.graal.python.runtime.exception.PythonThreadKillException;
@@ -612,6 +613,10 @@ private static final class AtExitHook {
612613
@GenerateInline(inlineByDefault = true)
613614
public abstract static class GetThreadStateNode extends Node {
614615

616+
public static GetThreadStateNode getUncached() {
617+
return GetThreadStateNodeGen.getUncached();
618+
}
619+
615620
public abstract PythonThreadState execute(Node inliningTarget, PythonContext context);
616621

617622
public final PythonThreadState execute(Node inliningTarget) {

0 commit comments

Comments
 (0)