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

Vladimir Kozlov vladimir.kozlov at oracle.com
Sat Aug 4 02:28:04 UTC 2018


Hi, Liu

Changes in TestOnSpinWait.java are fine.
But you last change in TestMeetIncompatibleInterfaceArrays.java is incorrect. SkippedException should be used with check 
- it does not do the check:
http://hg.openjdk.java.net/jdk/jdk/file/7b1ddbafa134/test/lib/jtreg/SkippedException.java

  You can't simple replace error exception with it.

Please, update Copyright year (2016 -> 2018) in header in TestOnSpinWait.java test.

I tested new changes and only serviceability/sa/ClhsdbFindPC.java failed as I said before.

cd test/hotspot/jtreg
jtreg -testjdk:$JAVA_HOME -va serviceability/sa/ClhsdbFindPC.java

It seems C2 immediately deoptimize it after compilation and debugger does not expect that:

   12984 1305   !b        jdk.test.lib.apps.LingeredApp::main (88 bytes)
   12989 1305   !         jdk.test.lib.apps.LingeredApp::main (88 bytes)   made not entrant

http://openjdk.java.net/jtreg/

Regards,
Vladimir

On 8/3/18 6:15 PM, Liu Xin wrote:
> Hi, Vladimir & Igor,
> 
> I found you decide to throw SkippedException if tieredCompilation doesn’t meet.
> I modified my patch to follow this direction. Could you review it?
> http://cr.openjdk.java.net/~phh/8207965/webrev.03/ <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.03/>
> 
> Besides the inputs from Vladimir, I mainly worked on TestOnSpinWait
> It will run both client and server VMs. It will skip c1-check if user only enable compiler2.
> 
> Tested using c2-only server and clientVM locally(x86-64+macosx).
> 
> Thanks,
> —lx
> 
>> On Aug 1, 2018, at 1:05 PM, Hohensee, Paul <hohensee at amazon.com <mailto:hohensee at amazon.com>> wrote:
>>
>> Webrev also here:http://cr.openjdk.java.net/~phh/8207965/webrev.02/ <http://cr.openjdk.java.net/%7Ephh/8207965/webrev.02/>
>> *From:*hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net 
>> <mailto:hotspot-compiler-dev-bounces at openjdk.java.net>> on behalf of Liu Xin <navy.xliu at gmail.com 
>> <mailto:navy.xliu at gmail.com>>
>> *Date:*Tuesday, July 31, 2018 at 6:31 PM
>> *To:*Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>
>> *Cc:*"hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>" 
>> <hotspot-compiler-dev at openjdk.java.net <mailto: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
>> 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
>>
>>             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/%7Ephh/8207965/webrev.01/>
>>                 <http://cr.openjdk.java.net/~phh/8207965/webrev.01/
>>             <http://cr.openjdk.java.net/%7Ephh/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>>
>>
>>                                      Webrev:http://cr.openjdk.java.net/~phh/8207965/webrev.00/
>>             <http://cr.openjdk.java.net/%7Ephh/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/>>>
>>
>>                                      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>>>.
>>
>>
>>                                      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
>>
> 


More information about the hotspot-compiler-dev mailing list