simplify pointer comparison of a PyLong-wrapped void* by going through Sulong

timfel · timfel · commit 16ff27c2e167 · 2019-01-07T15:32:57.000+01:00
diff --git a/graalpython/com.oracle.graal.python.cext/src/capi.c b/graalpython/com.oracle.graal.python.cext/src/capi.c
@@ -655,6 +655,21 @@ void* wrap_unsupported(void *fun, ...) {
     return NULL;
 }
 
-int truffle_ptr_compare(void* x, void* y) {
-    return x == y;
+int truffle_ptr_compare(void* x, void* y, int op) {
+    switch (op) {
+    case Py_LT:
+        return x < y;
+    case Py_LE:
+        return x <= y;
+    case Py_EQ:
+        return x == y;
+    case Py_NE:
+        return x != y;
+    case Py_GT:
+        return x > y;
+    case Py_GE:
+        return x >= y;
+    default:
+        return -1;
+    }
 }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/TruffleCextBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/TruffleCextBuiltins.java
@@ -415,14 +415,8 @@ PString run(PString str) {
     @Builtin(name = "do_richcompare", fixedNumOfPositionalArgs = 3)
     @GenerateNodeFactory
     abstract static class RichCompareNode extends PythonBuiltinNode {
-        private static final String[] opstrings = new String[]{"<", "<=", "==", "!=", ">", ">="};
-        private static final String[] opnames = new String[]{
-                        SpecialMethodNames.__LT__, SpecialMethodNames.__LE__, SpecialMethodNames.__EQ__, SpecialMethodNames.__NE__, SpecialMethodNames.__GT__, SpecialMethodNames.__GE__};
-        private static final String[] reversals = new String[]{
-                        SpecialMethodNames.__GT__, SpecialMethodNames.__GE__, SpecialMethodNames.__EQ__, SpecialMethodNames.__NE__, SpecialMethodNames.__GT__, SpecialMethodNames.__GE__};
-
         protected static BinaryComparisonNode create(int op) {
-            return BinaryComparisonNode.create(opnames[op], reversals[op], opstrings[op]);
+            return BinaryComparisonNode.create(SpecialMethodNames.COMPARE_OPNAMES[op], SpecialMethodNames.COMPARE_REVERSALS[op], SpecialMethodNames.COMPARE_OPSTRINGS[op]);
         }
 
         @Specialization(guards = "op == 0")
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/CExtNodes.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/CExtNodes.java
@@ -848,19 +848,36 @@ public static SizeofWCharNode create() {
         }
     }
 
-    public static class IsNode extends CExtBaseNode {
+    public static final class PointerCompareNode extends CExtBaseNode {
+        private final int op;
         @CompilationFinal private TruffleObject isFunc = null;
         @Child Node executeNode = Message.EXECUTE.createNode();
 
-        public boolean execute(PythonNativeObject a, PythonNativeObject b) {
+        private PointerCompareNode(int op) {
+            this.op = op;
+        }
+
+        private boolean executeCFunction(Object a, Object b) {
             try {
-                return (int) ForeignAccess.sendExecute(executeNode, getNativeFunction(), a.object, b.object) != 0;
+                return (int) ForeignAccess.sendExecute(executeNode, getNativeFunction(), a, b, op) != 0;
             } catch (UnsupportedTypeException | ArityException | UnsupportedMessageException e) {
                 CompilerDirectives.transferToInterpreter();
                 throw new IllegalStateException(NativeCAPISymbols.FUN_PTR_COMPARE + " didn't work!");
             }
         }
 
+        public boolean execute(PythonNativeObject a, PythonNativeObject b) {
+            return executeCFunction(a.object, b.object);
+        }
+
+        public boolean execute(PythonNativeObject a, long b) {
+            return executeCFunction(a.object, b);
+        }
+
+        public boolean execute(PythonNativeVoidPtr a, long b) {
+            return executeCFunction(a.object, b);
+        }
+
         private TruffleObject getNativeFunction() {
             if (isFunc == null) {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
@@ -869,10 +886,14 @@ private TruffleObject getNativeFunction() {
             return isFunc;
         }
 
-        public static IsNode create() {
-            return new CExtNodes.IsNode();
+        public static PointerCompareNode create(String specialMethodName) {
+            for (int i = 0; i < SpecialMethodNames.COMPARE_OPNAMES.length; i++) {
+                if (SpecialMethodNames.COMPARE_OPNAMES[i].equals(specialMethodName)) {
+                    return new CExtNodes.PointerCompareNode(i);
+                }
+            }
+            throw new RuntimeException("The special method used for Python C API pointer comparison must be a constant literal (i.e., interned) string");
         }
-
     }
 
     public abstract static class AllToJavaNode extends PNodeWithContext {
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/ints/IntBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/ints/IntBuiltins.java
@@ -56,6 +56,7 @@
 import com.oracle.graal.python.builtins.objects.bytes.PByteArray;
 import com.oracle.graal.python.builtins.objects.bytes.PBytes;
 import com.oracle.graal.python.builtins.objects.bytes.PIBytesLike;
+import com.oracle.graal.python.builtins.objects.cext.CExtNodes;
 import com.oracle.graal.python.builtins.objects.cext.CExtNodes.FromNativeSubclassNode;
 import com.oracle.graal.python.builtins.objects.cext.PythonNativeObject;
 import com.oracle.graal.python.builtins.objects.cext.PythonNativeVoidPtr;
@@ -90,10 +91,6 @@
 import com.oracle.truffle.api.dsl.NodeFactory;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.dsl.TypeSystemReference;
-import com.oracle.truffle.api.interop.ForeignAccess;
-import com.oracle.truffle.api.interop.Message;
-import com.oracle.truffle.api.interop.UnsupportedMessageException;
-import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.profiles.BranchProfile;
 import com.oracle.truffle.api.profiles.ConditionProfile;
 
@@ -1489,39 +1486,10 @@ boolean doDN(PythonNativeObject x, double y,
             return fromNativeNode.execute(x) < y;
         }
 
-        protected static Node createAsPointerNode() {
-            return Message.AS_POINTER.createNode();
-        }
-
-        protected static Node createIsPointerNode() {
-            return Message.IS_POINTER.createNode();
-        }
-
-        protected static Node createIsNullNode() {
-            return Message.IS_NULL.createNode();
-        }
-
         @Specialization
         boolean doVoidPtr(PythonNativeVoidPtr x, long y,
-                        @Cached("createIsPointerNode()") Node isPointerNode,
-                        @Cached("createAsPointerNode()") Node asPointerNode,
-                        @Cached("createIsNullNode()") Node isNullNode) {
-            boolean isPointer = ForeignAccess.sendIsPointer(isPointerNode, x.object);
-            if (isPointer) {
-                try {
-                    return ForeignAccess.sendAsPointer(asPointerNode, x.object) < y;
-                } catch (UnsupportedMessageException e) {
-                    CompilerDirectives.transferToInterpreter();
-                    throw new IllegalStateException("a void ptr should be a ptr");
-                }
-            } else if (ForeignAccess.sendIsNull(isNullNode, x.object)) {
-                return 0 < y;
-            } else if (y <= 0) {
-                return false;
-            } else {
-                CompilerDirectives.transferToInterpreter();
-                throw new IllegalStateException("cannot compare a wrapped void ptr that has no native allocation against arbitrary integers");
-            }
+                        @Cached("create(__LT__)") CExtNodes.PointerCompareNode ltNode) {
+            return ltNode.execute(x, y);
         }
 
         @SuppressWarnings("unused")
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/ObjectBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/ObjectBuiltins.java
@@ -225,7 +225,7 @@ public int hash(Object self) {
     public abstract static class EqNode extends PythonBinaryBuiltinNode {
         @Specialization
         public boolean eq(PythonNativeObject self, PythonNativeObject other,
-                        @Cached("create()") CExtNodes.IsNode nativeIsNode) {
+                        @Cached("create(__EQ__)") CExtNodes.PointerCompareNode nativeIsNode) {
             return nativeIsNode.execute(self, other);
         }
 
@@ -244,8 +244,8 @@ public abstract static class NeNode extends PythonBinaryBuiltinNode {
 
         @Specialization
         public boolean ne(PythonNativeObject self, PythonNativeObject other,
-                        @Cached("create()") CExtNodes.IsNode nativeIsNode) {
-            return !nativeIsNode.execute(self, other);
+                        @Cached("create(__NE__)") CExtNodes.PointerCompareNode nativeNeNode) {
+            return nativeNeNode.execute(self, other);
         }
 
         @Fallback
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/SpecialMethodNames.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/SpecialMethodNames.java
@@ -162,4 +162,10 @@ public abstract class SpecialMethodNames {
 
     public static final String RICHCMP = "__truffle_richcompare__";
     public static final String TRUFFLE_SOURCE = "__truffle_source__";
+
+    // (tfel): The order of these matches the one in CPython, and thus is assumed to remain the same
+    // in various places
+    public static final String[] COMPARE_OPSTRINGS = new String[]{"<", "<=", "==", "!=", ">", ">="};
+    public static final String[] COMPARE_OPNAMES = new String[]{__LT__, __LE__, __EQ__, __NE__, __GT__, __GE__};
+    public static final String[] COMPARE_REVERSALS = new String[]{__GT__, __GE__, __EQ__, __NE__, __GT__, __GE__};
 }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/expression/IsNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/expression/IsNode.java
@@ -165,7 +165,7 @@ boolean doDD(double left, double right) {
 
     @Specialization
     boolean doNative(PythonNativeObject left, PythonNativeObject right,
-                    @Cached("create()") CExtNodes.IsNode isNode) {
+                    @Cached("create(__EQ__)") CExtNodes.PointerCompareNode isNode) {
         return isNode.execute(left, right);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -165,7 +165,7 @@ boolean doDD(double left, double right) {`
`165`	`165`
`166`	`166`	`@Specialization`
`167`	`167`	`boolean doNative(PythonNativeObject left, PythonNativeObject right,`
`168`		`- @Cached("create()") CExtNodes.IsNode isNode) {`
	`168`	`+ @Cached("create(__EQ__)") CExtNodes.PointerCompareNode isNode) {`
`169`	`169`	`return isNode.execute(left, right);`
`170`	`170`	`}`
`171`	`171`