RFR (S): 8007898: Incorrect optimization of Memory Barriers in Matcher::post_store_load_barrier()

[Vladimir Kozlov]

Than you, Aleksey

On 7/11/13 6:36 AM, Aleksey Shipilev wrote:
> On 07/11/2013 07:38 AM, Vladimir Kozlov wrote:
>> http://cr.openjdk.java.net/~kvn/8007898/webrev/
>
> This looks good, but the regression test needs cleanup, see below.
>
>> I also cleaned/fixed corresponding code in Matcher which tries to
>> optimize out those multiply barriers. The problem with it was it did not
>> take into account biased locking when cas could be not executed.
>
> That's an interesting corner case, thanks for fixing it! I can't readily
> reproduce it though, here are two potential jcstress tests [1].
>
> On x86 maintaining the total store order, the effect can only be due to
> the compiler reordering, yet it does not happen with -XX:+StressLCM/GCM.
> The disassembly also shows the release barrier is emitted.
>
> Volkner, Martin, Goetz, can you try those on your PPC C2 port?
>
>> Added regression test which shows the problem with -XX:+StressLCM[GCM]
>> flags added by Aleksey some time ago.
>
> That regression test is the rewrite of the existing jcstress test. I
> think it needs cleanup and the rename, see the update here:
>    http://cr.openjdk.java.net/~shade/8007898/DekkerTest.java

I will update the test as you suggested.

>
> Note that because StressLCM/GCM are only working once during the code
> generation, this test is inherently intermittently passing. I don't see
> how to tell jtreg to do a few runs in the row, is it possible? Or, can
> we use whitebox API to deoptimize actor1/2?

I also noticed intermittent test behavior but it almost always fail once of 3 runs. I can repeat 3 times '@run' command 
to run it several times.

>
> For the record, the complete jcstress [2] run with -XX:+StressLCM/GCM on
> Solaris 10/x86_64 yield no failures on the patched build. That means
> lots of previous failures are accounted for with this patch.
>
>> Aleksey is testing performance of these changes and no regression is
>> observed so far.
>
> Yes, the targeted microbenchmarks [3] show no red flags [4]. The larger
> workloads are not regressing either, at least SPECjvm98 and SPECjbb2005
> are unaffected. SPECjvm2008 and SPECjbb2013 runs are still pending, I'll
> update in 24 hours.

Thank you for doing this testing.

Vladimir

>
> -Aleksey.
>
> [1] http://hg.openjdk.java.net/code-tools/jcstress/rev/6c339a5aa00d
> [2] openjdk.java.net/projects/code-tools/jcstress/
> [3] http://cr.openjdk.java.net/~shade/8007898/volatile-reorder.tar.gz
> [4] http://cr.openjdk.java.net/~shade/8007898/micros-scores.txt
>