RFR (small) 6518907: purge IA64 platform-specific ifdefs in source code (original) (raw)

Christian Thalinger christian.thalinger at oracle.com
Wed Nov 28 10:11:43 PST 2012

That's what I expected. I just double checked.

-- Chris

On Nov 28, 2012, at 12:06 AM, "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com> wrote:

Hi Chris,

Yes! And it's used a lot, especially in the generated assembler. Best regards, Goetz.

-----Original Message----- From: Christian Thalinger [mailto:christian.thalinger at oracle.com] Sent: Mittwoch, 28. November 2012 01:13 To: Lindenmaier, Goetz Cc: Morris Meyer; hotspot-dev at openjdk.java.net Subject: Re: RFR (small) 6518907: purge IA64 platform-specific ifdefs in source code Quick question: Do you have a non-empty implementation for CodeBuffer::flushbundle on IA64? -- Chris On Nov 8, 2012, at 12:25 AM, "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com> wrote: Hi Morris,

I had a look at your change. As you know we maintain a port for IA64. We basically use all the IA64 specific code, although some of the defines are obviously superfluous. The changes are not that essential that removing them would cause us huge problems, but nevertheless we would prefer if most of them stay in the code to reduce our deviation from your code. In more detail: Code to remove ============= We do not use the agent on ia64, so we don't care about agent/src/os/linux/LinuxDebuggerLocal.c agent/src/os/linux/libproc.h agent/src/os/win32/windbg/sawindbg.cpp We don't need the code here: src/os/bsd/vm/osbsd.cpp Here we removed the #define ourselves: src/share/vm/interpreter/bytecodeInterpreter.cpp src/share/vm/runtime/sharedRuntime.cpp src/share/vm/runtime/synchronizer.cpp Further you can remove src/share/vm/runtime/vframeArray.cpp We can easily work around needregisterstackbang(), and as it's rather platform dependent, just remove it in src/share/vm/opto/compile.hpp src/share/vm/opto/output.cpp

Code to keep =========== This is basic platform support, so keep it please: src/share/vm/runtime/vmversion.cpp src/share/vm/utilities/macros.hpp Keep this: src/share/vm/prims/forte.cpp Don't care: src/share/vm/runtime/os.cpp We extended the code at this place to cover more cases, see below. Therefore we don't care about the fix in openJDK. Note that our code is also used on AMD64: // Looks like all platforms except IA64 can use the same function to check // if C stack is walkable beyond current frame. The check for fp() is not // necessary on Sparc, but it's harmless. bool os::isfirstCframe(frame* fr) { #if defined(IA64) && !defined(WIN32) // On IA64 we have to check if the callers bsp is still valid // (i.e. within the register stack bounds). // Notice: this only works for threads created by the VM and only if // we walk the current stack!!! If we want to be able to walk // arbitrary other threads, we'll have to somehow store the thread // object in the frame. Thread *thread = Thread::current(); if ((address)fr->fp() <= thread->registerstackbase() HPUXONLY(+ 0x0) LINUXONLY(+ 0x50)) { // This check is a little hacky, because on Linux the frist C // frame's ('startthread') register stack frame starts at // "registerstackbase + 0x48" while on HPUX, the first C frame's _// ('pthreadboundbody') register stack frame seems to really // start at "registerstackbase". return true; } else { return false; } // On Windows AMD64 link() does not work as there's no back link on the stack. #elif (defined(IA64) || defined(AMD64)) && defined(WIN32) return true; #else // Load up sp, fp, sender sp and sender fp, check for reasonable values. // Check usp first, because if that's bad the other accessors may fault // on some architectures. Ditto ufp second, etc. uintptrt fpalignmask = (uintptrt)(sizeof(address)-1); The extension of the frameslots in src/share/vm/opto/output.cpp is only needed for the zapdead*locals stubs. But better keep it. We use most of the code in oslinux.cpp as is, please keep it. src/os/linux/vm/oslinux.cpp There are two patches where you could improve the code: You could use IA64ONLY(* 2) here: @@ -1174,12 +1170,7 @@ // for initial thread if its stack size exceeds 6M. Cap it at 2M, // in case other parts in glibc still assumes 2M max stack size. // FIXME: alt signal stack is gone, maybe we can relax this constraint? -#ifndef IA64 if (stacksize > 2 * K * K) stacksize = 2 * K * K; -#else - // Problem still exists RH7.2 (IA64 anyway) but 2MB is a little small - if (stacksize > 4 * K * K) stacksize = 4 * K * K; -#endif // Try to figure out where the stack base (top) is. This is harder. // You can apply this patch, instead I implemented the two functions empty in the platform file. @@ -4385,16 +4373,12 @@ if (isNPTL()) { return pthreadcondtimedwait(cond, mutex, abstime); } else { -#ifndef IA64 // 6292965: LinuxThreads pthreadcondtimedwait() resets FPU control // word back to default 64bit precision if condvar is signaled. Java // wants 53bit precision. Save and restore current value. int fpu = getfpucontrolword(); -#endif // IA64 int status = pthreadcondtimedwait(cond, mutex, abstime); -#ifndef IA64 setfpucontrolword(fpu); -#endif // IA64 return status; } } We use all of the #defines/#ifdefs here: src/os/windows/vm/oswindows.cpp But we changed the IA64 specific code a lot. It's unclear whether this change is needed: src/share/vm/oops/oop.inline.hpp but to avoid a regression we would like to keep it. Maybe it would be better to protect the change by #if defined(IA64) && defined(WIN32) as it's done at other shared code locations. Best regards, Goetz

-----Original Message----- From: Volker Simonis [mailto:volker.simonis at gmail.com] Sent: Montag, 5. November 2012 11:25 To: Morris Meyer Cc: Lindenmaier, Goetz Subject: Re: RFR (small) 6518907: purge IA64 platform-specific ifdefs in source code Hi Morris, we had a long weekend last week because last Thursday was public holiday in Germany, but my colleague Goetz is currently looking at the change and will come back to you. Thank you and best regards, Volker On Fri, Nov 2, 2012 at 10:42 PM, Morris Meyer <morris.meyer at oracle.com<mailto:morris.meyer at oracle.com>> wrote: Volker, Have you had a chance to review my changes for this bug? Thanks in advance, --morris

More information about the hotspot-dev mailing list