On Wed, 14 Oct 2020 07:52:58 GMT, Roland Westrelin <roland at openjdk.org> wrote:
> compiler/c2/TestDeadDataLoopIGVN.java ran with -server -Xcomp
> -XX:+CreateCoredumpOnCrash -ea -esa -XX:CompileThreshold=100
> -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation and
> the patch from 8223051 (long counted loops, not integrated yet) fails
> with:
>
> assert(no_dead_loop) failed: dead loop detected
>
> I can reproduce the failure with only this change from 8223051:
> diff --git a/src/hotspot/share/opto/callnode.cpp b/src/hotspot/share/opto/callnode.cpp
> index 268cec7732c..295cb4ecaf9 100644
> --- a/src/hotspot/share/opto/callnode.cpp
> +++ b/src/hotspot/share/opto/callnode.cpp
> @@ -1159,7 +1159,7 @@ Node* SafePointNode::Identity(PhaseGVN* phase) {
> if( in(TypeFunc::Control)->is_SafePoint() )
> return in(TypeFunc::Control);
>
> - if( in(0)->is_Proj() ) {
> + if (in(0)->is_Proj() && !phase->C->major_progress()) {
> Node *n0 = in(0)->in(0);
> // Check if he is a call projection (except Leaf Call)
> if( n0->is_Catch() ) {
> diff --git a/src/hotspot/share/opto/parse1.cpp b/src/hotspot/share/opto/parse1.cpp
> index 4b88c379dea..baf2bf9bacc 100644
> --- a/src/hotspot/share/opto/parse1.cpp
> +++ b/src/hotspot/share/opto/parse1.cpp
> @@ -2254,23 +2254,7 @@ void Parse::return_current(Node* value) {
>
> //------------------------------add_safepoint----------------------------------
> void Parse::add_safepoint() {
> - // See if we can avoid this safepoint. No need for a SafePoint immediately
> - // after a Call (except Leaf Call) or another SafePoint.
> - Node *proj = control();
> uint parms = TypeFunc::Parms+1;
> - if( proj->is_Proj() ) {
> - Node *n0 = proj->in(0);
> - if( n0->is_Catch() ) {
> - n0 = n0->in(0)->in(0);
> - assert( n0->is_Call(), "expect a call here" );
> - }
> - if( n0->is_Call() ) {
> - if( n0->as_Call()->guaranteed_safepoint() )
> - return;
> - } else if( n0->is_SafePoint() && n0->req() >= parms ) {
> - return;
> - }
> - }
>
> // Clear out dead values from the debug info.
> kill_dead_locals();
> so it's unrelated to the long counted loops transformation itself.
>
> At IGVN time, a dead loop is optimized. The loop contains the
> following subgraph:
>
> (LoadI .. (AddP (CastPP .. (Phi (Proj (CallStaticJava .. (AddI
>
> The projection is on the backedge of the phi. The AddI points back to
> the LoadI. The CallStaticJava is a boxing method call. The LoadI is a
> boxed value load.
>
> The phi is optimized out as the loop is unreachable:
>
> (LoadI .. (AddP (Proj (CallStaticJava .. (AddI
>
> The LoadI is then optimized by code MemNode::can_see_stored_value():
> // Load boxed value from result of valueOf() call is input parameter.
> if (this->is_Load() && ld_adr->is_AddP() &&
> (tp != NULL) && tp->is_ptr_to_boxed_value()) {
> intptr_t ignore = 0;
> Node* base = AddPNode::Ideal_base_and_offset(ld_adr, phase, ignore);
> BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
> base = bs->step_over_gc_barrier(base);
> if (base != NULL && base->is_Proj() &&
> base->as_Proj()->_con == TypeFunc::Parms &&
> base->in(0)->is_CallStaticJava() &&
> base->in(0)->as_CallStaticJava()->is_boxing_method()) {
> return base->in(0)->in(TypeFunc::Parms);
> }
> }
> to the AddI. Because the AddI has the LoadI as input we end up with a
> dead loop. I propose extending the dead loop safe logic to take the
> pattern with a boxing method load into account.
This pull request has now been integrated.
Changeset: f44fc6de
Author: Roland Westrelin <roland at openjdk.org>
URL: https://git.openjdk.java.net/jdk/commit/f44fc6de
Stats: 28 lines in 2 files changed: 23 ins; 4 del; 1 mod
8254734: "dead loop detected" assert failure with patch from 8223051
Reviewed-by: chagedorn, kvn
-------------
PR: https://git.openjdk.java.net/jdk/pull/649