Fix list/bytearray.reverse with native storage

Author: msimacek

graalpython/com.oracle.graal.python.test/src/tests/test_array.py

@@ -36,10 +36,11 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
-import sys
 
 from array import array
 
+from tests.util import storage_to_native
+
 
 def assert_raises(err, fn, *args, **kwargs):
     raised = False
@@ -102,9 +103,7 @@ def test_add_int_to_long_array():
 
 def test_array_native_storage():
     a = array('l', [1, 2, 3])
-    if sys.implementation.name == 'graalpy':
-        assert hasattr(__graalpython__, 'storage_to_native'), "Needs to be run with --python.EnableDebuggingBuiltins"
-        __graalpython__.storage_to_native(a)
+    storage_to_native(a)
     assert a[1] == 2
     a[1] = 3
     assert a == array('l', [1, 3, 3])

graalpython/com.oracle.graal.python.test/src/tests/test_bytes.py

@@ -37,8 +37,11 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-import unittest
 import sys
+import unittest
+
+from tests.util import storage_to_native
+
 
 def assert_raises(err, fn, *args, **kwargs):
     raised = False
@@ -107,6 +110,10 @@ def test_reverse():
     assert not b
     b.reverse()
     assert not b
+    b = bytearray(b'hello')
+    storage_to_native(b)
+    b.reverse()
+    assert b == b'olleh'
 
 
 def test_clear():

graalpython/com.oracle.graal.python.test/src/tests/test_list.py

@@ -3,9 +3,11 @@
 #
 # Licensed under the PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2
 
-import seq_tests
 import sys
 import unittest
+
+from tests.util import storage_to_native
+
 # import pickle
 
 LONG_NUMBER = 6227020800;
@@ -840,6 +842,14 @@ def test_generalize_store(self):
         l += [0x100000000, 'a']
         self.assertEqual([1, 0x100000000, 'a'], l)
 
+    def test_reverse(self):
+        l = [1, 2, 3]
+        self.assertEqual(None, l.reverse())
+        self.assertEqual([3, 2, 1], l)
+        storage_to_native(l)
+        l.reverse()
+        self.assertEqual([1, 2, 3], l)
+
 
 if __name__ == '__main__':
     unittest.main()

graalpython/com.oracle.graal.python.test/src/tests/test_socket.py

@@ -37,10 +37,11 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 import socket
-import sys
 import threading
 import unittest
 
+from tests.util import storage_to_native
+
 
 def test_inet_aton():
     assert socket.inet_aton('127.255.1.2') == bytes([127, 255, 1, 2])
@@ -90,9 +91,7 @@ def server():
         buffer = memoryview(b)[1:]
         sock.recv_into(buffer, 1)
         assert b == b'12a'
-        if sys.implementation.name == 'graalpy':
-            assert hasattr(__graalpython__, 'storage_to_native'), "Needs to be run with --python.EnableDebuggingBuiltins"
-            __graalpython__.storage_to_native(b)
+        storage_to_native(b)
         # Native buffer, no internal array
         buffer = memoryview(buffer)[1:]
         sock.recv_into(buffer, 1)

graalpython/com.oracle.graal.python.test/src/tests/util.py

@@ -0,0 +1,7 @@
+import sys
+
+
+def storage_to_native(s):
+    if sys.implementation.name == 'graalpy':
+        assert hasattr(__graalpython__, 'storage_to_native'), "Needs to be run with --python.EnableDebuggingBuiltins"
+        __graalpython__.storage_to_native(s)

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/bytes/ByteArrayBuiltins.java

@@ -686,8 +686,10 @@ static PByteArray copy(PByteArray byteArray,
     public abstract static class ReverseNode extends PythonBuiltinNode {
 
         @Specialization
-        static PNone reverse(PByteArray byteArray) {
-            byteArray.reverse();
+        static PNone reverse(PByteArray byteArray,
+                        @Bind("this") Node inliningTarget,
+                        @Cached SequenceStorageNodes.ReverseNode reverseNode) {
+            reverseNode.execute(inliningTarget, byteArray.getSequenceStorage());
             return PNone.NONE;
         }
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/bytes/PByteArray.java

@@ -84,10 +84,6 @@ public String formatByteArray(String typeName) {
         }
     }
 
-    public final void reverse() {
-        store.reverse();
-    }
-
     public long getExports() {
         return exports;
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/SequenceStorageNodes.java

@@ -1235,6 +1235,77 @@ protected static boolean isBasicSequenceStorage(Object o) {
         }
     }
 
+    @GenerateUncached
+    @GenerateInline
+    @GenerateCached(false)
+    @ImportStatic(SequenceStorageBaseNode.class)
+    public abstract static class ReverseNode extends Node {
+
+        public abstract void execute(Node inliningTarget, SequenceStorage storage);
+
+        @Specialization
+        static void doEmpty(@SuppressWarnings("unused") EmptySequenceStorage storage) {
+        }
+
+        @Specialization(limit = "MAX_ARRAY_STORAGES", guards = "storage.getClass() == cachedClass")
+        static void doBasic(BasicSequenceStorage storage,
+                        @Cached("storage.getClass()") Class<? extends BasicSequenceStorage> cachedClass) {
+            cachedClass.cast(storage).reverse();
+        }
+
+        @Specialization
+        @InliningCutoff
+        static void doNative(NativeSequenceStorage storage,
+                        @Cached(inline = false) ReverseNativeNode reverseNativeNode) {
+            reverseNativeNode.execute(storage);
+        }
+
+        @GenerateUncached
+        @GenerateInline(false)
+        abstract static class ReverseNativeNode extends Node {
+
+            abstract void execute(NativeSequenceStorage storage);
+
+            @Specialization
+            static void doNativeByte(NativeByteSequenceStorage storage,
+                            @Cached CStructAccess.ReadByteNode readByteNode,
+                            @Cached CStructAccess.WriteByteNode writeByteNode) {
+                int length = storage.length();
+                if (length > 0) {
+                    int head = 0;
+                    int tail = length - 1;
+                    int middle = (length - 1) / 2;
+                    Object ptr = storage.getPtr();
+
+                    for (; head <= middle; head++, tail--) {
+                        byte temp = readByteNode.readArrayElement(ptr, head);
+                        writeByteNode.writeArrayElement(ptr, head, readByteNode.readArrayElement(ptr, tail));
+                        writeByteNode.writeArrayElement(ptr, tail, temp);
+                    }
+                }
+            }
+
+            @Specialization
+            static void doNativeObject(NativeObjectSequenceStorage storage,
+                            @Cached CStructAccess.ReadPointerNode readPointerNode,
+                            @Cached CStructAccess.WritePointerNode writePointerNode) {
+                int length = storage.length();
+                if (length > 0) {
+                    int head = 0;
+                    int tail = length - 1;
+                    int middle = (length - 1) / 2;
+                    Object ptr = storage.getPtr();
+
+                    for (; head <= middle; head++, tail--) {
+                        Object temp = readPointerNode.readArrayElement(ptr, head);
+                        writePointerNode.writeArrayElement(ptr, head, readPointerNode.readArrayElement(ptr, tail));
+                        writePointerNode.writeArrayElement(ptr, tail, temp);
+                    }
+                }
+            }
+        }
+    }
+
     @GenerateUncached
     @ImportStatic(SequenceStorageBaseNode.class)
     @SuppressWarnings("truffle-inlining")       // footprint reduction 92 -> 75

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/ListBuiltins.java

@@ -805,9 +805,11 @@ static PNone clear(PList list) {
     public abstract static class ListReverseNode extends PythonUnaryBuiltinNode {
 
         @Specialization
-        static PList reverse(PList list) {
-            list.reverse();
-            return list;
+        static PNone reverse(PList list,
+                        @Bind("this") Node inliningTarget,
+                        @Cached SequenceStorageNodes.ReverseNode reverseNode) {
+            reverseNode.execute(inliningTarget, list.getSequenceStorage());
+            return PNone.NONE;
         }
 
     }

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/list/PList.java

@@ -85,10 +85,6 @@ public final String toString() {
         return store.toString();
     }
 
-    public final void reverse() {
-        store.reverse();
-    }
-
     @Ignore
     @Override
     public final boolean equals(Object other) {