RFR(XS): fix c2-only compilation and some tests

Liu Xin navy.xliu at gmail.com
Wed Aug 8 00:16:53 UTC 2018


Hi, Vladimir, 

Could you review the new version of my patch? 
Patch: http://cr.openjdk.java.net/~phh/8207965/webrev.04/ <http://cr.openjdk.java.net/~phh/8207965/webrev.04/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8207965 <https://bugs.openjdk.java.net/browse/JDK-8207965>

I added 2 new require properties.
i) vm.compiler1.enabled
i) vm.compiler2.enabled 

I verified Compiler. isC1Enabled() and ran jtreg tests with the patch.

Thanks,
—lx

> On Aug 1, 2018, at 1:05 PM, Hohensee, Paul <hohensee at amazon.com> wrote:
> 
> Webrev also here: http://cr.openjdk.java.net/~phh/8207965/webrev.02/ <http://cr.openjdk.java.net/~phh/8207965/webrev.02/>
>  
> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> on behalf of Liu Xin <navy.xliu at gmail.com>
> Date: Tuesday, July 31, 2018 at 6:31 PM
> To: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> Cc: "hotspot-compiler-dev at openjdk.java.net" <hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: RFR(XS): fix c2-only compilation and some tests
>  
> Hi, Vladimir, 
>  
> thanks for running it and the feedback. 
>  
> I made a modification according your feedback. Could you take a look?
> https://s3-us-west-2.amazonaws.com/openjdk-webrevs/jdk/c2_only_fix-v3/webrev/index.html <https://s3-us-west-2.amazonaws.com/openjdk-webrevs/jdk/c2_only_fix-v3/webrev/index.html>
>  
> 1. I don't suffer from SA failure. can I pass them all. 
>    TEST                                              TOTAL  PASS  FAIL ERROR
>    jtreg:test/hotspot/jtreg/serviceability/sa           22    22     0     0
>  
>  
> ClhsdbFindPC.java seems not to relate to 'TieredCompilation'
>  
> 2. I don't quite understand the testcase
> test/hotspot/jtreg/vmTestbase/jit/tiered/tieredTest.sh
> 
> 
> The java file is empty. it only triggers a shell without any parameter. 
> Actually, this test is dummy unless you pass in JTREG="VM_OPTIONS=-XX:+TieredCompilation"
> how to run it with different VM_OPTIONS. 
>  
> thanks,
> --lx
>  
> On Mon, Jul 30, 2018 at 10:24 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
> I hit several issues in testing.
> 
> 1. Build failures on SPARC because of missing include:
> 
> make/hotspot/src/native/dtrace/generateJvmOffsets.cpp
> @@ -40,6 +40,7 @@
> 
>  #include <proc_service.h>
>  #include "gc/shared/collectedHeap.hpp"
> +#include "memory/heap.hpp"
>  #include "runtime/vmStructs.hpp"
> 
>  typedef enum GEN_variant {
> 
> 2. Several tests failed because they want -XX:+TieredCompilation. Currently there is no check
> 
> vmTestbase/jit/tiered/TestDescription.java
> 
> test/hotspot/jtreg/vmTestbase/jit/tiered/tieredTest.sh  Mon Jul 30 21:48:51 2018 -0700
> @@ -46,6 +46,11 @@
>      exit 0
>  fi
> 
> +if grep "TieredCompilation not supported in this VM" $log; then
> +    echo "TEST PASSED: Non-tiered Server VM. The test is useless"
> +    exit 0
> +fi
> +
>  if ! egrep '^[0-9.]+: \[compile level=[0-9]' $log; then
>      if [ "${tiered}" == "on" ]; then
>          echo "TEST FAILED: No PrintTieredEvents output"
> 
> compiler/tiered/ConstantGettersTransitionsTest.java (change assert to check) - run with -ea -esa
> 
> compiler/onSpinWait/TestOnSpinWait.java (add skipOnTieredCompilation() check to skip C1 test)
> 
> There is also closed test failed because it uses C1 flag - need to add -XX:+IgnoreUnrecognizedVMOptions (it is work for us in Oracle).
> 
> serviceability/sa/ClhsdbFindPC.java (I don't know how to fix it yet)
> 
> Vladimir
> 
> 
> 
> On 7/30/18 6:19 PM, Vladimir Kozlov wrote:
> I started our tier1-3 testing with --with-jvm-features=-compiler1 build.
> 
> Regards,
> Vladimir
> 
> On 7/30/18 3:03 PM, Liu Xin wrote:
> hi, list,
> 
> We also submitted to the submit repo and ran it successfully.
> 
> I am not sure if it runs in the special configuration. This document doesn't say anything about configurations.
> https://wiki.openjdk.java.net/display/Build/Submit+Repo <https://wiki.openjdk.java.net/display/Build/Submit+Repo>
> 
> thanks,
> --lx
> 
> 
> On Mon, Jul 30, 2018 at 2:41 PM, Liu Xin <navy.xliu at gmail.com <mailto:navy.xliu at gmail.com> <mailto:navy.xliu at gmail.com <mailto:navy.xliu at gmail.com>>> wrote:
> 
>     hi, mail-list,
> 
>     Could you kindly review this patch?
>     Because the patch is from Vladimir,  we might need another reviewer to approve it.
> 
>     Problem: JDK-8207965
>     Webrev: http://cr.openjdk.java.net/~phh/8207965/webrev.01/ <http://cr.openjdk.java.net/~phh/8207965/webrev.01/>
>     <http://cr.openjdk.java.net/~phh/8207965/webrev.01/ <http://cr.openjdk.java.net/~phh/8207965/webrev.01/>>
> 
>     I passed the hotspot-tier1 on x86-64 with the following configuration:
>     --with-debug-level=fastdebug --with-jvm-features=-compiler1,zgc
> 
>     thanks,
>     --lx
> 
> 
>     On Fri, Jul 27, 2018 at 5:27 PM, Liu Xin <navy.xliu at gmail.com <mailto:navy.xliu at gmail.com> <mailto:navy.xliu at gmail.com <mailto:navy.xliu at gmail.com>>> wrote:
> 
>         hi, Vladimir,
>         thank for the patch.  I will verify it locally.
>         I will take a look at ZGC.
> 
>         thanks,
>         --lx
> 
> 
>         On Fri, Jul 27, 2018 at 5:19 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>
>         <mailto:vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>> wrote:
> 
>             Hi Liu
> 
>             I looked on it again and you are right. Method::_aot_code is defined and used only in
>             tiered case.
> 
>             But changes should be next:
>             1. The assert is needed for case when aot code is used.
>             2. AOTCompiledMethod::make_entrant() is used only in Tiered (add #ifdef guard).
>             3. #ifdef DirectivesStack::init() should check INCLUDE_JVMCI
>             4. Missing changes for ZGC.
>             5. In TestMeetIncompatibleInterfaceArrays.java skip test with C1 compilation request
>             when Tiered is off.
> 
>             Thanks,
>             Vladimir
> 
> 
>             diff -r 2ce72467c4e8 src/hotspot/share/aot/aotCodeHeap.cpp
>             --- a/src/hotspot/share/aot/aotCodeHeap.cpp
>             +++ b/src/hotspot/share/aot/aotCodeHeap.cpp
>             @@ -714,7 +714,7 @@
>               void AOTCodeHeap::sweep_method(AOTCompiledMethod *aot) {
>                 int indexes[] = {aot->method_index()};
>                 sweep_dependent_methods(indexes, 1);
>             -  vmassert(aot->method()->code() != aot && aot->method()->aot_code() == NULL, "method
>             still active");
>             +  vmassert(aot->method()->code() != aot TIERED_ONLY( && aot->method()->aot_code() ==
>             NULL), "method still active");
>               }
> 
> 
>             diff -r 2ce72467c4e8 src/hotspot/share/aot/aotCompiledMethod.cpp
>             --- a/src/hotspot/share/aot/aotCompiledMethod.cpp
>             +++ b/src/hotspot/share/aot/aotCompiledMethod.cpp
>             @@ -206,6 +206,7 @@
>                 return true;
>               }
> 
>             +#ifdef TIERED
>               bool AOTCompiledMethod::make_entrant() {
>                 assert(!method()->is_old(), "reviving evolved method!");
>                 assert(*_state_adr != not_entrant, "%s", method()->has_aot_code() ? "has_aot_code()
>             not cleared" : "caller didn't check has_aot_code()");
>             @@ -240,6 +241,7 @@
> 
>                 return true;
>               }
>             +#endif // TIERED
> 
>               // We don't have full dependencies for AOT methods, so flushing is
>               // more conservative than for nmethods.
>             diff -r 2ce72467c4e8 src/hotspot/share/aot/aotCompiledMethod.hpp
>             --- a/src/hotspot/share/aot/aotCompiledMethod.hpp
>             +++ b/src/hotspot/share/aot/aotCompiledMethod.hpp
>             @@ -194,7 +194,7 @@
>                 virtual address verified_entry_point() const { return _code +
>             _meta->verified_entry_offset(); }
>                 virtual void log_identity(xmlStream* stream) const;
>                 virtual void log_state_change() const;
>             -  virtual bool make_entrant();
>             +  virtual bool make_entrant() NOT_TIERED({ ShouldNotReachHere(); return false; });
>                 virtual bool make_not_entrant() { return make_not_entrant_helper(not_entrant); }
>                 virtual bool make_not_used() { return make_not_entrant_helper(not_used); }
>                 virtual address entry_point() const { return _code + _meta->entry_offset(); }
>             diff -r 2ce72467c4e8 src/hotspot/share/compiler/compilerDirectives.cpp
>             --- a/src/hotspot/share/compiler/compilerDirectives.cpp
>             +++ b/src/hotspot/share/compiler/compilerDirectives.cpp
>             @@ -442,7 +442,7 @@
>                 char str[] = "*.*";
>                 const char* error_msg = NULL;
>                 _default_directives->add_match(str, error_msg);
>             -#ifdef COMPILER1
>             +#if defined(COMPILER1) || INCLUDE_JVMCI
>                 _default_directives->_c1_store->EnableOption = true;
>               #endif
>               #ifdef COMPILER2
>             diff -r 2ce72467c4e8 src/hotspot/share/gc/z/zBarrierSet.cpp
>             --- a/src/hotspot/share/gc/z/zBarrierSet.cpp
>             +++ b/src/hotspot/share/gc/z/zBarrierSet.cpp
>             @@ -22,8 +22,12 @@
>                */
> 
>               #include "precompiled.hpp"
>             +#ifdef COMPILER1
>               #include "gc/z/c1/zBarrierSetC1.hpp"
>             +#endif
>             +#ifdef COMPILER2
>               #include "gc/z/c2/zBarrierSetC2.hpp"
>             +#endif
>               #include "gc/z/zBarrierSet.hpp"
>               #include "gc/z/zBarrierSetAssembler.hpp"
>               #include "gc/z/zGlobals.hpp"
>             @@ -33,8 +37,8 @@
> 
>               ZBarrierSet::ZBarrierSet() :
>                   BarrierSet(make_barrier_set_assembler<ZBarrierSetAssembler>(),
>             -               make_barrier_set_c1<ZBarrierSetC1>(),
>             -               make_barrier_set_c2<ZBarrierSetC2>(),
>             +               COMPILER1_PRESENT( make_barrier_set_c1<ZBarrierSetC1>() )
>             NOT_COMPILER1(NULL),
>             +               COMPILER2_PRESENT( make_barrier_set_c2<ZBarrierSetC2>() )
>             NOT_COMPILER2(NULL),
>                              BarrierSet::FakeRtti(BarrierSet::ZBarrierSet)) {}
> 
>               ZBarrierSetAssembler* ZBarrierSet::assembler() {
>             diff -r 2ce72467c4e8
>             test/hotspot/jtreg/compiler/types/TestMeetIncompatibleInterfaceArrays.java
>             --- a/test/hotspot/jtreg/compiler/types/TestMeetIncompatibleInterfaceArrays.java
>             +++ b/test/hotspot/jtreg/compiler/types/TestMeetIncompatibleInterfaceArrays.java
>             @@ -362,6 +362,12 @@
>                                   System.out.println((j + 1) + ". invokation of " + baseClassName +
>             i + "ASM.test() [::" +
>                                                      r.getName() + "() should be '" + tier[pass][j]
>             + "' compiled]");
> 
>             +                    // Skip Profiling compilation (C1) when Tiered is disabled.
>             +                    boolean profile = (level[pass][j] ==
>             CompilerWhiteBoxTest.COMP_LEVEL_FULL_PROFILE);
>             +                    if (profile && CompilerWhiteBoxTest.skipOnTieredCompilation(false)) {
>             +                        continue;
>             +                    }
>             +
>                                   WB.enqueueMethodForCompilation(r, level[pass][j]);
> 
>                                   try {
> 
>             On 7/26/18 1:11 PM, Liu Xin wrote:
> 
>                 hi, Vladimir,
> 
>                 Thank you for replying.
> 
>                 If I disable compiler1, I will run into compiler errors when I build fastdebug. The
>                 attachment is the output.
>                 Even if ignore the assertion, we still have problem to pass hs-tier1. it will crash
>                 when it try to use jvmci.
> 
>                 Here is my configuration:
>                 The existing configuration has been successfully updated in
>                 /Users/xxinliu/Devel/openjdk/jdk/build/macosx-x86_64-normal-server-fastdebug
>                 using configure arguments '--enable-option-checking=fatal
>                 --with-debug-level=fastdebug --with-jtreg=/Users/xxinliu/Devel/jtreg
>                 --with-jvm-features=-compiler1'.
> 
>                 Configuration summary:
>                 * Debug level:    fastdebug
>                 * HS debug level: fastdebug
>                 * JVM variants:   server
>                 * JVM features:   server: 'aot cds cmsgc compiler2 dtrace epsilongc g1gc graal jfr
>                 jni-check jvmci jvmti management nmt parallelgc serialgc services vm-structs'
>                 * OpenJDK target: OS: macosx, CPU architecture: x86, address length: 64
>                 * Version string: 12-internal+0-adhoc.xxinliu.jdk (12-internal)
> 
>                 thanks,
>                 --lx
> 
>                 On Thu, Jul 26, 2018 at 12:48 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>
>                 <mailto:vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> <mailto:vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>
>                 <mailto:vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>>> wrote:
> 
>                      This does not seems right. What failure you got in aotCodeHeap.cpp and
>                 aotCompiledMethod.cpp?
> 
>                      Thanks,
>                      Vladimir
> 
>                      On 7/26/18 10:16 AM, Liu Xin wrote:
> 
>                          Hi, hotspot community,
> 
> 
>                          Please review the small change to getc2-onlybuild to work.
> 
>                          Bug:https://bugs.openjdk.java.net/browse/JDK-8207965 <https://bugs.openjdk.java.net/browse/JDK-8207965>
>                 <https://bugs.openjdk.java.net/browse/JDK-8207965 <https://bugs.openjdk.java.net/browse/JDK-8207965>>
>                 <https://bugs.openjdk.java.net/browse/JDK-8207965 <https://bugs.openjdk.java.net/browse/JDK-8207965>
>                 <https://bugs.openjdk.java.net/browse/JDK-8207965 <https://bugs.openjdk.java.net/browse/JDK-8207965>>>
> 
>                          Webrev: http://cr.openjdk.java.net/~phh/8207965/webrev.00/ <http://cr.openjdk.java.net/~phh/8207965/webrev.00/>
>                 <http://cr.openjdk.java.net/~phh/8207965/webrev.00/ <http://cr.openjdk.java.net/~phh/8207965/webrev.00/>>
>                          <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/ <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/>
>                 <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/ <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/>>>
>                 <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/ <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/>
>                 <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/ <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/>>
> 
>                          <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/ <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/>
>                 <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/ <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.00/>>>>
> 
>                          It will meet 2 compiler errors if you configure project with “./configure
>                 --with-jvm-features=-compiler1
>                          --enable-option-checking=fatal --with-debug-level=fastdebug”.
> 
>                          the configure disable c1. Furthermore,  12 failures in hs-tier1 if we
>                 havec2-only.
> 
> 
>                          C2-onlybuilt met the following errors when I ran hs-tier1:
> 
>                          compiler/aot/cli/jaotc/CompileClassWithDebugTest.java: check that jaotc can
>                 compile a class with a --debug flag
>                          compiler/jvmci/compilerToVM/IsCompilableTest.java:
>                          compiler/jvmci/events/JvmciNotifyBootstrapFinishedEventTest.java:
>                          compiler/jvmci/events/JvmciNotifyInstallEventTest.java:
>                          compiler/jvmci/jdk.vm.ci <http://jdk.vm.ci/>.code.test/src/jdk/vm/ci/code/test/DataPatchTest.java:
>                 compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/MaxOopMapStackOffsetTest.java:
>                          compiler/jvmci/jdk.vm.ci <http://jdk.vm.ci/>.code.test/src/jdk/vm/ci/code/test/NativeCallTest.java:
>                 compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/SimpleCodeInstallationTest.java:
>                 compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/SimpleDebugInfoTest.java:
>                 compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/VirtualObjectDebugInfoTest.java:
>                          compiler/types/TestMeetIncompatibleInterfaceArrays.java: C2 can not handle
>                 returns with incompatible interface
>                          arrays
> 
>                          Most due to bug similar withJDK-8145331
>                 <https://bugs.openjdk.java.net/browse/JDK-8145331 <https://bugs.openjdk.java.net/browse/JDK-8145331>
>                 <https://bugs.openjdk.java.net/browse/JDK-8145331 <https://bugs.openjdk.java.net/browse/JDK-8145331>>
>                          <https://bugs.openjdk.java.net/browse/JDK-8145331 <https://bugs.openjdk.java.net/browse/JDK-8145331>
>                 <https://bugs.openjdk.java.net/browse/JDK-8145331 <https://bugs.openjdk.java.net/browse/JDK-8145331>>>>.
> 
> 
>                          CompilerDirectives::get_for(AbstractCompiler *comp)  returns _/c1/_store
> 
>                             if jvmci() enabled, but hotspot won’t enable it if COMPILER1 is
>                 disabled. As a result, hotspot messes up
>                          refcounts of _c1_store. This webrev fixes 11 out of 12 cases.
> 
> 
>                          The last test (compiler/types/TestMeetIncompatibleInterfaceArrays.java)
>                 makes use of WhiteBox to check tier
>                          levels. I can’t fix it easily. Shall I try to fix it in this webrev? If no,
>                 we can file a new bug to fix the
>                          test. Does anyone have any idea how to fix it?
> 
> 
>                          thanks,
> 
>                          --lx
> 
> 
> 
> 
> 
> 
>  

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180807/b85ef572/attachment-0001.html>


More information about the hotspot-compiler-dev mailing list