RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Roland Westrelin-3

http://cr.openjdk.java.net/~roland/8193518/webrev.00/

If a method has several vectorized loops, the logic that keep tracks of
Compile::_max_vector_size sets it to the max vector size of the last
loop (and not to the max vector size of all
loops). Compile::_max_vector_size is then used to mark nmethods that
need vector registers to be saved on a safepoint. If the last loop uses
small vectors but other loops in the method use large vectors, registers
are not saved correctly at a safepoint in one of the other loops and can
be corrupted.

The test case only fails with Serial GC AFAICT but I saw that failure
with G1 running some other applications.

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Nils Eliasson
Hi Roland,

Thanks for finding this bug!

You need to initalize _max_vector_size in Compile::init.

// Nils


On 2017-12-14 17:00, Roland Westrelin wrote:

> http://cr.openjdk.java.net/~roland/8193518/webrev.00/
>
> If a method has several vectorized loops, the logic that keep tracks of
> Compile::_max_vector_size sets it to the max vector size of the last
> loop (and not to the max vector size of all
> loops). Compile::_max_vector_size is then used to mark nmethods that
> need vector registers to be saved on a safepoint. If the last loop uses
> small vectors but other loops in the method use large vectors, registers
> are not saved correctly at a safepoint in one of the other loops and can
> be corrupted.
>
> The test case only fails with Serial GC AFAICT but I saw that failure
> with G1 running some other applications.
>
> Roland.

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Nils Eliasson
In reply to this post by Roland Westrelin-3
Sorry, I was fast and wrong.

Looks good,

Thanks for fixing Roland,

//Nils

----- Original Message -----
From: [hidden email]
To: [hidden email]
Sent: Thursday, December 14, 2017 5:12:39 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna
Subject: Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Hi Roland,

Thanks for finding this bug!

You need to initalize _max_vector_size in Compile::init.

// Nils


On 2017-12-14 17:00, Roland Westrelin wrote:

> http://cr.openjdk.java.net/~roland/8193518/webrev.00/
>
> If a method has several vectorized loops, the logic that keep tracks of
> Compile::_max_vector_size sets it to the max vector size of the last
> loop (and not to the max vector size of all
> loops). Compile::_max_vector_size is then used to mark nmethods that
> need vector registers to be saved on a safepoint. If the last loop uses
> small vectors but other loops in the method use large vectors, registers
> are not saved correctly at a safepoint in one of the other loops and can
> be corrupted.
>
> The test case only fails with Serial GC AFAICT but I saw that failure
> with G1 running some other applications.
>
> Roland.

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Roland Westrelin-3
In reply to this post by Nils Eliasson

Hi Nils,

Thanks for taking a look at this...

> You need to initalize _max_vector_size in Compile::init.

...and verifying something I didn't.

There's a:
set_max_vector_size(0);
in Compile::Init() so it should be all good.

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Tobias Hartmann-2
In reply to this post by Roland Westrelin-3
Hi Roland,

looks good to me too.

Best regards,
Tobias

On 14.12.2017 17:00, Roland Westrelin wrote:

>
> http://cr.openjdk.java.net/~roland/8193518/webrev.00/
>
> If a method has several vectorized loops, the logic that keep tracks of
> Compile::_max_vector_size sets it to the max vector size of the last
> loop (and not to the max vector size of all
> loops). Compile::_max_vector_size is then used to mark nmethods that
> need vector registers to be saved on a safepoint. If the last loop uses
> small vectors but other loops in the method use large vectors, registers
> are not saved correctly at a safepoint in one of the other loops and can
> be corrupted.
>
> The test case only fails with Serial GC AFAICT but I saw that failure
> with G1 running some other applications.
>
> Roland.
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Vladimir Kozlov
In reply to this post by Roland Westrelin-3
Nice find. Looks good. Why you changed type to uint?

Thanks,
Vladimir

On 12/14/17 8:00 AM, Roland Westrelin wrote:

>
> http://cr.openjdk.java.net/~roland/8193518/webrev.00/
>
> If a method has several vectorized loops, the logic that keep tracks of
> Compile::_max_vector_size sets it to the max vector size of the last
> loop (and not to the max vector size of all
> loops). Compile::_max_vector_size is then used to mark nmethods that
> need vector registers to be saved on a safepoint. If the last loop uses
> small vectors but other loops in the method use large vectors, registers
> are not saved correctly at a safepoint in one of the other loops and can
> be corrupted.
>
> The test case only fails with Serial GC AFAICT but I saw that failure
> with G1 running some other applications.
>
> Roland.
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Roland Westrelin-3

> Nice find. Looks good. Why you changed type to uint?

Thanks for the review.

gcc complains that the new comparison I added in superword.cpp is signed
vs unsigned and it seems natural that _max_vector_size be unsigned.

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Roland Westrelin-3
In reply to this post by Tobias Hartmann-2

Thanks for the review, Tobias.

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Vladimir Kozlov
In reply to this post by Roland Westrelin-3
Got it. Thanks.

Tobias, please, sponsor it (test and push).

Vladimir

On 12/14/17 8:33 AM, Roland Westrelin wrote:

>
>> Nice find. Looks good. Why you changed type to uint?
>
> Thanks for the review.
>
> gcc complains that the new comparison I added in superword.cpp is signed
> vs unsigned and it seems natural that _max_vector_size be unsigned.
>
> Roland.
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Tobias Hartmann-2

On 14.12.2017 17:37, Vladimir Kozlov wrote:
> Tobias, please, sponsor it (test and push).

Sure, I'll do so.

Best regards,
Tobias
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Tobias Hartmann-2
Hi,

The new TestVectorsNotSavedAtSafepoint.java fails on all platforms with "java.lang.OutOfMemoryError: Java heap space". I
think this is due to the GarbageProducerThread allocating thousands of Objects:

java.lang.OutOfMemoryError: Java heap space
at TestVectorsNotSavedAtSafepoint$GarbageProducerThread.run(TestVectorsNotSavedAtSafepoint.java:54)

Best regards,
Tobias

On 14.12.2017 17:41, Tobias Hartmann wrote:
>
> On 14.12.2017 17:37, Vladimir Kozlov wrote:
>> Tobias, please, sponsor it (test and push).
>
> Sure, I'll do so.
>
> Best regards,
> Tobias
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Roland Westrelin-3

Hi Tobias,

> The new TestVectorsNotSavedAtSafepoint.java fails on all platforms with "java.lang.OutOfMemoryError: Java heap space". I
> think this is due to the GarbageProducerThread allocating thousands of Objects:
>
> java.lang.OutOfMemoryError: Java heap space
> at TestVectorsNotSavedAtSafepoint$GarbageProducerThread.run(TestVectorsNotSavedAtSafepoint.java:54)

Thanks for running the tests. Can you try the new test?

http://cr.openjdk.java.net/~roland/8193518/webrev.01/

Roland.
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Tobias Hartmann-2
Hi Roland,

On 15.12.2017 13:26, Roland Westrelin wrote:
> Thanks for running the tests. Can you try the new test?
>
> http://cr.openjdk.java.net/~roland/8193518/webrev.01/

Thanks, I'll re-run testing.

Best regards,
Tobias

P.S. Please make sure to avoid trailing whitespaces in your patches:
src/hotspot/share/opto/superword.cpp:2444: Trailing whitespace
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XS): 8193518: C2: Vector registers sometimes corrupted at safepoint

Roland Westrelin-3

Thanks for pushing the fix, Vladimir.

Roland.