RFR (XS): 8208480: Fix for test failure: assert(is_bound() || is_unused()) after JDK-8206075 in C1

Vladimir Kozlov vladimir.kozlov at oracle.com
Mon Aug 20 19:52:05 UTC 2018


CCing to compiler group since changes are in JIT compiler.

This seems fine to me. I start testing it. Will let you know results.

Thanks,
Vladimir

On 8/20/18 7:22 AM, Haug, Gunter wrote:
> Hi all,
> 
> Here is the updated webrev:
> 
> http://cr.openjdk.java.net/~ghaug/webrevs/8208480.v1/
> 
> I have removed the redundant reset of the label.
> 
> Best regards,
> Gunter
> 
> On 20.08.18, 10:00, "Haug, Gunter" <gunter.haug at sap.com> wrote:
> 
>      Hi all,
>      
>      the bail-out path originates from  Compilation::emit_code_epilog not from LIR_Assembler. The design using the CHECK_BAILOUT() macro is a bit unfortunate in this case. I found it the easiest solution to reset the label in the destructor.
>      
>      In fact you're right, my change makes the original bail-out support redundant, I'll prepare an updated webrev.
>      
>      Best regards,
>      Gunter
>      
>      
>      
>      From: Liu Xin <navy.xliu at gmail.com>
>      Date: Friday, 17. August 2018 at 21:03
>      To: Lutz Schmidt <lutz.schmidt at sap.com>
>      Cc: "Haug, Gunter" <gunter.haug at sap.com>, "hotspot-runtime-dev at openjdk.java.net" <hotspot-runtime-dev at openjdk.java.net>, "Doerr, Martin" <martin.doerr at sap.com>, "xxinliu at amazon.com" <xxinliu at amazon.com>
>      Subject: Re: RFR (XS): 8208480: Fix for test failure: assert(is_bound() || is_unused()) after JDK-8206075 in C1
>      
>      hi, Lutz and Gunter,
>      
>      Thank you for providing the information.
>      I met the full codecache in C1 in compiler/codegen/TestCharVect2.java
>      In my understanding,  Compilation::emit_code_epilog(LIR_Assembler* assembler) checks bailout for every statement.
>      Do you meet the problem only on s390?
>      
>      I am not object to your patch. if you need to reset  the label in c1_LIRAssember's dtor, I think bailout  change  is redundant.
>      Remove it make code clearer.
>      
>      diff -r fbb62267e5e9 src/hotspot/share/c1/c1_LIRAssembler.cpp
>      --- a/src/hotspot/share/c1/c1_LIRAssembler.cpp
>      Thu Aug 09 15:52:23 2018 -0700
>      +++ b/src/hotspot/share/c1/c1_LIRAssembler.cpp
>      Fri Aug 17 11:57:21 2018 -0700
>      @@ -112,6 +112,9 @@
>      
>      
>       LIR_Assembler::~LIR_Assembler() {
>      +  // The unwind handler label may be unbound if this destructor is invoked because of a bail-out.
>      +  // Reset it here to avoid an assertion.
>      +  _unwind_handler_entry.reset();
>       }
>      
>      
>      diff -r fbb62267e5e9 src/hotspot/share/c1/c1_LIRAssembler.hpp
>      --- a/src/hotspot/share/c1/c1_LIRAssembler.hpp
>      Thu Aug 09 15:52:23 2018 -0700
>      +++ b/src/hotspot/share/c1/c1_LIRAssembler.hpp
>      Fri Aug 17 11:57:21 2018 -0700
>      @@ -71,11 +71,7 @@
>         void record_non_safepoint_debug_info();
>      
>         // unified bailout support
>      -  void bailout(const char* msg) {
>      -    // reset the label in case it hits assertion in destructor.
>      -    _unwind_handler_entry.reset();
>      -    compilation()->bailout(msg);
>      -  }
>      +  void bailout(const char* msg) const { compilation()->bailout(msg); }
>         bool bailed_out() const                        { return compilation()->bailed_out(); }
>      
>         // code emission patterns and accessors
>      
>      
>      On Fri, Aug 17, 2018 at 8:35 AM Schmidt, Lutz <mailto:lutz.schmidt at sap.com> wrote:
>      Hi,
>       
>      I’m responding on Gunter’s behalf. He just left the office a few minutes ago.
>       
>      The bailout case was a “codecache full” condition. For the error to occur, you must run into that condition when a reference to the label is already generated, but the label is not bound yet. This is not JCK test specific. We just happened to hit “codecache full” there.
>       
>      “jck_simple_api” is a subset of the api suite we compiled for our own purposes. The tests contained therein are just “simple” tests: they do not run too long, they can run massively parallel, they do not depend on specific resources (like ports, addresses, …).
>       
>      I’m afraid this doesn’t help you much. At least I tried. �� If you can’t reproduce on your own, please let us know what tracing you need. We could then try to produce the output on our test systems.
>       
>      Thanks,
>      Lutz
>       
>      From: Liu Xin <mailto:navy.xliu at gmail.com>
>      Date: Friday, 17. August 2018 at 17:15
>      To: "Haug, Gunter" <mailto:gunter.haug at sap.com>
>      Cc: "mailto:hotspot-runtime-dev at openjdk.java.net" <mailto:hotspot-runtime-dev at openjdk.java.net>, Lutz Schmidt <mailto:lutz.schmidt at sap.com>, "Doerr, Martin (mailto:martin.doerr at sap.com)" <mailto:martin.doerr at sap.com>, "mailto:xxinliu at amazon.com" <mailto:xxinliu at amazon.com>
>      Subject: Re: RFR (XS): 8208480: Fix for test failure: assert(is_bound() || is_unused()) after JDK-8206075 in C1
>       
>      hi, Gunter,
>      I do consider bailout case. why didn't it catch your case?
>      c1_LIRAssember.hpp
>       // unified bailout support
>        void bailout(const char* msg) {
>          // reset the label in case it hits assertion in destructor.
>          _unwind_handler_entry.reset();
>          compilation()->bailout(msg);
>        }
>       
>      can I reproduce this jck_simple_api_work?
>      /priv/jvmtests/output_sapjvm12_o_jdk-test_dbgU_linuxs390x/jck_simple_api_work/hs_err_pid108809.log
>       
>      On Fri, Aug 17, 2018 at 6:39 AM Haug, Gunter <mailto:gunter.haug at sap.com> wrote:
>      Hi all,
>      
>      could I please have  a review and a sponsor for this tiny fix:
>      
>      https://bugs.openjdk.java.net/browse/JDK-8208480
>      http://cr.openjdk.java.net/~ghaug/webrevs/8208480
>      
>      C1 holds a label to the unwind handler during compilation. There are bail-out paths where a branch to this label has already been emitted but the handler hasn't (e.g. code cache full). The label is therefore unbound when the destructor is invoked and the assertion fires.
>      
>      Thanks,
>      Gunter
>      
>      
> 


More information about the hotspot-runtime-dev mailing list