Second Zero review request

[Tom Rodriguez]

I'm looking through these changes right now but I finally looked into  
getting the arch into the build directory name.  Basically I added an  
extra variable called VARIANTARCH that is used to override BUILDARCH  
in PLATFORM_DIR and modified the zero build rules to use ZERO_LIBARCH  
for this.  This results in linux_i386_zero for example, so it won't  
use the same arch names as regular hotspot builds since they use i486  
for historical reasons but it's a more accurate naming than zero_zero.

I also found that I couldn't build zero by simply cd'ing to make/linux  
and type make jvmgzero (plus all the required ZERO_ defs) as I would  
expect.  The reason is that the code that generates platform_zero  
refers to OUTPUTDIR which doesn't mean anything unless you invoke make  
from make/Makefile.  It has a value but that value isn't the  
necessarily the actual output directory.  The platform makefiles  
always build in the current directory so simply removing $(OUTPUTDIR)  
for that platform_zero rule allows it to work as expected and  
shouldn't change it's behaviour since OUTPUTDIR is always equivalent  
to pwd.  I've attached a patch that does both the VARIANTARCH changes  
and the platform_zero changes.  I was able to build successfully with  
both make/Makefile and make/linux/Makefile producing linux_i386_zero  
as the build directory.  You might consider making the generated  
platform file be platform_zero_$(ZERO_LIBARCH) instead of just the  
bare platform_zero.

Unfortunately the bits crashed when running Queens but that happened  
with or without my changes.  The make command I issued was:

make ZERO_ARCHDEF=IA32 ZERO_BUILD=true ZERO_LIBARCH=i386  
ZERO_ENDIANNESS=little ARCH_DATA_MODEL=32 ZERO_ARCHFLAG=-m32 jvmgzero  
LIBFFI_LIBS=-lffi

This is the crash message.

cd linux_i386_zero/jvmg && ./test_gamma
java full version "1.6.0_14-b08"
# To suppress the following error report, specify this argument
# after -XX: or in .hotspotrc:  SuppressErrorAt=/ 
bytecodeInterpreter.cpp:427
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/never/hotspot/src/share/vm/interpreter/ 
bytecodeInterpreter.cpp:427), pid=9886, tid=1082800448
#  Error: assert(istate->_stack_limit == istate->_thread- 
 >last_Java_sp() + 1,"wrong")
#
# JRE version: 6.0_14-b08
# Java VM: OpenJDK Zero VM (17.0-b01-internal-jvmg interpreted mode  
linux-i386 )
# An error report file with more information is saved as:
# /home/never/hotspot/make/linux/linux_i386_zero/jvmg/hs_err_pid9886.log
#
# If you would like to submit a bug report, please visit:
#   http://java.sun.com/webapps/bugreport/crash.jsp
#
Current thread is 1082800448
Dumping core ...
Aborted
make: *** [jvmgzero] Error 134

I can provide more details if that's useful.

tom

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bp.patch
Type: application/octet-stream
Size: 2816 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/hotspot-dev/attachments/20090930/674ba492/attachment.obj 
-------------- next part --------------


On Sep 24, 2009, at 5:56 AM, Gary Benson wrote:

> Hi all,
>
> Zero is an interpreter-only port of HotSpot that uses no assembler and
> can trivially be built on any Linux system.  The following webrev adds
> Zero support to OpenJDK:
>
>  http://cr.openjdk.java.net/~gbenson/zero-10/
>
> Building is largely the same as the previous webrev (zero-07) except
> for the following changes:
>
>  - The variable CORE_BUILD is no longer used.
>  - The variable ZERO_BITSPERWORD has been replaced with  
> ARCH_DATA_MODEL
>
> Summary of changes
> ==================
>
> - The code has been reformatted to better match HotSpot's coding
>   conventions.  Notably, all opening braces have been moved to the
>   end of the previous line, rather than being on their own lines.
>
> - The variable CORE_BUILD is no longer required.  Setting ZERO_BUILD
>   is all that is required to enable Zero.
>
> - The Zero build process no longer hijacks the core rules in the
>   makefiles, but provides its own rules.  A consequence of this is
>   that the build directory is now linux_zero_zero rather than
>   linux_zero_core.  Another consequence is that Zero now has its
>   own includeDB file, into which all direct #includes have been
>   moved.
>
> - The variable ZERO_BITSPERWORD has been replaced with the existing
>   ARCH_DATA_MODEL.
>
> - The type definition ZeroEntry::method_entry_t has been renamed.
>   Tom Rodriguez suggested MethodEntryFunc, but Zero now supports
>   OSR, which requires a different calling convention, so there is
>   now ZeroEntry::NormalEntryFunc and ZeroEntry::OSREntryFunc.
>
> - Zero no longer hijacks C2_BASE_DIR in hotspot/make/Makefile.
>   ZERO_BASE_DIR is used instead.
>
> - A duplicate definition of STATIC_CXX was removed.
>
> - Zero's ICache now overrides some AbstractICache methods with
>   empty methods, avoiding conditionals in icache.cpp.  Comments
>   in icache_zero.hpp explain this rationale.
>
> - JNIHandles::clear is now protected on all platforms, not just Zero.
>
> - Code from the ZeroStackPrinter class has been migrated to methods
>   in ZeroFrame and its subclasses.  stackPrinter_zero.hpp (previously
>   directly included from vmError.cpp) has been removed, and most of
>   the Zero-specific code in VMError::report has likewise been
>   replaced.  There is still a small amount of conditional code in
>   VMError::report, but this could be removed entirely if it is not
>   acceptable (at the expense of losing the super-detailed stack
>   traces that Zero emits, eg http://tinyurl.com/y9rp4r3).
>
> - frame::frame uses find_blob_unsafe instead of find_blob.
>
> - All stubs contain ShouldNotCallThis() instead of Unimplemented().
>
> Summary of non-changes
> ======================
>
> - The variable ZERO_ENDIANNESS remains unchanged.  I haven't renamed
>   it as ENDIANNESS because it's only used during Zero builds.  It's
>   possible that the makefiles could be refactored such that, for
>   example, existing platforms set ENDIANNESS (leaving Zero requiring
>   it to be set externally) but I wasn't sure I should be doing this
>   in this patch.
>
> - The build directory is linux_zero_zero (or linux_zero_shark) rather
>   than linux_$(ARCH)_zero.  There is no variable defining the name of
>   the build directory as such, it's always referenced as
>   $(OSNAME)_$(BUILDARCH)_whatever, and in addition to specifiying the
>   build directory, BUILDARCH is used in conditionals in various
>   makefiles to add extra compiler flags, etc.  If BUILDARCH is set to
>   "i486", for example, then extra compiler flags are added which are
>   unnecessary (and possibly problematic) for Zero.
>
> - The variable ZERO_LIBARCH remains.  It's used in several different
>   places, and I couldn't figure out a place to put the logic to
>   convert a uname string to the correct ZERO_LIBARCH value where the
>   result would be accessible everywhere it is required.
>
> Build Instructions
> ==================
>
>  There are a number of environment variables that need setting before
>  running make, but if you are using jdk/make/jdk_generic_profile.sh
>  to set up your environment then you only need to set one, and a build
>  can be as simple as:
>
>    export ZERO_BUILD=true
>    . jdk/make/jdk_generic_profile.sh
>    gmake sanity && gmake
>
>  The mandatory environment variable does the following:
>
>    ZERO_BUILD
>      Setting ZERO_BUILD to "true" will cause the Zero interpreter to
>      be used.  If ZERO_BUILD is unset, or set to any other value than
>      "true", the standard, platform-specific interpreter will be used.
>
>  Of the other environment variables the following are required when
>  ZERO_BUILD is set to "true".  These are set by
>  jdk/make/jdk_generic_profile.sh based on the result of uname -m:
>
>    ZERO_LIBARCH
>      This is the name of the architecture-specific subdirectory under
>      $JAVA_HOME/jre/lib.  Typically this will be the same as the  
> output
>      of uname -m, although there are some exceptions: "amd64" instead
>      of "x86_64", for example, and "i386" instead of "i686".
>
>    ZERO_ARCHDEF
>      The value of ZERO_ARCHDEF will be passed to the C++ compiler  
> using
>      -D${ZERO_ARCHDEF} to allow conditionalized platform-specific  
> code.
>      This is typically set to ZERO_LIBARCH converted to uppercase but,
>      again, there are exceptions.  "i386" becomes "IA32", to match  
> what
>      HotSpot already does, and on platforms with both 32- and 64-bit
>      variants ZERO_ARCHDEF corresponds to the 32-bit version, so both
>      ppc and ppc64 have ZERO_ARCHDEF set to "PPC".
>
>    ZERO_ENDIANNESS
>      This is set to "little" or "big".
>
>    ARCH_DATA_MODEL
>      This is set to "32" or "64".
>
>    ZERO_ARCHFLAG
>      This is a flag that will be passed to the C++ compiler and to the
>      linker to instruct them to generate code for the particular
>      platform.  This is required for platforms with both 32- and 64- 
> bit
>      variants where the compiler needs to be told which variant to
>      build for.  ZERO_ARCHFLAG will typically be set to "-m32" or
>      "-m64", except on 31-bit zSeries where it will be set to "-m31".
>
>  Zero uses one external library, libffi, for JNI method invocation.
>  The following two variables are used to tell the compiler and linker
>  how to find libffi.  These can be set by the user, but if left unset
>  then jdk/make/jdk_generic_profile.sh will attempt to set them using
>  pkg-config or, failing that, by supplying defaults pointing to the
>  standard locations:
>
>    LIBFFI_CFLAGS
>      Flags to be passed to the C++ compiler to build against libffi.
>      If unset (and pkg-config is not installed, or if libffi is not
>      built with pkg-config) then this defaults to "".
>
>    LIBFFI_LIBS
>      Flags to be passed to the linker to link against libffi.  If
>      unset (and pkg-config is not installed, or if libffi is not
>      built with pkg-config) then this defaults to "-lffi".
>
> -- 
> http://gbenson.net/