RFR: JDK-8166748 Clean out Windows IA64 support
George Triantafillou
george.triantafillou at oracle.com
Thu Jun 8 12:00:28 UTC 2017
Hi Kim,
Thanks for the review. Comments inline.
On 6/7/2017 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).
Fixed.
>
> ------------------------------------------------------------------------------
> 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.
Fixed.
> ------------------------------------------------------------------------------
> 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.
Fixed. Thanks!
-George
> ------------------------------------------------------------------------------
>
More information about the core-libs-dev
mailing list