[9] RFR(S): 8148490: RegisterSaver::restore_live_registers() fails to restore xmm registers on 32 bit

[Tobias Hartmann]

Hi Uwe,

On 02.02.2016 00:17, Uwe Schindler wrote:
> Hi Tobias,
> 
> thanks for taking care about this issue! Since last week we get tons of failures on 32bit JVMs with Apache Lucene (AVX2/Haswell machine). It looks like those were caused by this issue, because the previously tested build 94 worked perfectly (and this one was before this got broken). We are currently confirming with -XX:-UseSuperWord that this was caused by this issue. So we hope for an fixed EA build soon (together with the compact strings fix):
> 
> jenkins at serv1:~/tools/java/32bit/jdk-9-ea+102/bin$ ./java -XX:+PrintFlagsFinal -version | grep AVX
>      intx UseAVX                                    = 2                                   {ARCH product}
> java version "9-ea"
> Java(TM) SE Runtime Environment (build 9-ea+102-2016-01-21-001243.javare.4316.nc)
> Java HotSpot(TM) Server VM (build 9-ea+102-2016-01-21-001243.javare.4316.nc, mixed mode)

Thanks for letting me know! The problem was introduced with JDK-8142980 in b96.

The fix for Compact Strings (JDK-8144212) is making its way through the repos. It's currently in the jdk9-dev repo [1] and will we pushed to master soon.

> P.S.: This issue was shown on FOSDEM as example for the JDK9 testing we do: https://fosdem.org/2016/schedule/event/bug_hunting_lucene/, page/slide 39
> P.P.S.: Maybe add label "apache-lucene-found" to this issue :-)

Sure, done.

Best,
Tobias

[1] http://hg.openjdk.java.net/jdk9/dev/hotspot/rev/bf74058d67ec


> -----
> Uwe Schindler
> uschindler at apache.org 
> ASF Member, Apache Lucene PMC / Committer
> Bremen, Germany
> http://lucene.apache.org/
>> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
>> bounces at openjdk.java.net] On Behalf Of Tobias Hartmann
>> Sent: Friday, January 29, 2016 3:16 PM
>> To: hotspot-compiler-dev at openjdk.java.net
>> Subject: [9] RFR(S): 8148490: RegisterSaver::restore_live_registers() fails to
>> restore xmm registers on 32 bit
>>
>> Hi,
>>
>> please review the following patch:
>> https://bugs.openjdk.java.net/browse/JDK-8148490
>> http://cr.openjdk.java.net/~thartmann/8148490/webrev.00/
>>
>> RegisterSaver::save_live_registers() and
>> RegisterSaver::restore_live_registers() are used by the safepoint handling
>> code to save and restore registers. The following code is emitted to save and
>> restore XMM/YMM registers on 32 bit:
>>
>> Save:
>>    ...
>>    0xf34ca12e:	vmovdqu %xmm0,0xb0(%esp)
>>    0xf34ca137:	vmovdqu %xmm1,0xc0(%esp)
>>    ...
>>    0xf34ca16d:	vmovdqu %xmm7,0x120(%esp)
>>    0xf34ca176:	sub    $0x80,%esp
>>    0xf34ca17c:	vextractf128 $0x1,%ymm0,(%esp)
>>    0xf34ca183:	vextractf128 $0x1,%ymm1,0x10(%esp)
>>    ...
>>    0xf34ca1b3:	vextractf128 $0x1,%ymm7,0x70(%esp)
>>    ...
>>
>> Restore:
>>    ...
>>    0xf34ca202:	vinsertf128 $0x1,(%esp),%ymm0,%ymm0
>>    0xf34ca209:	vinsertf128 $0x1,0x10(%esp),%ymm1,%ymm1
>>    ...
>>    0xf34ca239:	vinsertf128 $0x1,0x70(%esp),%ymm7,%ymm7
>>    0xf34ca241:	add    $0x80,%esp
>>    0xf34ca247:	vmovdqu 0x130(%esp),%xmm0
>>    0xf34ca250:	vmovdqu 0x140(%esp),%xmm1
>>    ...
>>    0xf34ca286:	vmovdqu 0x1a0(%esp),%xmm7
>>    ...
>>
>> The stack offsets for the vmovdqu instructions are wrong, causing the XMM
>> registers to contain random values after a safepoint. The problem is that
>> "additional_frame_bytes" is added to the stack offset although the stack
>> pointer is incremented just before:
>>
>> 283     __ addptr(rsp, additional_frame_bytes); // Save upper half of YMM
>> registers
>>
>> The regression test fails with "Test failed: array[0] = 1973.0 but should be
>> 10.000" because the vectorized loop returns a wrong result.
>>
>> I spotted and fixed the following other problems:
>> - the vmovdqu instructions should be emitted before restoring YMM and
>> ZMM because they zero the upper part of the XMM registers (i.e.
>> YMM/ZMM)
>> - if 'UseAVX > 2' is set/available, we save the ZMM registers as well but we
>> do not increment 'additional_frame_words' accordingly (we need another
>> 8*32 bytes of stack space)
>>
>> Unfortunately, I don't have access to a CPU with the AVX-512 instruction set
>> to test the "UseAVX > 2" related changes. Michael, could you verify the
>> changes?
>>
>> The problems were introduced by the fix for JDK-8142980.
>>
>> Thanks,
>> Tobias
>