[lworld] RFR: 8260363: [lworld] C2 compilation fails with assert(n->Opcode() != Op_Phi) failed: cannot match
Roland Westrelin
roland at openjdk.java.net
Thu Jan 28 12:31:21 UTC 2021
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.
-------------
Commit messages:
- adjust_flattened_array_access_aliases() fix
Changes: https://git.openjdk.java.net/valhalla/pull/317/files
Webrev: https://webrevs.openjdk.java.net/?repo=valhalla&pr=317&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8260363
Stats: 111 lines in 5 files changed: 98 ins; 0 del; 13 mod
Patch: https://git.openjdk.java.net/valhalla/pull/317.diff
Fetch: git fetch https://git.openjdk.java.net/valhalla pull/317/head:pull/317
PR: https://git.openjdk.java.net/valhalla/pull/317
More information about the valhalla-dev
mailing list