[jdk18] RFR: 8279654: jdk/incubator/vector/Vector256ConversionTests.java crashes randomly with SVE
Fei Gao
fgao at openjdk.java.net
Thu Jan 13 06:40:51 UTC 2022
While testing some vector api cases on SVE system, we see a random
failure:
"assert(C->node_arena()->contains(s->_leaf) || !has_new_node
(s->_leaf)) failed: duplicating node that's already been matched".
The root cause is that the if-condition in pd_clone_node()[1] is not
aligned with predicate conditions[2] of match rules in the backend.
Not all StoreVector (VectorStoreMask src) node patterns can be matched
and subsumed into one instruction in the backend. For example, there
is no match rule to combine them when the element basic type is
T_BYTE, as the cost of VectorStoreMask for byte type is relative
low and there is no need to do narrow operations.
Here is the analysis about root cause in detail. When a multi-use
VectorStoreMask node, whose type is byte, showed as below,
is identified by pd_clone_node() successfully, it will not be set
shared in the stage of find_shared() if only the root selector visits
the call node earlier than StoreVector node[3]. The preorder walk
in xform()[4][5] visits the VectorStoreMask node and reduces it by
ReduceInst() starting from the call node first, then visits the
VectorStoreMask node again starting from the StoreVector node as
mentioned before. During the second visit, in the stage of
Label_Root(), the postorder walk along use-def edges starts from the
StoreVector node and then visits the VectorStoreMask node. It takes
the VectorStoreMask node as an interior of the subtree and generates
the corresponding state tree[6]. But in the stage of
ReduceInst_Interior(), there is no internal rule in the backend to
reduce the VectorStoreMask node as the state tree guides. Thus, the
reducer has to reduce the VectorStoreMask node independently and visit
it by ReduceInst()[7]. Therefore, it tries to reduce the VectorStoreMask
node twice by ReduceInst(), resulting in the assertion failure. Assuming
that there was a match rule to combine these two byte nodes,
StoreVector (VectorStoreMask src), it could find an internal rule in the
backend to help skip the interior VectorStoreMask node, do nothing
but recurse[8], and no assertion happens. If we delete the check for
VectorStoreMask in pd_clone_node(), the VectorStoreMask node is going to
be set shared in find_shared() and could be definitely reduced only once
with no assertion failure[9][10].
There are two different methods to fix this issue.
The first one is setting the condition in pd_clone_node() the same as
matching rules, making sure that all instruction sequences declared in
pd_clone_node() can be subsumed by matching rules. However, the
condition code has to be revisited when matching rules change in the
future. It's not easy to maintain.
The other one is that we can remove the if-condition code for
VectorStoreMask in pd_clone_node(). As a result, when a VectorStoreMask
node has multiple use, it can't be matched by any chained rules. But
after JDK-8273949, a multi-use VectorStoreMask node only exists for
safepoint[11], which is not very common. Furthermore, even if a
multi-use VectorStoreMask node occurs, the pattern is identified in the
pd_clone_node() and a match rule is ready to subsume it in the backend,
the combination may not happen due to some unexpected matching order
issue. For example, in this case,
if we visit the StoreVector node first then the call node, the
VectorStoreMask node is still going to be set shared and the prevention
from pd_clone_node() doesn't work based on the current logic of
find_shared()[12]. Once the node is set shared, there is no code path to
use internal combination rule to subsume the chained
StoreVector-VectorStoreMask nodes into one machine instruction.
Based on the above mentioned considerations, to make the code more
maintainable, we choose the second one, which decouples pd_clone_node()
from predicate conditions of matching rules in the backend with minimal
impact on performance.
I tested the patch using all vector cases under compiler/vectorization,
compiler/vectorapi and jdk/incubator/vector on SVE for multiple times.
It passed all these tests.
[1] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/cpu/aarch64/aarch64.ad#L2738
[2] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/cpu/aarch64/aarch64_sve.ad#L2101
[3] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L2151
[4] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L1097
[5] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L1119
[6] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L1694
[7] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L1977
[8] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L1975
[9] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L1692
[10] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L1969
[11] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/vector.cpp#L262
[12] https://github.com/openjdk/jdk/blob/8d1a1e83f40f7a147e033be6b2221c1bb1abd8ab/src/hotspot/share/opto/matcher.cpp#L2125
-------------
Commit messages:
- 8279654: jdk/incubator/vector/Vector256ConversionTests.java crashes randomly with SVE
Changes: https://git.openjdk.java.net/jdk18/pull/98/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk18&pr=98&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8279654
Stats: 5 lines in 1 file changed: 0 ins; 3 del; 2 mod
Patch: https://git.openjdk.java.net/jdk18/pull/98.diff
Fetch: git fetch https://git.openjdk.java.net/jdk18 pull/98/head:pull/98
PR: https://git.openjdk.java.net/jdk18/pull/98
More information about the hotspot-compiler-dev
mailing list