RFR: 8282590: C2: assert(addp->is_AddP() && addp->outcnt() > 0) failed: Don't process dead nodes [v4]

[Tobias Hartmann]

On Tue, 8 Mar 2022 09:02:03 GMT, Emanuel Peter <duke at openjdk.java.net> wrote:

>> Problem:
>> Sometimes nodes are generated, and then not properly added to any other node, nor added to the worklist, so that it could be deleted. In some cases, a node can thus survive after IGVN and enter into `ConnectionGraph::compute_escape`, where no dead nodes are expected (except constants).
>> 
>> In `ArrayCopyNode::prepare_array_copy`:
>> In one, I now add the new ConvI2LNode to the worklist if we are aborting.
>> In the other case, I could safely reorder the code, such new ConvI2LNode would only be generated once we know we are not going to abort.
>> 
>> In `SubTypeCheckNode::Ideal`:
>> Replaced `set_req` with `set_req_X`, such that the input-node that is replaced (which is now potentially a dead node) is put on the worklist.
>> 
>> In `ArrayCopyNode::Ideal`:
>> If we decide to `return NULL` we also need to add the potentially dead forward / backward memory nodes to the worklist, they may turn out to be dead.
>> 
>> Added a regression test: the test + flag combination that first reproduced this issue.
>> 
>> All Tests, including this new regression test are passing.
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixing typo

src/hotspot/share/opto/arraycopynode.cpp line 300:

> 298:     if (src_offset->is_top() || dest_offset->is_top()) {
> 299:       // Offset is out of bounds (the ArrayCopyNode will be removed)
> 300:       // Below: make sure that we record the nodes, so that they can be deleted later (if they are dead)

What about this?

src_offset = Compile::conv_I2X_index(phase, src_offset, ary_src->size());
if (src_offset->is_top()) {
  return false;
}
dest_offset = Compile::conv_I2X_index(phase, dest_offset, ary_dest->size());
if (dest_offset->is_top()) {
  if (can_reshape) {
    phase->is_IterGVN()->_worklist.push(src_offset);
  }
  return false;
}

src/hotspot/share/opto/arraycopynode.cpp line 652:

> 650:         phase->is_IterGVN()->_worklist.push(backward_mem);
> 651:       }
> 652:     }

I don't think you need to check if the mem is really dead. Adding it to the worklist does not hurt. Would this work as well?


if (can_reshape) {
  phase->is_IterGVN()->_worklist.push(mem):
}

-------------

PR: https://git.openjdk.java.net/jdk/pull/7728