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

[Lindenmaier, Goetz]

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