JDK-8265126 : [REDO] unified handling for VectorMask object re-materialization during de-optimization
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 17
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2021-04-13
  • Updated: 2021-05-24
  • Resolved: 2021-05-19
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17
17 b23Fixed
Related Reports
Cloners :  
JDK-8264954 - unified handling for VectorMask object re-materialization during de-optimization
Sub Tasks
JDK-8265084 :  
[BACKOUT] 8264954: unified handling for VectorMask object re-materialization during de-optimization - Resolved
Description
Currently mask values are propagated using vector registers, shape of mask vector matches the shape of the operands of a vector operation.
Objects allocated in the compiled frame are re-constructed during de-optimization.

To provide a unified handling for VectorMask instance reconstruction when mask is held either in a predicate register (for Intel AVX512 or ARM SVE) or vector register.
Comments
Changeset: 65a8bf58 Author: Jatin Bhateja <jbhateja@openjdk.org> Date: 2021-05-19 05:19:16 +0000 URL: https://git.openjdk.java.net/jdk/commit/65a8bf58bef1a3c50d434b0b351247b5a3a426cb
19-05-2021
Changing the title back to "[REDO]" as described here: https://openjdk.java.net/guide/#backing-out-a-change
28-04-2021
Hi Jatin [~jbhateja], If you look at Vladimir's log, I think something unexpected happened here: --N: o1842 VectorStoreMask === _ o1731 o248 [[o500 o1640 o1957 o524 121 182 ]] #vectord[4]:{bool} --N: o1731 LoadVector === o1712 o166 o1730 |o179 [[o1732 o1842 ]] @bool[int:>=0]:NotNull:exact[0] *, idx=10; mismatched #vectord[4]:{bool} We expect VectorStoreMask's input is a "mask" type vector (e.g. VectorLoadMask, VectorMaskCmp, AndV etc). However, its input is now a normal vector produced by "LoadVector" Look closer to your original code, the GVN transform [1] itself is correct, i.e. VectorStoreMask (VectorLoadMask v) ==> v So, the vec_value now is a normal vector v after transform. When safepoints size is larger than 0 [2], it loops back and tries to new a VectorStoreMaskNode with input of vec_value, which is now a normal vector instead of mask vector. I think that's why we see VectorStoreMask has input of LoadVector. If we re-initialize vec_value from vec_box->in(VectorBoxNode::Value) in each safepoint iteration, I think that might be fine? Regarding to aarch64 backend to support 4 bools/bytes, I think aarch64 limits normal vector to 8 bytes [3], we expect the type of VectorLoadMask is a normal vector type, so current patterns looks fine to me. [1] https://github.com/openjdk/jdk/blob/f71be8b5d712766ff8c3f19d131ec414c1b4b109/src/hotspot/share/opto/vector.cpp#L273 [2] https://github.com/openjdk/jdk/blob/f71be8b5d712766ff8c3f19d131ec414c1b4b109/src/hotspot/share/opto/vector.cpp#L250 [3] https://github.com/openjdk/jdk/blob/f71be8b5d712766ff8c3f19d131ec414c1b4b109/src/hotspot/cpu/aarch64/aarch64.ad#L2491
20-04-2021
Hi Ningsheng, 1) Existing patterns for VectorStoreMask from byte vector in aarch64_neon.as only handler vectors of length 8 and 16. instruct storemask8B(vecD dst, vecD src , immI_1 size) %{ predicate(n->as_Vector()->length() == 8); instruct storemask16B(vecX dst, vecX src , immI_1 size) %{ predicate(n->as_Vector()->length() == 16); Crash reported above by Vladimir is for length 4. 2) VectorStoreMask Identity conversion is folding following pattern which looks correct VectorStoreMask (VectorLoadMask mask) ==> mask 3) Other failures seen on x86 may not be related to this patch. https://bugs.openjdk.java.net/browse/JDK-8265317 Can you kindly extend VectorStoreMask instruction patterns for AARCH64 to support other legal vector lengths. Thanks, Jatin
19-04-2021
In original patch, + vec_value = gvn.transform(VectorStoreMaskNode::make(gvn, vec_value, bt, vt->length())); So the vec_value might become a non-mask vector after gvn transform, and when it loops back, it is not the expected mask node.
14-04-2021
Some Vector API cases also failed on my x86 system, with option -XX:+DeoptimizeALot.
14-04-2021
Running with fastdebug VM on aarch64 I got crash in matcher when trying to match VectorStoreMask (I think it is one which was introduced in JDK-8264954): jdk/incubator/vector/Short64VectorTests.java o1842 VectorStoreMask === _ o1731 o248 [[o500 o1640 o1957 o524 121 182 ]] #vectord[4]:{bool} --N: o1842 VectorStoreMask === _ o1731 o248 [[o500 o1640 o1957 o524 121 182 ]] #vectord[4]:{bool} --N: o1731 LoadVector === o1712 o166 o1730 |o179 [[o1732 o1842 ]] @bool[int:>=0]:NotNull:exact[0] *, idx=10; mismatched #vectord[4]:{bool} VECD 0 VECD --N: o248 ConI === o0 [[o1639 o2496 o2100 o1842 ]] #int:1 # Internal Error (/workspace/open/src/hotspot/share/opto/matcher.cpp:1685), pid=3323106, tid=3323121 # assert(false) failed: bad AD file # # Problematic frame: # V [libjvm.so+0x12d11cc] Matcher::Label_Root(Node const*, State*, Node*, Node*&)+0xa1c V [libjvm.so+0x12d11cc] Matcher::Label_Root(Node const*, State*, Node*, Node*&)+0xa1c V [libjvm.so+0x12d1560] Matcher::match_tree(Node const*)+0x180 V [libjvm.so+0x12db4b8] Matcher::xform(Node*, int)+0xf94 V [libjvm.so+0x12df908] Matcher::match()+0x2918 V [libjvm.so+0x998ff8] Compile::Code_Gen()+0x94 V [libjvm.so+0x9a2bdc] Compile::Compile(ciEnv*, ciMethod*, int, bool, bool, bool, bool, DirectiveSet*)+0x1018
13-04-2021
[~kvn], I think CPU/OS should still be "generic" for this REDO because the enhancement is not aarch64 specific.
13-04-2021
Before pushing implementation someone from Oracle have to run mach5 testing up-to tier3 at least.
13-04-2021
Failure happens only on Aarch64. It seems JDK-8264954 did not take into account Vector API implementation on Aarch64. Failed tests are next incubator vector tests: jdk/incubator/vector/ShortMaxVectorTests.java jdk/incubator/vector/Short128VectorTests.java jdk/incubator/vector/Byte64VectorTests.java jdk/incubator/vector/Byte128VectorTests.java jdk/incubator/vector/ByteMaxVectorTests.java No additional flags passed to tests when they run. Here is the issue (it seems Aarch64 code does not support these types of vectors): java.lang.UnsupportedOperationException: Bad vector element type: short (should be either int or long, to reinterpret as float or double) at jdk.incubator.vector/jdk.incubator.vector.LaneType.badElementType(LaneType.java:170) at jdk.incubator.vector/jdk.incubator.vector.LaneType.asFloating(LaneType.java:104) at jdk.incubator.vector/jdk.incubator.vector.ShortVector.viewAsFloatingLanes(ShortVector.java:3362) at ShortMaxVectorTests.viewAsFloatingLanesTest(ShortMaxVectorTests.java:1245)
13-04-2021
The original fix was backed out by JDK-8265084 due to JDK-8265083.
13-04-2021