[vmkit-commits] [PATCH 2/3] Unsafe.inc: Rearrange code into related groups of methods.

Nicolas Geoffray nicolas.geoffray at gmail.com
Wed Nov 16 12:13:07 PST 2011


Looks good, with one comment.

On Wed, Nov 16, 2011 at 12:15 AM, Will Dietz <wdietz2 at illinois.edu> wrote:

> Inlined below.
>
> No functionality change other than switching baseToPtr to fieldPtr.
>
> This patch mostly is to make the next patch cleaner.
>
> ~Will
>
> >From 0c7a31e4a5fecae30ddf92cb4d2886e8f5cfae76 Mon Sep 17 00:00:00 2001
> From: Will Dietz <w at wdtz.org>
> Date: Tue, 15 Nov 2011 16:55:11 -0600
> Subject: [PATCH 2/3] Unsafe.inc: Rearrange code into related groups of
>  methods.
>
> Also, rename baseToPtr, with more useful fieldPtr.
> ---
>  lib/J3/ClassLib/Unsafe.inc |  371
> ++++++++++++++++++++++++--------------------
>  1 files changed, 203 insertions(+), 168 deletions(-)
>
> diff --git a/lib/J3/ClassLib/Unsafe.inc b/lib/J3/ClassLib/Unsafe.inc
> index dcccc65..5fe64db 100644
> --- a/lib/J3/ClassLib/Unsafe.inc
> +++ b/lib/J3/ClassLib/Unsafe.inc
> @@ -10,107 +10,175 @@
>  #include "VMStaticInstance.h"
>  #include <stdlib.h>
>
> -// Convert a 'base' JavaObject to its pointer representation.
> -// Handles our special VMStaticInstance wrapper.
> -static inline uint8 *baseToPtr(JavaObject *base) {
> -  if (VMStaticInstance::isVMStaticInstance(base))
> -    return (uint8*)((VMStaticInstance*)base)->getStaticInstance();
> +/// fieldPtr - Compute the address of the field specified by the given
> +/// base/offset pair.  Non-trivial due to our handling of static
> instances,
> +/// and this also handles null-checking as required.
> +///
> +static inline uint8 *fieldPtr(JavaObject *base, long long offset,
>

Please use int64_t instead of long long.


> +  bool throwOnNull = true) {
> +
> +  // For most uses, a 'null' base should throw an exception.
> +  if (throwOnNull) verifyNull(base);
> +
> +  if (base && VMStaticInstance::isVMStaticInstance(base))
> +    return (uint8*)((VMStaticInstance*)base)->getStaticInstance() +
> offset;
>   else
> -    return (uint8*)base;
> +    return (uint8*)base + offset;
>  }
>
>  extern "C" {
>
> -// Never throws.
> -JNIEXPORT bool JNICALL Java_sun_misc_Unsafe_compareAndSwapLong(
> -#ifdef NATIVE_JNI
> -JNIEnv *env,
> -#endif
> -JavaObject* unsafe, JavaObject* obj, jlong offset, jlong expect,
> jlong update) {
> +//===--- Base/Offset methods
> ----------------------------------------------===//
>
> +/// staticFieldOffset - Return the offset of a particular static field
> +/// Only valid to be used with the corresponding staticFieldBase
> +///
> +JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_staticFieldOffset(
> +JavaObject* unsafe, JavaObjectField* _field) {
>   llvm_gcroot(unsafe, 0);
> -  llvm_gcroot(obj, 0);
> -  jlong *ptr;
> -  jlong  value;
> +  llvm_gcroot(_field, 0);
>
> -  ptr = (jlong *) (((uint8 *) obj) + offset);
> +  jlong res = 0;
> +  BEGIN_NATIVE_EXCEPTION(0)
>
> -  value = *ptr;
> +  JavaField * field = JavaObjectField::getInternalField(_field);
> +  assert(field);
>
> -  if (value == expect) {
> -    *ptr = update;
> -    return true;
> -  } else {
> -    return false;
> -  }
> +  res = field->ptrOffset;
>
> +  END_NATIVE_EXCEPTION;
> +
> +  return res;
>  }
>
> -// Never throws.
> -JNIEXPORT bool JNICALL Java_sun_misc_Unsafe_compareAndSwapInt(
> +/// staticFieldBase - Return a JavaObject* representing the static
> instance.
> +/// Note that our static instances aren't actually java objects, so we use
> +/// a placeholder object "VMStaticInstance" that also ensures that
> +/// the corresponding class doesn't get GC'd underneath it.
> +///
> +JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_staticFieldBase(
> +JavaObject* unsafe, JavaObjectField* _field) {
> +  JavaObject* res = 0;
> +  llvm_gcroot(unsafe, 0);
> +  llvm_gcroot(_field, 0);
> +  llvm_gcroot(res, 0);
> +  BEGIN_NATIVE_EXCEPTION(0)
> +
> +  JavaField * field = JavaObjectField::getInternalField(_field);
> +  assert(field);
> +  field->classDef->initialiseClass(JavaThread::get()->getJVM());
> +
> +  res = VMStaticInstance::allocate(field->classDef);
> +
> +  END_NATIVE_EXCEPTION;
> +
> +  return res;
> +}
> +
> +/// arrayBaseOffset - Offset from the array object where the actual
> +/// element data begins.
> +///
> +JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_arrayBaseOffset(
> +JavaObject* unsafe, JavaObject* clazz) {
> +  // Array starts at beginning of object
> +  return 0;
> +}
> +
> +/// arrayIndexScale - Indexing scale for the element type in
> +/// the specified array.  For use with arrayBaseOffset,
> +/// NthElementPtr = ArrayObject + BaseOffset + N*IndexScale
> +/// Return '0' if our JVM stores the elements in a way that
> +/// makes this type of access impossible or unsupported.
> +///
> +JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_arrayIndexScale(
> +JavaObject* unsafe, JavaObject* clazz) {
> +  // For now, just return '0', indicating we don't support this indexing.
> +  // TODO: Implement this for the array types we /do/ support this way.
> +  return 0;
> +}
> +
> +
> +
> +/// objectFieldOffset - Pointer offset of the specified field
> +///
> +JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_objectFieldOffset(
>  #ifdef NATIVE_JNI
>  JNIEnv *env,
>  #endif
> -JavaObject* unsafe, JavaObject* obj, jlong offset, jint expect, jint
> update) {
> +JavaObject* Unsafe, JavaObjectField* Field) {
>
> -  llvm_gcroot(unsafe, 0);
> -  llvm_gcroot(obj, 0);
> -  jint *ptr;
> +  llvm_gcroot(Field, 0);
> +  llvm_gcroot(Unsafe, 0);
>
> -  ptr = (jint *) (((uint8 *) obj) + offset);
> +  JavaField* field = JavaObjectField::getInternalField(Field);
> +  return (jlong)field->ptrOffset;
> +}
>
> -  return __sync_bool_compare_and_swap(ptr, expect, update);
> +//===--- Double-register addressing field accessors
> -----------------------===//
> +// See objectFieldOffset, staticFieldOffset, staticFieldBase
> +// Can also be an array, if/when we support
> arrayIndexScale/arrayBaseOffset
> +#define GET_PUT_OFFSET(Type,jtype,shorttype) \
> +JNIEXPORT jtype JNICALL Java_sun_misc_Unsafe_get ## Type ##
> __Ljava_lang_Object_2J( \
> +JavaObject* unsafe, JavaObject* base, jlong offset) { \
> +  jtype res = 0; \
> +  BEGIN_NATIVE_EXCEPTION(0) \
> +  jtype* ptr = (jtype*)fieldPtr(base,offset); \
> +  res = *ptr; \
> +  END_NATIVE_EXCEPTION \
> +  return res; \
> +} \
> + \
> +JNIEXPORT void JNICALL Java_sun_misc_Unsafe_put ## Type ##
> __Ljava_lang_Object_2J ## shorttype( \
> +JavaObject* unsafe, JavaObject* base, jlong offset, jtype val) { \
> +  BEGIN_NATIVE_EXCEPTION(0) \
> +  jtype* ptr = (jtype*)fieldPtr(base, offset); \
> +  *ptr = val; \
> +  END_NATIVE_EXCEPTION \
>  }
>
> +GET_PUT_OFFSET(Boolean,jboolean,Z)
> +GET_PUT_OFFSET(Byte,jbyte,B)
> +GET_PUT_OFFSET(Char,jchar,C)
> +GET_PUT_OFFSET(Short,jshort,S)
> +GET_PUT_OFFSET(Int,jint,I)
> +GET_PUT_OFFSET(Long,jlong,J)
> +GET_PUT_OFFSET(Float,jfloat,F)
> +GET_PUT_OFFSET(Double,jdouble,D)
> +
> +//===--- Get/Put of Objects, due to GC needs special handling
> -------------===//
> +// JavaObject field accessors:
>  // Never throws.
> -JNIEXPORT bool JNICALL Java_sun_misc_Unsafe_compareAndSwapObject(
> -#ifdef NATIVE_JNI
> -JNIEnv *env,
> -#endif
> -JavaObject* unsafe, JavaObject* obj, jlong offset, JavaObject* expect,
> -JavaObject* update) {
> +JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_getObjectVolatile(
> +JavaObject* unsafe, JavaObject* base, jlong offset) {
> +  JavaObject * res = 0;
>   llvm_gcroot(unsafe, 0);
> -  llvm_gcroot(obj, 0);
> -  llvm_gcroot(expect, 0);
> -  llvm_gcroot(update, 0);
> +  llvm_gcroot(base, 0);
> +  llvm_gcroot(res, 0);
>
> -  JavaObject** ptr = (JavaObject**) (((uint8 *) obj) + offset);
> +  BEGIN_NATIVE_EXCEPTION(0)
> +  JavaObject** ptr = (JavaObject**)fieldPtr(base, offset);
> +  res = *ptr;
> +  END_NATIVE_EXCEPTION;
>
> -  return mvm::Collector::objectReferenceTryCASBarrier((gc*)obj,
> (gc**)ptr, (gc*)expect, (gc*)update);
> +  return res;
>  }
>
> +// Volatile JavaObject field accessors:
>  // Never throws.
>  JNIEXPORT void JNICALL Java_sun_misc_Unsafe_putObjectVolatile(
>  #ifdef NATIVE_JNI
>  JNIEnv *env,
>  #endif
> -JavaObject* unsafe, JavaObject* obj, jlong offset, JavaObject* value) {
> +JavaObject* unsafe, JavaObject* base, jlong offset, JavaObject* value) {
>   llvm_gcroot(unsafe, 0);
> -  llvm_gcroot(obj, 0);
> +  llvm_gcroot(base, 0);
>   llvm_gcroot(value, 0);
>
> -  JavaObject** ptr = (JavaObject**) (((uint8 *) obj) + offset);
> -  mvm::Collector::objectReferenceWriteBarrier((gc*)obj, (gc**)ptr,
> (gc*)value);
> -}
> -
> -JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_allocateMemory(
> -JavaObject* unsafe, jlong size) {
> -  // TODO: Invalid size/OOM/etc handling!
> -  jlong res = 0;
> -  BEGIN_NATIVE_EXCEPTION(0)
> -  res = (jlong)malloc(size);
> -  END_NATIVE_EXCEPTION
> -  return res;
> -}
> -
> -JNIEXPORT void JNICALL Java_sun_misc_Unsafe_freeMemory(
> -JavaObject* unsafe, jlong ptr) {
> -  // TODO: Exception handling...
> -  BEGIN_NATIVE_EXCEPTION(0)
> -  free((void*)ptr);
> -  END_NATIVE_EXCEPTION
> +  JavaObject** ptr = (JavaObject**)fieldPtr(base, offset);
> +  mvm::Collector::objectReferenceWriteBarrier((gc*)base, (gc**)ptr,
> (gc*)value);
>  }
>
> +//===--- Misc get/put defined, TODO: generalize!
> --------------------------===//
>  JNIEXPORT void JNICALL Java_sun_misc_Unsafe_putLong__JJ(
>  JavaObject* unsafe, jlong ptr, jlong value) {
>   BEGIN_NATIVE_EXCEPTION(0)
> @@ -128,104 +196,78 @@ JavaObject* unsafe, jlong ptr) {
>   return res;
>  }
>
> -JNIEXPORT void JNICALL Java_sun_misc_Unsafe_ensureClassInitialized(
> -JavaObject* unsafe, JavaObject* clazz) {
> -  llvm_gcroot(unsafe, 0);
> -  llvm_gcroot(clazz, 0);
> -  BEGIN_NATIVE_EXCEPTION(0)
> -
> -  Jnjvm* vm = JavaThread::get()->getJVM();
>
> -  CommonClass * cl = JavaObject::getClass(clazz);
> -  assert(cl && cl->isClass());
> -  cl->asClass()->resolveClass();
> -  cl->asClass()->initialiseClass(vm);
> -
> -  END_NATIVE_EXCEPTION;
> -}
> +//===--- CompareAndSwap field accessors
> -----------------------------------===//
> +// Never throws.
> +JNIEXPORT bool JNICALL Java_sun_misc_Unsafe_compareAndSwapLong(
> +JavaObject* unsafe, JavaObject* obj, jlong offset, jlong expect,
> jlong update) {
>
> -JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_staticFieldOffset(
> -JavaObject* unsafe, JavaObjectField* _field) {
>   llvm_gcroot(unsafe, 0);
> -  llvm_gcroot(_field, 0);
> -
> -  jlong res = 0;
> -  BEGIN_NATIVE_EXCEPTION(0)
> +  llvm_gcroot(obj, 0);
> +  jlong *ptr;
> +  jlong  value;
>
> -  JavaField * field = JavaObjectField::getInternalField(_field);
> -  assert(field);
> +  ptr = (jlong *) (((uint8 *) obj) + offset);
>
> -  res = field->ptrOffset;
> +  value = *ptr;
>
> -  END_NATIVE_EXCEPTION;
> +  if (value == expect) {
> +    *ptr = update;
> +    return true;
> +  } else {
> +    return false;
> +  }
>
> -  return res;
>  }
>
> -JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_staticFieldBase(
> -JavaObject* unsafe, JavaObjectField* _field) {
> -  JavaObject* res = 0;
> -  llvm_gcroot(unsafe, 0);
> -  llvm_gcroot(_field, 0);
> -  llvm_gcroot(res, 0);
> -  BEGIN_NATIVE_EXCEPTION(0)
> -
> -  JavaField * field = JavaObjectField::getInternalField(_field);
> -  assert(field);
> -  field->classDef->initialiseClass(JavaThread::get()->getJVM());
> +// Never throws.
> +JNIEXPORT bool JNICALL Java_sun_misc_Unsafe_compareAndSwapInt(
> +#ifdef NATIVE_JNI
> +JNIEnv *env,
> +#endif
> +JavaObject* unsafe, JavaObject* obj, jlong offset, jint expect, jint
> update) {
>
> -  res = VMStaticInstance::allocate(field->classDef);
> +  llvm_gcroot(unsafe, 0);
> +  llvm_gcroot(obj, 0);
> +  jint *ptr;
>
> -  END_NATIVE_EXCEPTION;
> +  ptr = (jint *) (((uint8 *) obj) + offset);
>
> -  return res;
> +  return __sync_bool_compare_and_swap(ptr, expect, update);
>  }
>
> -/// objectFieldOffset - Pointer offset of the specified field
> -///
> -JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_objectFieldOffset(
> +// Never throws.
> +JNIEXPORT bool JNICALL Java_sun_misc_Unsafe_compareAndSwapObject(
>  #ifdef NATIVE_JNI
>  JNIEnv *env,
>  #endif
> -JavaObject* Unsafe, JavaObjectField* Field) {
> +JavaObject* unsafe, JavaObject* obj, jlong offset, JavaObject* expect,
> +JavaObject* update) {
> +  llvm_gcroot(unsafe, 0);
> +  llvm_gcroot(obj, 0);
> +  llvm_gcroot(expect, 0);
> +  llvm_gcroot(update, 0);
>
> -  llvm_gcroot(Field, 0);
> -  llvm_gcroot(Unsafe, 0);
> +  JavaObject** ptr = (JavaObject**) (((uint8 *) obj) + offset);
>
> -  JavaField* field = JavaObjectField::getInternalField(Field);
> -  return (jlong)field->ptrOffset;
> +  return mvm::Collector::objectReferenceTryCASBarrier((gc*)obj,
> (gc**)ptr, (gc*)expect, (gc*)update);
>  }
>
> -JNIEXPORT JavaObject* JNICALL Java_sun_misc_Unsafe_getObjectVolatile(
> -JavaObject* unsafe, JavaObject* base, jlong offset) {
> -  JavaObject * res = 0;
> +//===--- Class-related functions
> ------------------------------------------===//
> +JNIEXPORT void JNICALL Java_sun_misc_Unsafe_ensureClassInitialized(
> +JavaObject* unsafe, JavaObject* clazz) {
>   llvm_gcroot(unsafe, 0);
> -  llvm_gcroot(base, 0);
> -  llvm_gcroot(res, 0);
> -
> +  llvm_gcroot(clazz, 0);
>   BEGIN_NATIVE_EXCEPTION(0)
> -  JavaObject** ptr = (JavaObject**) (baseToPtr(base) + offset);
> -  res = *ptr;
> -  END_NATIVE_EXCEPTION;
>
> -  return res;
> -}
> +  Jnjvm* vm = JavaThread::get()->getJVM();
>
> -JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_arrayBaseOffset(
> -JavaObject* unsafe, JavaObject* clazz) {
> -  // Array starts at beginning of object
> -  return 0;
> -}
> +  CommonClass * cl = JavaObject::getClass(clazz);
> +  assert(cl && cl->isClass());
> +  cl->asClass()->resolveClass();
> +  cl->asClass()->initialiseClass(vm);
>
> -JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_arrayIndexScale(
> -#ifdef NATIVE_JNI
> -JNIEnv *env,
> -#endif
> -JavaObject* unsafe, JavaObject* clazz) {
> -  // Return '0' if we don't support indexing this way.
> -  // (We might pack fields specially, etc)
> -  // TODO: Implement this for the array types we support this way
> -  return 0;
> +  END_NATIVE_EXCEPTION;
>  }
>
>  JNIEXPORT JavaObject* JNICALL
>
> Java_sun_misc_Unsafe_defineClass__Ljava_lang_String_2_3BIILjava_lang_ClassLoader_2Ljava_security_ProtectionDomain_2(
> @@ -281,6 +323,31 @@ JavaObject* unsafe, JavaObjectClass * clazz) {
>  }
>
>
> +//===--- Memory functions
> -------------------------------------------------===//
> +JNIEXPORT jlong JNICALL Java_sun_misc_Unsafe_allocateMemory(
> +JavaObject* unsafe, jlong size) {
> +  // TODO: Invalid size/OOM/etc handling!
> +  jlong res = 0;
> +  BEGIN_NATIVE_EXCEPTION(0)
> +  res = (jlong)malloc(size);
> +  END_NATIVE_EXCEPTION
> +  return res;
> +}
> +
> +JNIEXPORT void JNICALL Java_sun_misc_Unsafe_freeMemory(
> +JavaObject* unsafe, jlong ptr) {
> +  // TODO: Exception handling...
> +  BEGIN_NATIVE_EXCEPTION(0)
> +  free((void*)ptr);
> +  END_NATIVE_EXCEPTION
> +}
> +
> +
> +//===--- Misc support functions
> -------------------------------------------===//
> +JNIEXPORT void JNICALL Java_sun_misc_Unsafe_registerNatives(JavaObject*) {
> +  // Nothing, we define the Unsafe methods with the expected signatures.
> +}
> +
>  JNIEXPORT void JNICALL Java_sun_misc_Unsafe_throwException(
>  JavaObject* unsafe, JavaObject * obj) {
>   llvm_gcroot(unsafe, 0);
> @@ -289,38 +356,6 @@ JavaObject* unsafe, JavaObject * obj) {
>   JavaThread::get()->throwException(obj);
>  }
>
> -JNIEXPORT void JNICALL Java_sun_misc_Unsafe_registerNatives(JavaObject*) {
> -  // Nothing
> -}
> -
> -// TODO: Add the Volatile variants
> -#define GET_PUT_OFFSET(Type,jtype,shorttype) \
> -JNIEXPORT jtype JNICALL Java_sun_misc_Unsafe_get ## Type ##
> __Ljava_lang_Object_2J( \
> -JavaObject* unsafe, JavaObject* base, jlong offset) { \
> -  jtype res = 0; \
> -  BEGIN_NATIVE_EXCEPTION(0) \
> -  jtype* ptr = (jtype*) (baseToPtr(base) + offset); \
> -  res = *ptr; \
> -  END_NATIVE_EXCEPTION \
> - \
> -  return res; \
> -} \
> - \
> -JNIEXPORT void JNICALL Java_sun_misc_Unsafe_put ## Type ##
> __Ljava_lang_Object_2J ## shorttype( \
> -JavaObject* unsafe, JavaObject* base, jlong offset, jtype val) { \
> -  BEGIN_NATIVE_EXCEPTION(0) \
> -  jtype* ptr = (jtype*) (baseToPtr(base) + offset); \
> -  *ptr = val; \
> -  END_NATIVE_EXCEPTION \
> -}
>
> -GET_PUT_OFFSET(Boolean,jboolean,Z)
> -GET_PUT_OFFSET(Byte,jbyte,B)
> -GET_PUT_OFFSET(Char,jchar,C)
> -GET_PUT_OFFSET(Short,jshort,S)
> -GET_PUT_OFFSET(Int,jint,I)
> -GET_PUT_OFFSET(Long,jlong,J)
> -GET_PUT_OFFSET(Float,jfloat,F)
> -GET_PUT_OFFSET(Double,jdouble,D)
>
>  }
> --
> 1.7.5.1
> _______________________________________________
> vmkit-commits mailing list
> vmkit-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111116/f24ac68e/attachment.html>


More information about the vmkit-commits mailing list