Skip to content

Commit fffa889

Browse files
fangererfangerer
committed
[GR-52811] [GR-52813] Improve PyFloat_AsDouble and PyComplex_AsCComplex.
PullRequest: graalpython/3263
2 parents c64f173 + d276dfa commit fffa889

File tree

8 files changed

+168
-63
lines changed

8 files changed

+168
-63
lines changed

‎graalpython/com.oracle.graal.python.cext/src/capi.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,15 +113,21 @@ typedef struct {
113113
} propertyobject;
114114

115115
typedef struct {
116-
PyObject_VAR_HEAD
116+
PyObject_HEAD
117117
int32_t handle_table_index;
118118
} GraalPyObject;
119119

120120
typedef struct {
121121
GraalPyObject ob_base;
122+
Py_ssize_t ob_size;
122123
PyObject **ob_item;
123124
} GraalPyVarObject;
124125

126+
typedef struct {
127+
GraalPyObject ob_base;
128+
double ob_fval;
129+
} GraalPyFloatObject;
130+
125131
// {{start CAPI_BUILTINS}}
126132
#include "capi.gen.h"
127133

‎graalpython/com.oracle.graal.python.cext/src/complexobject.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -324,18 +324,14 @@ PyComplex_AsCComplex(PyObject *op)
324324
// GraalPy change: different implementation
325325
/* If op is already of type PyComplex_Type, return its value */
326326
if (!points_to_py_handle_space(op) && PyComplex_Check(op)) {
327-
return ((PyComplexObject *)op)->cval;
327+
return ((PyComplexObject*) op)->cval;
328328
}
329-
PyObject* parts = GraalPyTruffleComplex_AsCComplex(op);
330-
Py_complex result;
331-
if(parts != NULL) {
332-
result.real = PyFloat_AsDouble(PyTuple_GetItem(parts, 0));
333-
result.imag = PyFloat_AsDouble(PyTuple_GetItem(parts, 1));
334-
Py_DecRef(parts);
335-
return result;
336-
}
337-
Py_complex c_error = {-1., 0.};
338-
return c_error;
329+
Py_complex result;
330+
if (GraalPyTruffleComplex_AsCComplex(op, &result) == 0) {
331+
return result;
332+
}
333+
Py_complex c_error = { -1., 0. };
334+
return c_error;
339335
}
340336

341337
#if 0 // GraalPy change

‎graalpython/com.oracle.graal.python.cext/src/floatobject.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,22 @@ float_dealloc(PyObject *op)
299299
double
300300
PyFloat_AsDouble(PyObject *op)
301301
{
302-
// GraalPy change: upcall for managed
303-
if (points_to_py_handle_space(op)) {
304-
return GraalPyTruffleFloat_AsDouble(op);
305-
}
302+
// GraalPy change: read from native object stub or upcall for managed
303+
if (points_to_py_handle_space(op)) {
304+
#ifndef GRAALVM_PYTHON_LLVM_MANAGED
305+
if (PyFloat_Check(op)) {
306+
double val = ((GraalPyFloatObject*) pointer_to_stub(op))->ob_fval;
307+
#ifndef NDEBUG
308+
if (PyTruffle_Debug_CAPI() && GraalPyTruffleFloat_AsDouble(op) != val) {
309+
Py_FatalError("ob_size of native stub and managed object differ");
310+
}
311+
#endif
312+
return val;
313+
}
314+
#endif /* GRAALVM_PYTHON_LLVM_MANAGED */
315+
return GraalPyTruffleFloat_AsDouble(op);
316+
}
317+
306318
PyNumberMethods *nb;
307319
PyObject *res;
308320
double val;

‎graalpython/com.oracle.graal.python.cext/src/object.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -867,7 +867,7 @@ Py_ssize_t PyTruffle_SIZE(PyObject *ob) {
867867
* PyVarObject in future.
868868
*/
869869
if (ptr->ob_type == &PyTuple_Type) {
870-
res = ((PyVarObject *) ptr)->ob_size;
870+
res = ((GraalPyVarObject *) ptr)->ob_size;
871871
#ifndef NDEBUG
872872
if (PyTruffle_Debug_CAPI() && GraalPy_get_PyVarObject_ob_size(a) != res)
873873
{

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.TypeError;
4444
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Direct;
4545
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Ignored;
46+
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Int;
47+
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Pointer;
4648
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObject;
4749
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectTransfer;
4850
import static com.oracle.graal.python.nodes.SpecialMethodNames.T___FLOAT__;
@@ -55,8 +57,9 @@
5557
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiUnaryBuiltinNode;
5658
import com.oracle.graal.python.builtins.objects.PNone;
5759
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor;
60+
import com.oracle.graal.python.builtins.objects.cext.structs.CFields;
61+
import com.oracle.graal.python.builtins.objects.cext.structs.CStructAccess;
5862
import com.oracle.graal.python.builtins.objects.complex.PComplex;
59-
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
6063
import com.oracle.graal.python.lib.PyObjectGetAttr;
6164
import com.oracle.graal.python.nodes.PRaiseNode;
6265
import com.oracle.graal.python.nodes.call.CallNode;
@@ -75,20 +78,24 @@
7578

7679
public final class PythonCextComplexBuiltins {
7780

78-
@CApiBuiltin(ret = PyObjectTransfer, args = {PyObject}, call = Ignored)
79-
abstract static class PyTruffleComplex_AsCComplex extends CApiUnaryBuiltinNode {
81+
@CApiBuiltin(ret = Int, args = {PyObject, Pointer}, call = Ignored)
82+
abstract static class PyTruffleComplex_AsCComplex extends CApiBinaryBuiltinNode {
8083
@Specialization
81-
static PTuple asComplex(PComplex c,
82-
@Shared @Cached PythonObjectFactory factory) {
83-
return factory.createTuple(new Object[]{c.getReal(), c.getImag()});
84+
static int asComplex(PComplex c, Object out,
85+
@Shared @Cached CStructAccess.WriteDoubleNode writeDoubleNode) {
86+
writeDoubleNode.write(out, CFields.Py_complex__real, c.getReal());
87+
writeDoubleNode.write(out, CFields.Py_complex__imag, c.getImag());
88+
return 0;
8489
}
8590

8691
@Specialization(guards = "!isPComplex(obj)")
87-
static Object asComplex(Object obj,
92+
static int doGeneric(Object obj, Object out,
8893
@Cached ComplexNode complexNode,
89-
@Shared @Cached PythonObjectFactory factory) {
94+
@Shared @Cached CStructAccess.WriteDoubleNode writeDoubleNode) {
9095
PComplex c = (PComplex) complexNode.execute(null, PythonBuiltinClassType.PComplex, obj, PNone.NO_VALUE);
91-
return factory.createTuple(new Object[]{c.getReal(), c.getImag()});
96+
writeDoubleNode.write(out, CFields.Py_complex__real, c.getReal());
97+
writeDoubleNode.write(out, CFields.Py_complex__imag, c.getImag());
98+
return 0;
9299
}
93100
}
94101

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

Lines changed: 114 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
package com.oracle.graal.python.builtins.objects.cext.capi.transitions;
4242

4343
import static com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper.IMMORTAL_REFCNT;
44+
import static com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper.MANAGED_REFCNT;
4445

4546
import java.lang.ref.ReferenceQueue;
4647
import java.lang.ref.WeakReference;
@@ -82,6 +83,7 @@
8283
import com.oracle.graal.python.builtins.objects.cext.structs.CStructAccess.AllocateNode;
8384
import com.oracle.graal.python.builtins.objects.cext.structs.CStructAccess.FreeNode;
8485
import com.oracle.graal.python.builtins.objects.cext.structs.CStructs;
86+
import com.oracle.graal.python.builtins.objects.floats.PFloat;
8587
import com.oracle.graal.python.builtins.objects.getsetdescriptor.DescriptorDeleteMarker;
8688
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
8789
import com.oracle.graal.python.nodes.PGuards;
@@ -102,6 +104,7 @@
102104
import com.oracle.truffle.api.dsl.Bind;
103105
import com.oracle.truffle.api.dsl.Cached;
104106
import com.oracle.truffle.api.dsl.Cached.Exclusive;
107+
import com.oracle.truffle.api.dsl.Cached.Shared;
105108
import com.oracle.truffle.api.dsl.GenerateCached;
106109
import com.oracle.truffle.api.dsl.GenerateInline;
107110
import com.oracle.truffle.api.dsl.GenerateUncached;
@@ -390,7 +393,7 @@ public static void pollReferenceQueue() {
390393
* free'd at context finalization.
391394
*/
392395
long stubPointer = HandlePointerConverter.pointerToStub(reference.pointer);
393-
if (subNativeRefCount(stubPointer, PythonAbstractObjectNativeWrapper.MANAGED_REFCNT) == 0) {
396+
if (subNativeRefCount(stubPointer, MANAGED_REFCNT) == 0) {
394397
freeNativeStub(stubPointer);
395398
} else {
396399
/*
@@ -429,7 +432,7 @@ public static void pollReferenceQueue() {
429432
*/
430433
private static void processNativeObjectReference(NativeObjectReference reference, NativeObjectReferenceArrayWrapper referencesToBeFreed) {
431434
LOGGER.fine(() -> PythonUtils.formatJString("releasing %s", reference.toString()));
432-
if (subNativeRefCount(reference.pointer, PythonAbstractObjectNativeWrapper.MANAGED_REFCNT) == 0) {
435+
if (subNativeRefCount(reference.pointer, MANAGED_REFCNT) == 0) {
433436
referencesToBeFreed.add(reference.pointer);
434437
}
435438
}
@@ -752,6 +755,7 @@ public static boolean pointsToPyHandleSpace(long pointer) {
752755
@GenerateUncached
753756
@GenerateInline
754757
@GenerateCached(false)
758+
@ImportStatic(CApiGuards.class)
755759
public abstract static class FirstToNativeNode extends Node {
756760

757761
public static long executeUncached(PythonAbstractObjectNativeWrapper wrapper, boolean immortal) {
@@ -765,55 +769,127 @@ public final long execute(Node inliningTarget, PythonAbstractObjectNativeWrapper
765769
public abstract long execute(Node inliningTarget, PythonAbstractObjectNativeWrapper wrapper, boolean immortal);
766770

767771
@Specialization
768-
static long doGeneric(Node inliningTarget, PythonAbstractObjectNativeWrapper wrapper, boolean immortal,
772+
static long doPrimitiveNativeWrapper(Node inliningTarget, PrimitiveNativeWrapper wrapper, boolean immortal,
773+
@Shared @Cached(inline = false) CStructAccess.WriteDoubleNode writeDoubleNode,
774+
@Shared @Cached InlinedConditionProfile isFloatObjectProfile,
775+
@Shared @Cached AllocateNativeObjectStubNode allocateNativeObjectStubNode) {
776+
boolean isFloat = isFloatObjectProfile.profile(inliningTarget, wrapper.isDouble());
777+
CStructs ctype = isFloat ? CStructs.GraalPyFloatObject : CStructs.GraalPyObject;
778+
Object type;
779+
if (wrapper.isBool()) {
780+
type = PythonBuiltinClassType.Boolean;
781+
} else if (wrapper.isIntLike()) {
782+
type = PythonBuiltinClassType.PInt;
783+
} else if (isFloat) {
784+
type = PythonBuiltinClassType.PFloat;
785+
} else {
786+
throw CompilerDirectives.shouldNotReachHere();
787+
}
788+
long taggedPointer = allocateNativeObjectStubNode.execute(inliningTarget, wrapper, type, ctype, immortal);
789+
790+
// allocate a native stub object (C type: GraalPy*Object)
791+
if (isFloat) {
792+
long realPointer = HandlePointerConverter.pointerToStub(taggedPointer);
793+
writeDoubleNode.write(realPointer, CFields.GraalPyFloatObject__ob_fval, wrapper.getDouble());
794+
}
795+
return taggedPointer;
796+
}
797+
798+
@Specialization(guards = "!isPrimitiveNativeWrapper(wrapper)")
799+
static long doOther(Node inliningTarget, PythonAbstractObjectNativeWrapper wrapper, boolean immortal,
800+
@Cached(inline = false) CStructAccess.WriteLongNode writeLongNode,
801+
@Cached(inline = false) CStructAccess.WritePointerNode writePointerNode,
802+
@Shared @Cached(inline = false) CStructAccess.WriteDoubleNode writeDoubleNode,
803+
@Exclusive @Cached InlinedConditionProfile isVarObjectProfile,
804+
@Shared @Cached InlinedConditionProfile isFloatObjectProfile,
805+
@Cached GetClassNode getClassNode,
806+
@Shared @Cached AllocateNativeObjectStubNode allocateNativeObjectStubNode) {
807+
808+
assert !(wrapper instanceof TruffleObjectNativeWrapper);
809+
assert !(wrapper instanceof PrimitiveNativeWrapper);
810+
811+
Object delegate = wrapper.getDelegate();
812+
Object type = getClassNode.execute(inliningTarget, delegate);
813+
814+
CStructs ctype;
815+
if (isVarObjectProfile.profile(inliningTarget, delegate instanceof PTuple)) {
816+
ctype = CStructs.GraalPyVarObject;
817+
} else if (isFloatObjectProfile.profile(inliningTarget, delegate instanceof Double || delegate instanceof PFloat)) {
818+
ctype = CStructs.GraalPyFloatObject;
819+
} else {
820+
ctype = CStructs.GraalPyObject;
821+
}
822+
823+
long taggedPointer = allocateNativeObjectStubNode.execute(inliningTarget, wrapper, type, ctype, immortal);
824+
825+
// allocate a native stub object (C type: GraalPy*Object)
826+
if (ctype == CStructs.GraalPyVarObject) {
827+
assert delegate instanceof PTuple;
828+
SequenceStorage sequenceStorage = ((PTuple) delegate).getSequenceStorage();
829+
long realPointer = HandlePointerConverter.pointerToStub(taggedPointer);
830+
writeLongNode.write(realPointer, CFields.GraalPyVarObject__ob_size, sequenceStorage.length());
831+
Object obItemPtr = 0L;
832+
if (sequenceStorage instanceof NativeSequenceStorage nativeSequenceStorage) {
833+
obItemPtr = nativeSequenceStorage.getPtr();
834+
}
835+
writePointerNode.write(realPointer, CFields.GraalPyVarObject__ob_item, obItemPtr);
836+
} else if (ctype == CStructs.GraalPyFloatObject) {
837+
assert delegate instanceof Double || delegate instanceof PFloat;
838+
long realPointer = HandlePointerConverter.pointerToStub(taggedPointer);
839+
double fval;
840+
if (delegate instanceof Double d) {
841+
fval = d;
842+
} else {
843+
fval = ((PFloat) delegate).getValue();
844+
}
845+
writeDoubleNode.write(realPointer, CFields.GraalPyFloatObject__ob_fval, fval);
846+
}
847+
848+
return taggedPointer;
849+
}
850+
}
851+
852+
@GenerateUncached
853+
@GenerateInline
854+
@GenerateCached(false)
855+
abstract static class AllocateNativeObjectStubNode extends Node {
856+
857+
abstract long execute(Node inliningTarget, PythonAbstractObjectNativeWrapper wrapper, Object type, CStructs ctype, boolean immortal);
858+
859+
@Specialization
860+
static long doGeneric(Node inliningTarget, PythonAbstractObjectNativeWrapper wrapper, Object type, CStructs ctype, boolean immortal,
769861
@Cached(inline = false) GilNode gil,
770862
@Cached(inline = false) CStructAccess.AllocateNode allocateNode,
771863
@Cached(inline = false) CStructAccess.WriteLongNode writeLongNode,
772-
@Cached(inline = false) CStructAccess.WritePointerNode writePointerNode,
773864
@Cached(inline = false) CStructAccess.WriteObjectNewRefNode writeObjectNode,
774865
@Cached(inline = false) CStructAccess.WriteIntNode writeIntNode,
775-
@Cached InlinedConditionProfile isVarObjectProfile,
776-
@Cached InlinedExactClassProfile wrapperProfile,
777-
@Cached GetClassNode getClassNode,
778866
@Cached CoerceNativePointerToLongNode coerceToLongNode) {
779867

868+
log(wrapper);
869+
pollReferenceQueue();
870+
871+
long initialRefCount = immortal ? IMMORTAL_REFCNT : MANAGED_REFCNT;
872+
873+
// allocate a native stub object (C type: GraalPy*Object)
874+
Object nativeObjectStub = allocateNode.alloc(ctype);
875+
876+
HandleContext handleContext = PythonContext.get(inliningTarget).nativeContext;
877+
long stubPointer = coerceToLongNode.execute(inliningTarget, nativeObjectStub);
878+
long taggedPointer = HandlePointerConverter.stubToPointer(stubPointer);
879+
880+
writeLongNode.write(stubPointer, CFields.PyObject__ob_refcnt, initialRefCount);
881+
writeObjectNode.write(stubPointer, CFields.PyObject__ob_type, type);
882+
883+
// TODO(fa): this should not require the GIL (GR-51314)
780884
boolean acquired = gil.acquire();
781885
try {
782-
log(wrapper);
783-
assert !(wrapper instanceof TruffleObjectNativeWrapper);
784-
pollReferenceQueue();
785-
786-
long initialRefCount = immortal ? IMMORTAL_REFCNT : PythonAbstractObjectNativeWrapper.MANAGED_REFCNT;
787-
788-
Object delegate = NativeToPythonNode.handleWrapper(inliningTarget, wrapperProfile, false, wrapper);
789-
Object type = getClassNode.execute(inliningTarget, delegate);
790-
791-
// allocate a native stub object (C type: PyObject)
792-
boolean isTuple = isVarObjectProfile.profile(inliningTarget, delegate instanceof PTuple);
793-
Object nativeObjectStub = allocateNode.alloc(isTuple ? CStructs.GraalPyVarObject : CStructs.GraalPyObject);
794-
writeLongNode.write(nativeObjectStub, CFields.PyObject__ob_refcnt, initialRefCount);
795-
writeObjectNode.write(nativeObjectStub, CFields.PyObject__ob_type, type);
796-
if (isTuple) {
797-
SequenceStorage sequenceStorage = ((PTuple) delegate).getSequenceStorage();
798-
writeLongNode.write(nativeObjectStub, CFields.PyVarObject__ob_size, sequenceStorage.length());
799-
Object obItemPtr = 0L;
800-
if (sequenceStorage instanceof NativeSequenceStorage nativeSequenceStorage) {
801-
obItemPtr = nativeSequenceStorage.getPtr();
802-
}
803-
writePointerNode.write(nativeObjectStub, CFields.GraalPyVarObject__ob_item, obItemPtr);
804-
}
805-
HandleContext handleContext = PythonContext.get(inliningTarget).nativeContext;
806-
long stubPointer = coerceToLongNode.execute(inliningTarget, nativeObjectStub);
807-
long taggedPointer = HandlePointerConverter.stubToPointer(stubPointer);
808886
int idx = nativeStubLookupReserve(handleContext);
809887
// We don't allow 'handleTableIndex == 0' to avoid that zeroed memory
810888
// accidentally maps to some valid object.
811889
assert idx > 0;
812890
writeIntNode.write(stubPointer, CFields.GraalPyObject__handle_table_index, idx);
813891
PythonObjectReference ref = PythonObjectReference.create(handleContext, wrapper, immortal, taggedPointer, idx);
814892
nativeStubLookupPut(handleContext, ref);
815-
816-
return logResult(taggedPointer);
817893
} catch (OverflowException e) {
818894
/*
819895
* The OverflowException may be thrown by 'nativeStubLookupReserve' and indicates
@@ -825,6 +901,7 @@ static long doGeneric(Node inliningTarget, PythonAbstractObjectNativeWrapper wra
825901
} finally {
826902
gil.release(acquired);
827903
}
904+
return logResult(taggedPointer);
828905
}
829906
}
830907

@@ -1283,7 +1360,7 @@ static Object handleWrapper(Node node, InlinedExactClassProfile wrapperProfile,
12831360
* *MUST* have done an incref and so the refcount must be greater than
12841361
* MANAGED_REFCNT.
12851362
*/
1286-
assert objectNativeWrapper.getRefCount() > PythonAbstractObjectNativeWrapper.MANAGED_REFCNT;
1363+
assert objectNativeWrapper.getRefCount() > MANAGED_REFCNT;
12871364
objectNativeWrapper.decRef();
12881365
}
12891366
if (profiledWrapper instanceof PrimitiveNativeWrapper primitive) {
@@ -1480,7 +1557,7 @@ private static Object createAbstractNativeObject(HandleContext handleContext, Ob
14801557
NativeObjectReference ref = new NativeObjectReference(handleContext, result, pointer);
14811558
nativeLookupPut(getContext(), pointer, ref);
14821559

1483-
long refCntDelta = PythonAbstractObjectNativeWrapper.MANAGED_REFCNT - (transfer ? 1 : 0);
1560+
long refCntDelta = MANAGED_REFCNT - (transfer ? 1 : 0);
14841561
addNativeRefCount(pointer, refCntDelta);
14851562
return result;
14861563
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/structs/CFields.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,9 @@ public enum CFields {
116116
PyVarObject__ob_size(Py_ssize_t),
117117

118118
GraalPyObject__handle_table_index(Int),
119+
GraalPyVarObject__ob_size(Py_ssize_t),
119120
GraalPyVarObject__ob_item(PyObjectPtr),
121+
GraalPyFloatObject__ob_fval(Double),
120122

121123
PyModuleDef__m_name(ConstCharPtr),
122124
PyModuleDef__m_doc(ConstCharPtr),
@@ -302,6 +304,9 @@ public enum CFields {
302304

303305
PyModuleDef_Base__m_index(Py_ssize_t),
304306

307+
Py_complex__real(Double),
308+
Py_complex__imag(Double),
309+
305310
PyComplexObject__cval__real(Double),
306311
PyComplexObject__cval__imag(Double),
307312

0 commit comments

Comments
 (0)