C2: Understanding LoadNode::find_previous_arraycopy()

[Reingruber, Richard]

Hi,

I'd need a little help please understanding the implementation of the following method.

Specifically, L556 is the only location where mem is changed. NULL will be returned if we reach there.

memnode.cpp:

 531  // Find an arraycopy that must have set (can_see_stored_value=true) or
 532  // could have set (can_see_stored_value=false) the value for this load
 533  Node* LoadNode::find_previous_arraycopy(PhaseTransform* phase, Node* ld_alloc, Node*& mem, bool can_see_stored_value) const {
 534    ArrayCopyNode* ac = find_array_copy_clone(phase, ld_alloc, mem);
 535    if (ac != NULL) {
 536      return ac;
 537    } else if (mem->is_Proj() && mem->in(0) != NULL && mem->in(0)->is_ArrayCopy()) {
 538      ArrayCopyNode* ac = mem->in(0)->as_ArrayCopy();
 539
 540      if (ac->is_arraycopy_validated() ||
 541            ac->is_copyof_validated() ||
 542            ac->is_copyofrange_validated()) {
 543        Node* ld_addp = in(MemNode::Address);
 544        if (ld_addp->is_AddP()) {
 545            Node* ld_base = ld_addp->in(AddPNode::Address);
 546            Node* ld_offs = ld_addp->in(AddPNode::Offset);
 547
 548            Node* dest = ac->in(ArrayCopyNode::Dest);
 549
 550            if (dest == ld_base) {
 551              const TypeX *ld_offs_t = phase->type(ld_offs)->isa_intptr_t();
 552              if (ac->modifies(ld_offs_t->_lo, ld_offs_t->_hi, phase, can_see_stored_value)) {
 553                return ac;
 554              }
 555              if (!can_see_stored_value) {
 556                mem = ac->in(TypeFunc::Memory);
 557              }
 558            }
 559        }
 560      }
 561    }
 562    return NULL;
 563  }


The only call that can reach L556 is in MemNode::find_previous_store() L709:

 709      } else if (find_previous_arraycopy(phase, alloc, mem, false) != NULL) {
 710        if (prev != mem) {
 711            // Found an arraycopy but it doesn't affect that load
 712            continue;
 713        }
 714        // Found an arraycopy that may affect that load
 715        return mem;

Observation: 712 is dead code.

Is this a bug? I'd think so. I'd expect find_previous_arraycopy() to change mem
if it can prove that current mem is produced by an arraycopy that does not
affect self and in that case L712 should be reached.

Should we return ac when executing L556?

Background: I'm currently working on JDK-8262295 [1] and my fix [2] changes
find_previous_arraycopy() and I'm trying to understand what needs to be returned
in that context if can_see_stored_value == true.

Thanks, Richard.

[1] JDK-8262295: C2: Out-of-Bounds Array Load from Clone Source
    https://bugs.openjdk.java.net/browse/JDK-8262295

[2] PR for JDK-8262295 [1]
    https://github.com/openjdk/jdk/pull/2708