RFR: 8224974: Implement JEP 352

Thu May 30 16:58:49 UTC 2019

On 5/30/19 9:08 AM, Andrew Dinn wrote:
> HI Vladimir,
> 
> Thank you for reviewing the patch.
> 
> On 29/05/2019 20:49, Vladimir Kozlov wrote:
>> I tried to test these changes and build failed on all systems except
>> Linux because:
>>
>> workspace/open/src/hotspot/share/prims/unsafe.cpp:446:3: error: use of
>> undeclared identifier 'JNU_ThrowRuntimeException'
>>     JNU_ThrowRuntimeException(env, "writeback is not implemented");
>>     ^
> 
> Apologies for that. I forgot to test this via the submit repo after
> cut-and-pasting the checks for OS and CPU support from the map0 native
> method to the Unsafe writeback method.
> 
> I had to make some tweaks to this code anyway in order to spot an issue
> Alan noticed when checking the CSR (the map code was not distinguishing
> the precise cases where IOException and UnsupportedOperationException
> needed to be thrown and would sometimes have replaced the latter with
> the former on Windows/x86_64).

Okay.

> 
> I have uploaded a new webrev which attempts to address the problem.
> 
>    http://cr.openjdk.java.net/~adinn/8224974/webrev.03

I looked only on HotSpot code.

stubGenerator_x86_64.cpp - in generate_data_cache_writeback() next are not used:

+    bool optimized = VM_Version::supports_clflushopt();
+    bool no_evict = VM_Version::supports_clwb();

vm_version_x86.hpp it should check CPUID flag in 32-bit:

+#else
+  static bool supports_clflush() { return true; }

We don't check has_match_rule() in LibraryCallKit any more.
In .ad files you need to add predicate to new insrtructions:

   predicate(VM_Version::supports_data_cache_line_flush());

Also add this check to Matcher::match_rule_supported() which you can use then in C2Compiler::is_intrinsic_supported().
DISABLE_UNSAFE_WRITEBACK_INTRINSIC should be checked much early may be together with os::supports_map_sync() when you 
set _data_cache_line_flush_size.

> 
> The test to see whether writeback is enabled on the current cpu_os
> combination is now performed in Java from methods Unsafe.writebackMemory
> and FileChannelImpl.map, using a call to Unsafe.isWritebackEnabled()
> 
> There are also 'belt and braces' checks in the corresponding native
> implementation methods:
> 
> Unsafe asserts that VM_Version::supports_data_cache_line_writeback()
> returns true. The result of this method indicates whether support is
> available on both CPU and OS. It returns a value computed using a call
> to a new OS-specific method os::supports_map_sync() and, on hardware for
> which that is true (AArch64 and x86_64), a test of the relevant CPU
> status bits.
> 
> FileChannelImpl still relies on conditional compilation to reject calls
> on invalid OS/CPU combinations (the VM_VERSION method is not available
> for it to call). In the branch for !LINUX || !(AArch64 || amd64) it
> throws an InternalError as this path not be reached.
> 
> Unfortunately, this latest webrev still fails when uploaded to the
> submit repo. The problem seems to be specific to Windows/x86_64 builds.
> The branch that failed is JDK-8224974-03. The returned text is appended
> after my signature. Would you be able to provide some details about the
> errors
  t:/workspace/open/src/java.base/windows/native/libnio/ch/FileChannelImpl.c(64): error C2220: warning treated as error 
- no 'object' file generated
  t:/workspace/open/src/java.base/windows/native/libnio/ch/FileChannelImpl.c(64): warning C4029: declared formal 
parameter list different from definition

> 
>> ------------------------------------------------------------
>> Also Graal test should be fixed for new intrinsics (add them to
>> 'toBeInvestigated' for isJDK13orHigher):
>>
>> src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/CheckGraalIntrinsics.java
> That has been fixed in the webrev mentioned above.

Good.

Thanks,
Vladimir

> 
> regards,
> 
> 
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
> 
> 
> ----- 8< -------- 8< -------- 8< -------- 8< -------- 8< -------- 8< ---
> 
> BuildId: 2019-05-30-1509485.adinn.source
> No failed tests
> Tasks Summary
> 
>      UNABLE_TO_RUN: 18
>      NOTHING_TO_RUN: 0
>      KILLED: 0
>      EXECUTED_WITH_FAILURE: 4
>      FAILED: 0
>      HARNESS_ERROR: 0
>      NA: 0
>      PASSED: 55
>      Build
> 
>          1 Unable to run
>              windows-x64-install-windows-x64-build-19 Dependency task
> failed: mach5...1512-2804499-windows-x64-windows-x64-build-12
>          4 Executed with failure
>              windows-x64-windows-x64-build-12 error while building,
> return value: 2
>              windows-x64-debug-windows-x64-build-13 error while building,
> return value: 2
>              windows-x64-open-windows-x64-build-14 error while building,
> return value: 2
>              windows-x64-open-debug-windows-x64-build-15 error while
> building, return value: 2
> 
>      Test
> 
>          17 Unable to run
> 
> tier1-product-open_test_hotspot_jtreg_tier1_common-windows-x64-22
> Dependency task failed:
> mach5...1512-2804499-windows-x64-windows-x64-build-12
> 
> tier1-debug-open_test_hotspot_jtreg_tier1_common-windows-x64-debug-28
> Dependency task failed:
> mach5...804499-windows-x64-debug-windows-x64-build-13
> 
> tier1-debug-open_test_hotspot_jtreg_tier1_compiler_1-windows-x64-debug-31 Dependency
> task failed: mach5...804499-windows-x64-debug-windows-x64-build-13
> 
> tier1-debug-open_test_hotspot_jtreg_tier1_compiler_2-windows-x64-debug-34 Dependency
> task failed: mach5...804499-windows-x64-debug-windows-x64-build-13
> 
> tier1-debug-open_test_hotspot_jtreg_tier1_compiler_3-windows-x64-debug-37 Dependency
> task failed: mach5...804499-windows-x64-debug-windows-x64-build-13
> 
> tier1-debug-open_test_hotspot_jtreg_tier1_compiler_not_xcomp-windows-x64-debug-40
> Dependency task failed:
> mach5...804499-windows-x64-debug-windows-x64-build-13
> 
> tier1-debug-open_test_hotspot_jtreg_tier1_gc_1-windows-x64-debug-43
> Dependency task failed:
> mach5...804499-windows-x64-debug-windows-x64-build-13
> 
> tier1-debug-open_test_hotspot_jtreg_tier1_gc_2-windows-x64-debug-46
> Dependency task failed:
> mach5...804499-windows-x64-debug-windows-x64-build-13
> 
> tier1-product-open_test_hotspot_jtreg_tier1_gc_gcbasher-windows-x64-25
> Dependency task failed:
> mach5...1512-2804499-windows-x64-windows-x64-build-12
> 
> tier1-debug-open_test_hotspot_jtreg_tier1_gc_gcbasher-windows-x64-debug-49
> Dependency task failed:
> mach5...804499-windows-x64-debug-windows-x64-build-13
>              See all 17...
> 