RFR: JDK-8166748 Clean out Windows IA64 support

Doerr, Martin martin.doerr at sap.com
Thu Jun 8 09:30:20 UTC 2017


Hi George,

thanks for doing that. We don't need this old code any more in jdk10. (We may possibly support hpux on IA64 in our hotspot based JVM, but certainly not Windows).

I think the following should also get removed from the arch_array:
{IMAGE_FILE_MACHINE_IA64,      (char*)"IA 64"}

I agree with Kim's indentation comments.
Besides that, the change looks good to me. I don't need to see a new webrev either.

Thanks and best regards,
Martin


-----Original Message-----
From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of coleen.phillimore at oracle.com
Sent: Donnerstag, 8. Juni 2017 01:33
To: hotspot-runtime-dev at openjdk.java.net
Subject: Re: RFR: JDK-8166748 Clean out Windows IA64 support

George, This looks good to me.  I didn't notice the indentation like Kim 
did but I agree with the last one (to out-dent it).  I don't need to see 
another webrev.
thanks!
Coleen

On 6/7/17 5:32 PM, Kim Barrett wrote:
>> On Jun 7, 2017, at 2:50 PM, George Triantafillou <george.triantafillou at oracle.com> wrote:
>>
>> Please review this fix to clean out Windows IA64 support:
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8166748
>> open webrev (jdk): http://cr.openjdk.java.net/~gtriantafill/8166748-webrev/jdk/webrev/index.html <http://cr.openjdk.java.net/%7Egtriantafill/8166748-webrev/jdk/webrev/index.html>
>> open webrev (hotspot): http://cr.openjdk.java.net/~gtriantafill/8166748-webrev/hotspot/webrev/index.html <http://cr.openjdk.java.net/%7Egtriantafill/8166748-webrev/hotspot/webrev/index.html>
>>
>> Please note that this issue is specifically for removing Windows IA64 support.
>>
>> Built and tested on Windows x64.  Thanks.
>>
>> -George
> Thanks for doing this.
>
> Looks good, except for a couple of minor indentation nits. I don't
> need a new webrev for these.
>
> ------------------------------------------------------------------------------
> src/os/windows/vm/os_windows.cpp
> 3594   if (GetThreadContext(handle, &context)) {
> 3595     #ifdef _M_AMD64
> 3596       return ExtendedPC((address) context.Rip);
> 3597     #else
> 3598       return ExtendedPC((address) context.Eip);
> 3599     #endif
> 3600   } else {
>
> Why was the indentation of these lines changed?  They were fine
> before, and now they look unusual (at least to me).
>
> ------------------------------------------------------------------------------
> src/os/windows/vm/os_windows.cpp
> 2122 LONG Handle_Exception(struct _EXCEPTION_POINTERS* exceptionInfo,
> 2123                       address handler) {
> 2124     JavaThread* thread = (JavaThread*) Thread::current_or_null();
> 2125   // Save pc in thread
>
> Pre-existing: Line 2124 is mis-indented; should only be indented 2
> spaces.
>
> ------------------------------------------------------------------------------
> src/os/windows/vm/os_windows.cpp
> 2126   #ifdef _M_AMD64
> 2127     // Do not blow up if no thread info available.
> 2128     if (thread) {
> 2129       thread->set_saved_exception_pc((address)(DWORD_PTR)exceptionInfo->ContextRecord->Rip);
> 2130     }
> 2131     // Set pc to handler
> 2132     exceptionInfo->ContextRecord->Rip = (DWORD64)handler;
> 2133     #else
> 2134     // Do not blow up if no thread info available.
> 2135     if (thread) {
> 2136       thread->set_saved_exception_pc((address)(DWORD_PTR)exceptionInfo->ContextRecord->Eip);
> 2137     }
> 2138     // Set pc to handler
> 2139     exceptionInfo->ContextRecord->Eip = (DWORD)(DWORD_PTR)handler;
> 2140   #endif
>
> Why was the indentation of these lines changed? They were fine as is,
> except with the removal of the #ifdef _M_IA64 I would have out-dented
> the remaining #ifdef/#else/#endif.  Now this looks unusual (at least to
> me).  The indentation of the #else is particularly problematic.
>
> ------------------------------------------------------------------------------
>



More information about the hotspot-runtime-dev mailing list