Re: Patch to build 64 bit VM - Greg Lewis - net.java.openjdk.bsd-port-dev

22 messages in net.java.openjdk.bsd-port-devRe: Patch to build 64 bit VM

From	Sent On	Attachments
Xiaobin Lu	Dec 17, 2008 9:13 pm
Xiaobin Lu	Dec 17, 2008 9:22 pm
Greg Lewis	Dec 22, 2008 6:09 am
Xiaobin Lu	Dec 22, 2008 11:05 am	.Other
Landon Fuller	Dec 22, 2008 11:25 am
Greg Lewis	Dec 25, 2008 11:23 pm
Xiaobin Lu	Dec 25, 2008 11:34 pm
Greg Lewis	Dec 26, 2008 9:26 am
Greg Lewis	Dec 28, 2008 12:28 am
Greg Lewis	Dec 28, 2008 12:54 pm	.diff
Xiaobin Lu	Dec 28, 2008 1:35 pm
Greg Lewis	Dec 29, 2008 8:45 am
Xiaobin Lu	Dec 31, 2008 8:32 pm
Michael Franz	Jan 2, 2009 7:58 pm
Kelly O'Hair	Jan 3, 2009 10:59 am
Greg Lewis	Jan 4, 2009 9:18 pm
Michael Franz	Jan 5, 2009 4:42 am
Xiaobin Lu	Jan 5, 2009 11:14 am
Michael Franz	Jan 5, 2009 4:43 pm
Greg Lewis	Jan 5, 2009 10:11 pm
Dalibor Topic	Jan 6, 2009 4:45 am
Michael Franz	Jan 6, 2009 7:36 am

Subject:	Re: Patch to build 64 bit VM
From:	Greg Lewis (gle...@eyesbeyond.com)
Date:	Dec 29, 2008 8:45:48 am
List:	net.java.openjdk.bsd-port-dev

G'day Xiaobin,

On Sun, Dec 28, 2008 at 01:35:49PM -0800, Xiaobin Lu wrote:

I tested your patch and it worked fine on 32 & 64 bit build. However, we still need the patch for src/os_cpu/bsd_x86/vm/bytes_bsd_x86.inline.hpp. The patch looks like the following:

diff -r de4c58dbee8f src/os_cpu/bsd_x86/vm/bytes_bsd_x86.inline.hpp --- a/src/os_cpu/bsd_x86/vm/bytes_bsd_x86.inline.hpp Wed Nov 26 05:05:13 2008 -0800 +++ b/src/os_cpu/bsd_x86/vm/bytes_bsd_x86.inline.hpp Wed Dec 17 21:06:49 2008 -0800 @@ -32,9 +32,9 @@

#if defined(AMD64) # if defined(__APPLE__) -# define bswap16(x) OSSwapInt16(x) -# define bswap32(x) OSSwapInt32(x) -# define bswap64(x) OSSwapInt64(x) +# define bswap_16(x) OSSwapInt16(x) +# define bswap_32(x) OSSwapInt32(x) +# define bswap_64(x) OSSwapInt64(x) # elif defined(__OpenBSD__) # define bswap_16(x) swap16(x) # define bswap_32(x) swap32(x)

You may want to check in the above as well. By the way, I've already integrated my fix to solve the intptr_t and int32_t into hotspot-rt repository (http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot). It will probably take another week for the fix to show up in the hotspot main repository.

Yep, we do need that too. Can you please check it in? I'll check my change in too and we should have a working 64 bit build. It sounds like the 32 bit build will be fixed once your other change bubbles up into the main tree and we do a merge.

BTW, how are you passing in -m64 to the build on Mac OS X?

Greg Lewis wrote:

On Sun, Dec 28, 2008 at 12:29:26AM -0800, Greg Lewis wrote:

On Fri, Dec 26, 2008 at 09:27:36AM -0800, Greg Lewis wrote:

On Thu, Dec 25, 2008 at 11:34:58PM -0800, Xiaobin Lu wrote:

So with my openjdk id, I can commit the changes without extra permission?

I think I should have looked harder at the change first, sorry. I tried compiling with that change in and it broke the i386 build :(. I'm looking at things side by side with the current Linux code to try and get the braces and #ifdef's right, but I have to say this part of the code is becoming quite confusing with the proliferation of preprocessor conditionals.

I hope to have something later today that you can test on Mac OS X.

Instead I shovelled snow/ice for a couple of days so we could get out of our house to the main roads :).

Anyway, attached is a patch. Its not yet compile tested, I've got the compile started however and will check in the morning. Please give it a try on Mac OS X. Basically it looks like the special handling for 64 bit Mac OS X was half merged into the x86 handling.

Oops, as Kurt pointed out I forgot to attach the patch... Trying again.

Greg Lewis wrote:

G'day Xiaobin,

On Mon, Dec 22, 2008 at 11:06:30AM -0800, Xiaobin Lu wrote:

I am attaching the patch to make the VM build pass on Mac OS 64 bit. Here is some explanation on the change:

bytes_bsd_x86.inline.cpp:

Change bswap16(x) to bswap_16(x) and the similar, I believe there was a typo for this. When some ported this to Apple platform, they missed a underscore.

os_bsd_x86.cpp

There was a "}" missing for 64 bit build.

Thats a lot clearer when I use diff -b :).

Other changes to make the build pass will be covered by my fix to 6787106 which I am going to put back to open JDK 7 tree soon.

Please help me review the patch and feel free to let me know if there was anything wrong.

Looks good to me -- I say go ahead and commit it.

Greg Lewis wrote:

G'day Xiaobin,

On Wed, Dec 17, 2008 at 09:14:16PM -0800, Xiaobin Lu wrote:

I am attaching the patch for building 64 bit VM. They are BSD specific files. Would someone review it and help me check it in?

Sure. Can you comment a little on the changes to src/os_cpu/bsd_x86/vm/bytes_bsd_x86.inline.hpp? It seems like the diff is bigger than the changes if that makes sense (i.e. there are some whitespace changes due to control flow changes which are hiding those).

	Start a set with this search
	Include this search in one of my sets
	Exclude this search from one of my sets