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

[Christian Thalinger]

Quick question:  Do you have a non-empty implementation for CodeBuffer::flush_bundle 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/os_bsd.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 need_register_stack_bang(), 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/vm_version.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::is_first_C_frame(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->register_stack_base() HPUX_ONLY(+ 0x0) LINUX_ONLY(+ 0x50)) {
> 
>    // This check is a little hacky, because on Linux the frist C
> 
>    // frame's ('start_thread') register stack frame starts at
> 
>    // "register_stack_base + 0x48" while on HPUX, the first C frame's
> 
>    // ('__pthread_bound_body') register stack frame seems to really
> 
>    // start at "register_stack_base".
> 
>    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.
> 
>  uintptr_t fp_align_mask = (uintptr_t)(sizeof(address)-1);
> 
> 
> 
> 
> 
> The extension of the frame_slots in
> 
>  src/share/vm/opto/output.cpp
> 
> is only needed for the _zap_dead_*_locals stubs. But better keep it.
> 
> 
> 
> We use most of the code in os_linux.cpp as is, please keep it.
> 
>  src/os/linux/vm/os_linux.cpp
> 
> 
> 
> There are two patches where you could improve the code:
> 
> 
> 
> You could use IA64_ONLY(* 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 (stack_size > 2 * K * K) stack_size = 2 * K * K;
> 
> -#else
> 
> -  // Problem still exists RH7.2 (IA64 anyway) but 2MB is a little small
> 
> -  if (stack_size > 4 * K * K) stack_size = 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 (is_NPTL()) {
> 
>       return pthread_cond_timedwait(_cond, _mutex, _abstime);
> 
>    } else {
> 
> -#ifndef IA64
> 
>       // 6292965: LinuxThreads pthread_cond_timedwait() resets FPU control
> 
>       // word back to default 64bit precision if condvar is signaled. Java
> 
>       // wants 53bit precision.  Save and restore current value.
> 
>       int fpu = get_fpu_control_word();
> 
> -#endif // IA64
> 
>       int status = pthread_cond_timedwait(_cond, _mutex, _abstime);
> 
> -#ifndef IA64
> 
>       set_fpu_control_word(fpu);
> 
> -#endif // IA64
> 
>       return status;
> 
>    }
> 
> }
> 
> 
> 
> We use all of the #defines/#ifdefs here:
> 
>  src/os/windows/vm/os_windows.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