RFR: 8248817: Windows: Improving common cross-platform code
David Holmes
david.holmes at oracle.com
Mon Jul 13 02:43:30 UTC 2020
Hi Ludovic,
On 9/07/2020 11:55 pm, Ludovic Henry wrote:
> Hello,
>
> As part of adding support for Windows-AArch64, I've had the opportunity to read through most of the Windows-x86 code. In doing so, I found some code that I think can be simplified and made easier to read and maintain.
>
> The three areas I have found are:
> - Atomics: Hotspot doesn't make use of existing intrinsics provided by MSVC and Win32, even ones available since Windows XP.
> - Exception handling: there is some code repetition which, even if functional, is subpar.
> - Frames: we can use the existing os::fetch_frame_from_context to simplify the code and reduce frame parsing logic duplication.
>
> I've split the webrevs along the above lines, making each simpler to review. I'm also hosting these webrevs on Bernhard Urban's CR as I currently do not have authorship. I'll also work with him to update the description of the JBS.
Thanks for doing the split!
As a general comment can you please ensure that the Oracle copyright
second year is updated to 2020. Thanks.
Overall these cleanups look good. Thanks for providing them.
> JBS: https://bugs.openjdk.java.net/browse/JDK-8248817
> Webrevs:
> http://cr.openjdk.java.net/~burban/luhenry/8248817-atomics/
Love this cleanup! Great to see all the stubroutines go for x86.
src/hotspot/os_cpu/windows_x86/atomic_windows_x86.hpp
Please delete this entire (archaic) comment block.
42 // The following alternative implementations are needed because
43 // Windows 95 doesn't support (some of) the corresponding Windows NT
44 // calls. Furthermore, these versions allow inlining in the caller.
45 // (More precisely: The documentation for InterlockedExchange says
46 // it is supported for Windows 95. However, when single-stepping
47 // through the assembly code we cannot step into the routine and
48 // when looking at the routine address we see only garbage code.
49 // Better safe then sorry!). Was bug 7/31/98 (gri).
50 //
51 // Performance note: On uniprocessors, the 'lock' prefixes are not
52 // necessary (and expensive). We should generate separate cases if
53 // this becomes a performance problem.
In this (and elsewhere):
80 DEFINE_STUB_ADD(4, long, InterlockedAdd)
81 DEFINE_STUB_ADD(8, __int64, InterlockedAdd64)
can we use __int32 for clarity rather than "long"?
> http://cr.openjdk.java.net/~burban/luhenry/8248817-exception-handling/
Looks good!
> http://cr.openjdk.java.net/~burban/luhenry/8248817-frames/
Looks good!
Thanks,
David
-----
> Tests: jtreg:hotspot:tier, jtreg:jdk:tier1, jtreg:jdk:tier2, jtreg:langtools on Windows-x86 and Windows-x86_64, no regressions.
>
> Thank you,
>
> --
> Ludovic
>
More information about the hotspot-runtime-dev
mailing list