Could you name the file ArrayCopy.inc instead? I'm fine with having it in lib/J3ClassLib, like you have done for SetProperties.inc. So I think you should continue doing with with shared files whose names describe what's in it.<div>
<br></div><div>Otherwise, the change looks good.<br><br><div class="gmail_quote">On Tue, Nov 1, 2011 at 6:01 AM, Will Dietz <span dir="ltr"><<a href="mailto:wdietz2@illinois.edu">wdietz2@illinois.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Inlined below.<br>
<br>
Almost entirely a minor refactoring patch, FWIW.<br>
<br>
I'm not particular about where to put the common code (thoughts<br>
welcome), but "Shared.inc" seemed as good as any for the place to put<br>
shared ClassLib code that isn't already in its own neat file.<br>
<br>
Thanks!<br>
<br>
~Will<br>
<br>
>From eb5a72d6de2bfd98e44e4bb0729954b29b4411a6 Mon Sep 17 00:00:00 2001<br>
From: Will Dietz <<a href="mailto:w@wdtz.org">w@wdtz.org</a>><br>
Date: Thu, 27 Oct 2011 17:30:41 -0500<br>
Subject: [PATCH 7/8] Move ArrayCopy to new 'Shared.inc', use in both CP and<br>
 OJ.<br>
<br>
---<br>
 lib/J3/ClassLib/GNUClasspath/ClasspathVMSystem.inc |   72 +-----------------<br>
 lib/J3/ClassLib/GNUClasspath/JavaUpcalls.cpp       |    1 +<br>
 lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp            |    1 +<br>
 lib/J3/ClassLib/OpenJDK/OpenJDK.inc                |   17 ++++-<br>
 lib/J3/ClassLib/Shared.inc                         |   82 ++++++++++++++++++++<br>
 5 files changed, 101 insertions(+), 72 deletions(-)<br>
 create mode 100644 lib/J3/ClassLib/Shared.inc<br>
<br>
diff --git a/lib/J3/ClassLib/GNUClasspath/ClasspathVMSystem.inc<br>
b/lib/J3/ClassLib/GNUClasspath/ClasspathVMSystem.inc<br>
index 187bfa5..05469ed 100644<br>
--- a/lib/J3/ClassLib/GNUClasspath/ClasspathVMSystem.inc<br>
+++ b/lib/J3/ClassLib/GNUClasspath/ClasspathVMSystem.inc<br>
@@ -30,77 +30,7 @@ jint sstart,<br>
 JavaObject* dst,<br>
 jint dstart,<br>
 jint len) {<br>
-<br>
-  JavaObject* cur = 0;<br>
-  llvm_gcroot(src, 0);<br>
-  llvm_gcroot(dst, 0);<br>
-  llvm_gcroot(cur, 0);<br>
-  assert(src->getVirtualTable());<br>
-  assert(dst->getVirtualTable());<br>
-<br>
-  JavaThread* th = JavaThread::get();<br>
-  Jnjvm *vm = th->getJVM();<br>
-<br>
-  if (src == NULL || dst == NULL) {<br>
-    th->throwException(vm->CreateNullPointerException());<br>
-    UNREACHABLE();<br>
-  }<br>
-<br>
-  if (!(JavaObject::getClass(src)->isArray() &&<br>
-        JavaObject::getClass(dst)->isArray())) {<br>
-    th->throwException(vm->CreateArrayStoreException(<br>
-      (JavaVirtualTable*)dst->getVirtualTable()));<br>
-    UNREACHABLE();<br>
-  }<br>
-<br>
-  UserClassArray* ts = (UserClassArray*)JavaObject::getClass(src);<br>
-  UserClassArray* td = (UserClassArray*)JavaObject::getClass(dst);<br>
-  UserCommonClass* dstType = td->baseClass();<br>
-  UserCommonClass* srcType = ts->baseClass();<br>
-<br>
-  sint32 srcSize = JavaArray::getSize(src);<br>
-  sint32 dstSize = JavaArray::getSize(dst);<br>
-<br>
-  if (len > srcSize) {<br>
-    th->throwException(vm->CreateIndexOutOfBoundsException(len));<br>
-  } else if (len > dstSize) {<br>
-    th->throwException(vm->CreateIndexOutOfBoundsException(len));<br>
-  } else if (len + sstart > srcSize) {<br>
-    th->throwException(vm->CreateIndexOutOfBoundsException(len + sstart));<br>
-  } else if (len + dstart > dstSize) {<br>
-    th->throwException(vm->CreateIndexOutOfBoundsException(len + dstart));<br>
-  } else if (dstart < 0) {<br>
-    th->throwException(vm->CreateIndexOutOfBoundsException(dstart));<br>
-  } else if (sstart < 0) {<br>
-    th->throwException(vm->CreateIndexOutOfBoundsException(sstart));<br>
-  } else if (len < 0) {<br>
-    th->throwException(vm->CreateIndexOutOfBoundsException(len));<br>
-  } else if ((dstType->isPrimitive() || srcType->isPrimitive()) &&<br>
-             srcType != dstType) {<br>
-    th->throwException(vm->CreateArrayStoreException(<br>
-      (JavaVirtualTable*)dst->getVirtualTable()));<br>
-  }<br>
-<br>
-  if (!(dstType->isPrimitive())) {<br>
-    for (int i = 0; i < len; i++) {<br>
-      cur = ArrayObject::getElement((ArrayObject*)src, i + sstart);<br>
-      if (cur) {<br>
-        if (!(JavaObject::getClass(cur)->isAssignableFrom(dstType))) {<br>
-          th->throwException(vm->CreateArrayStoreException(<br>
-              (JavaVirtualTable*)dst->getVirtualTable()));<br>
-          break;<br>
-        } else {<br>
-          ArrayObject::setElement((ArrayObject*)dst, cur, i + dstart);<br>
-        }<br>
-      }<br>
-    }<br>
-  } else {<br>
-    uint32 logSize = dstType->asPrimitiveClass()->logSize;<br>
-    void* ptrDst = (void*)((int64_t)JavaArray::getElements(dst) +<br>
(dstart << logSize));<br>
-    void* ptrSrc = (void*)((int64_t)JavaArray::getElements(src) +<br>
(sstart << logSize));<br>
-    memmove(ptrDst, ptrSrc, len << logSize);<br>
-  }<br>
-<br>
+  Shared_ArrayCopy(src, sstart, dst, dstart, len);<br>
 }<br>
<br>
 JNIEXPORT jint JNICALL Java_java_lang_VMSystem_identityHashCode(<br>
diff --git a/lib/J3/ClassLib/GNUClasspath/JavaUpcalls.cpp<br>
b/lib/J3/ClassLib/GNUClasspath/JavaUpcalls.cpp<br>
index bc797d1..bb47192 100644<br>
--- a/lib/J3/ClassLib/GNUClasspath/JavaUpcalls.cpp<br>
+++ b/lib/J3/ClassLib/GNUClasspath/JavaUpcalls.cpp<br>
@@ -1075,6 +1075,7 @@ void Classpath::InitializeSystem(Jnjvm * jvm) {<br>
 #include "Classpath.inc"<br>
 #include "ClasspathField.inc"<br>
 #include "ClasspathMethod.inc"<br>
+#include "Shared.inc"<br>
 #include "ClasspathVMClass.inc"<br>
 #include "ClasspathVMClassLoader.inc"<br>
 #include "ClasspathVMObject.inc"<br>
diff --git a/lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp<br>
b/lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp<br>
index 43ff227..a9d8c73 100644<br>
--- a/lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp<br>
+++ b/lib/J3/ClassLib/OpenJDK/JavaUpcalls.cpp<br>
@@ -901,4 +901,5 @@ void Classpath::InitializeSystem(Jnjvm * jvm) {<br>
 #include "Classpath.inc"<br>
 #include "ClasspathField.inc"<br>
 #include "ClasspathMethod.inc"<br>
+#include "Shared.inc"<br>
 #include "OpenJDK.inc"<br>
diff --git a/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
b/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
index e19bf96..fc4663c 100644<br>
--- a/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
+++ b/lib/J3/ClassLib/OpenJDK/OpenJDK.inc<br>
@@ -215,7 +215,22 @@ JVM_NanoTime(JNIEnv *env, jclass ignored) {<br>
 JNIEXPORT void JNICALL<br>
 JVM_ArrayCopy(JNIEnv *env, jclass ignored, jobject jsrc, jint sstart,<br>
               jobject jdst, jint dstart, jint len) {<br>
-  NYI();<br>
+  JavaObject *src = 0;<br>
+  JavaObject *dst = 0;<br>
+  JavaObject* cur = 0;<br>
+  llvm_gcroot(src, 0);<br>
+  llvm_gcroot(dst, 0);<br>
+  llvm_gcroot(cur, 0);<br>
+<br>
+  BEGIN_JNI_EXCEPTION<br>
+<br>
+  src = *(JavaObject**)jsrc;<br>
+  dst = *(JavaObject**)jdst;<br>
+<br>
+  Shared_ArrayCopy(src, sstart, dst, dstart, len);<br>
+<br>
+  RETURN_VOID_FROM_JNI<br>
+  END_JNI_EXCEPTION<br>
 }<br>
<br>
 JNIEXPORT jobject JNICALL<br>
diff --git a/lib/J3/ClassLib/Shared.inc b/lib/J3/ClassLib/Shared.inc<br>
new file mode 100644<br>
index 0000000..f4d987e<br>
--- /dev/null<br>
+++ b/lib/J3/ClassLib/Shared.inc<br>
@@ -0,0 +1,82 @@<br>
+//===-- Shared.inc - Code common from ClassLib implementations<br>
------------===//<br>
+//<br>
+//                            The VMKit project<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+<br>
+void Shared_ArrayCopy(JavaObject* src, jint sstart, JavaObject* dst,<br>
jint dstart, jint len)<br>
+{<br>
+  JavaObject* cur = 0;<br>
+  llvm_gcroot(src, 0);<br>
+  llvm_gcroot(dst, 0);<br>
+  llvm_gcroot(cur, 0);<br>
+  assert(src->getVirtualTable());<br>
+  assert(dst->getVirtualTable());<br>
+<br>
+  JavaThread* th = JavaThread::get();<br>
+  Jnjvm *vm = th->getJVM();<br>
+<br>
+  if (src == NULL || dst == NULL) {<br>
+    th->throwException(vm->CreateNullPointerException());<br>
+    UNREACHABLE();<br>
+  }<br>
+<br>
+  if (!(JavaObject::getClass(src)->isArray() &&<br>
+        JavaObject::getClass(dst)->isArray())) {<br>
+    th->throwException(vm->CreateArrayStoreException(<br>
+      (JavaVirtualTable*)dst->getVirtualTable()));<br>
+    UNREACHABLE();<br>
+  }<br>
+<br>
+  UserClassArray* ts = (UserClassArray*)JavaObject::getClass(src);<br>
+  UserClassArray* td = (UserClassArray*)JavaObject::getClass(dst);<br>
+  UserCommonClass* dstType = td->baseClass();<br>
+  UserCommonClass* srcType = ts->baseClass();<br>
+<br>
+  sint32 srcSize = JavaArray::getSize(src);<br>
+  sint32 dstSize = JavaArray::getSize(dst);<br>
+<br>
+  if (len > srcSize) {<br>
+    th->throwException(vm->CreateIndexOutOfBoundsException(len));<br>
+  } else if (len > dstSize) {<br>
+    th->throwException(vm->CreateIndexOutOfBoundsException(len));<br>
+  } else if (len + sstart > srcSize) {<br>
+    th->throwException(vm->CreateIndexOutOfBoundsException(len + sstart));<br>
+  } else if (len + dstart > dstSize) {<br>
+    th->throwException(vm->CreateIndexOutOfBoundsException(len + dstart));<br>
+  } else if (dstart < 0) {<br>
+    th->throwException(vm->CreateIndexOutOfBoundsException(dstart));<br>
+  } else if (sstart < 0) {<br>
+    th->throwException(vm->CreateIndexOutOfBoundsException(sstart));<br>
+  } else if (len < 0) {<br>
+    th->throwException(vm->CreateIndexOutOfBoundsException(len));<br>
+  } else if ((dstType->isPrimitive() || srcType->isPrimitive()) &&<br>
+             srcType != dstType) {<br>
+    th->throwException(vm->CreateArrayStoreException(<br>
+      (JavaVirtualTable*)dst->getVirtualTable()));<br>
+  }<br>
+<br>
+  if (!(dstType->isPrimitive())) {<br>
+    for (int i = 0; i < len; i++) {<br>
+      cur = ArrayObject::getElement((ArrayObject*)src, i + sstart);<br>
+      if (cur) {<br>
+        if (!(JavaObject::getClass(cur)->isAssignableFrom(dstType))) {<br>
+          th->throwException(vm->CreateArrayStoreException(<br>
+              (JavaVirtualTable*)dst->getVirtualTable()));<br>
+          break;<br>
+        } else {<br>
+          ArrayObject::setElement((ArrayObject*)dst, cur, i + dstart);<br>
+        }<br>
+      }<br>
+    }<br>
+  } else {<br>
+    uint32 logSize = dstType->asPrimitiveClass()->logSize;<br>
+    void* ptrDst = (void*)((int64_t)JavaArray::getElements(dst) +<br>
(dstart << logSize));<br>
+    void* ptrSrc = (void*)((int64_t)JavaArray::getElements(src) +<br>
(sstart << logSize));<br>
+    memmove(ptrDst, ptrSrc, len << logSize);<br>
+  }<br>
+}<br>
<font color="#888888">--<br>
1.7.5.1<br>
_______________________________________________<br>
vmkit-commits mailing list<br>
<a href="mailto:vmkit-commits@cs.uiuc.edu">vmkit-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits</a><br>
</font></blockquote></div><br></div>