RFR: JDK-8166748 Clean out Windows IA64 support
George Triantafillou
george.triantafillou at oracle.com
Thu Jun 8 12:01:20 UTC 2017
Hi Coleen,
Thanks for the review. I've fixed the indentation per Kim's email.
-George
On 6/7/2017 7:33 PM, coleen.phillimore at oracle.com wrote:
> 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