[lworld] RFR: 8260363: [lworld] C2 compilation fails with assert(n->Opcode() != Op_Phi) failed: cannot match

Tobias Hartmann thartmann at openjdk.java.net
Mon Feb 1 10:12:04 UTC 2021


On Thu, 28 Jan 2021 12:26:39 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> This is caused by a StoreCM whose OopStore doesn't point to a Store
> node because it was optimized out but to a memory Phi. The Phi is for
> slice TypeAryPtr::INLINES. When
> Compile::adjust_flattened_array_access_aliases() runs, the
> TypeAryPtr::INLINES slice becomes dead and is replaced by independent
> slices for each field of flat array elements. Because the StoreCM
> doesn't point to a Store, it's impossible to tell what slice it's for
> so its input still points to the TypeAryPtr::INLINES Phi once
> Compile::adjust_flattened_array_access_aliases() is over.
> 
> Given the TypeAryPtr::INLINES slice is dead that's a problem: assuming
> we can end up with a StoreCM that points to a Phi for a non eliminated
> Store (the Store would be behind the Phi I suppose in that case), then
> after Compile::adjust_flattened_array_access_aliases() runs, the Store
> would have moved to a new Phi but the StoreCM wouldn't have been
> updated.
> 
> But that's not what causes the crash. The crash occurs because the
> TypeAryPtr::INLINES memory subgraph is partially killed by
> Compile::adjust_flattened_array_access_aliases() so when matching
> occurs the Phi is only reachable through a precedence edge from the
> StoreCM (final graph reshaping moves the OopStore input to precedence
> edges). It's unreachable from root at matching time (because that code
> doesn't follow precendence edge) and is not marked as shared
> by Matcher::find_shared().
> 
> When Compile::adjust_flattened_array_access_aliases() runs, the
> StoreCM OopStore is actually first set to a MergeMem node that
> captures slices created for flat array fields but when
> StoreCMNode::Ideal() executes it sets the input to the
> TypeAryPtr::INLINES input of the MergeMem. I think that last steps is
> what's incorrect after
> Compile::adjust_flattened_array_access_aliases(). So I changed that
> logic so it keeps the MergeMem input and also changed final graph
> reshape so it adds as many precedence edges as there are flat field
> inputs to the MergeMem. These changes are required to preserve
> correctness, I believe.
> 
> With that, i still hit the assert because the bottom memory input to
> the MergeMem can end up as a precedence edge to the StoreCM (if that
> entry to the MergeMem is top). That bottom memory in the case of the
> regression test is a Phi that was created by
> Compile::adjust_flattened_array_access_aliases() and is usually
> optimized out. It's only reachable from the StoreCM precedence edge. I
> don't think there's a fundamental problem here so I change matching so
> it looks at precedence edges when looking for shared node. I also had
> to change node scheduling in a similar way.

Looks scary but reasonable to me! :)

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

Marked as reviewed by thartmann (Committer).

PR: https://git.openjdk.java.net/valhalla/pull/317


More information about the valhalla-dev mailing list