Potential FloatVector.fromArray miscompilation when using index map on AVX-512 machine

[Joel Knighton]

Hello,
I have a very repeatable JVM crash when using the Panama Vector API that
appears to be due to improper assembly emitted when compiling
FloatVector.fromArray in certain circumstances. This crash occurs on (at
least) OpenJDK 20 (OpenJDK Runtime Environment Temurin-20.0.2+9 (build
20.0.2+9)), OpenJDK 21 (OpenJDK Runtime Environment
Temurin-21+35-202309042127 (build 21-beta+35-ea)
), and OpenJDK 22-internal-adhoc built from the latest in source control. I
haven't yet been able to produce the compilation conditions necessary in a
neatly isolated JMH benchmark, but I hope to sufficiently illustrate the
conditions such that inspection of the relevant code can confirm or deny
whether I'm diagnosing the problem correctly.

The crash can occur when using `FloatVector.fromArray(VectorSpecies<Float>
species, float[] a, int offset, int[] indexMap, int mapOffset)` on a
machine with AVX-512 extensions. hsdis on the crash dump shows the crash
occurs at an instruction of the form `vgatherdps
(%rsi,%zmm27,4),%zmm0{%k7}`. In particular, when using VSIB addressing, I
noticed the index register was used nowhere else in the disassembly and
seemed incorrect, as earlier assembly preparing the indexMap used a
different destination register. Preceding the referenced `vgatherdps ...`
instruction, the preparation of the indexMap was using destination %zmm19.
Instrumenting assembler_x86 confirms that the code generating the
`vgatherdps` intended the index register encoding to be 19. The crash never
occurs when `vgatherdps` is emitted using index registers available prior
to AVX-512.

With the addressing used here, I expect the index register encoding to be
split across EVEX.V’, EVEX.X, and SIB.index. Since the register encoding is
always off by 8 when the crash occurs, I suspect EVEX.X, the fourth bit.
Inspection of `Assembler::evgatherdps` shows that `Assembler::vex_prefix`
is responsible for choosing the value of EVEX.X. vex_prefix defers this
responsibility to `adr.xmmindex_needs_rex()` for the source address `adr`,
which checks that the encoding of xmmindex is greater than or equal to 8.
This check appears insufficient for AVX-512 extensions, where we wouldn't
expect the fourth bit (EVEX.X) to be set for registers 16-23.

On an OpenJDK build I produced with patched `xmmindex_needs_rex` to not set
EVEX.X for index registers zmm16-23, I was able to run the code without
crashing. I'm not sure if this is a correct or comprehensive fix, but it
seems to further confirm my understanding of the cause of the crash.

Here are gists of the patch I used (
https://gist.github.com/jkni/d81262034917c4039c6f3a217dc5c04b) and an
excerpt of the crash dump (
https://gist.github.com/jkni/5854501cce6ce23e15d93eae1f7c2da2).

Please let me know whether additional information would be helpful in
diagnosing this issue. I would appreciate any insight.

Thanks,
Joel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/panama-dev/attachments/20231004/294a48af/attachment.htm>