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.triantafil...@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
------------------------------------------------------------------------------
