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

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::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
> 