[14] RFR (S): 8226411: C2: Avoid memory barriers around off-heap unsafe accesses

[Vladimir Ivanov]

> Please carry on with your changes then.

Just in case: if the bug severely affects Shenandoah and the fix 
requires time, as the stop-the-gap solution I can keep barriers around 
off-heap accesses when Shenandoah is used until proper fix arrives.

Best regards,
Vladimir Ivanov

> 
> Thanks,
> Roman
> 
>> Roman,
>>
>> JDK-8220714 looks like a bug in Shenandoah barrier expansion.
>>
>> I slightly modified the test to simplify the analysis [1].
>>
>> While running the test [2] I'm seeing the following:
>>
>>    127 LoadI  === 44   7 125
>>    160 StoreI === 44   7  91 127
>>     94 LoadI  === 44   7  91
>>    193 StoreI === 44 160 125  94
>>
>> After expansion is over, it looks as follows:
>>
>>    127 LoadI  ===  44 209 125
>>    160 StoreI ===  44 209  91 127
>>     94 LoadI  ===  44 160  91
>>    193 StoreI ===  44 160 125  94
>>
>> Note that 94 LoadI depends on 160 StoreI memory now. Before the
>> expansion they were independent (7 Parm == initial memory state).
>>
>> And then 94 goes away, since it now reads updated value:
>>
>> <         < 94    LoadI    === _ _ _  [[]]   [3200094]
>>> int         127    LoadI    ===  44  209  125  [[ 160  193 ]]
>> @rawptr:BotPTR, idx=Raw; unsafe #int (does not depend only on test)
>> !orig=[94] !jvms: Unsafe::getInt @ bci:3
>> TestUnsafeOffheapSwap$Memory::getInt @ bci:14
>> TestUnsafeOffheapSwap$Memory::swap @ bci:10
>> TestUnsafeOffheapSwap::testUnsafeHelper @ bci:7
>>
>> The final graph is evidently wrong:
>>
>>    127 LoadI  ===  44 209 125
>>    160 StoreI ===  44 209  91 127
>>    193 StoreI ===  44 160 125 127
>>
>> Let me know how you want to proceed with it.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1]
>>
>> diff --git
>> a/test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeOffheapSwap.java
>> b/test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeOffheapSwap.java
>> --- a/test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeOffheapSwap.java
>> +++ b/test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeOffheapSwap.java
>> @@ -57,6 +57,16 @@
>>           }
>>       }
>>
>> +    static void testUnsafeHelper(int i) {
>> +        mem.swap(i - 1, i);
>> +    }
>> +
>> +    static void testArrayHelper(int[] arr, int i) {
>> +        int tmp = arr[i - 1];
>> +        arr[i - 1] = arr[i];
>> +        arr[i] = tmp;
>> +    }
>> +
>>       static void test() {
>>           Random rnd = new Random(SEED);
>>           for (int i = 0; i < SIZE; i++) {
>> @@ -72,10 +82,8 @@
>>           }
>>
>>           for (int i = 1; i < SIZE; i++) {
>> -            mem.swap(i - 1, i);
>> -            int tmp = arr[i - 1];
>> -            arr[i - 1] = arr[i];
>> -            arr[i] = tmp;
>> +            testUnsafeHelper(i);
>> +            testArrayHelper(arr, i);
>>           }
>>
>>           for (int i = 0; i < SIZE; i++) {
>>
>> [2] $ java -cp
>> JTwork/classes/gc/shenandoah/compiler/TestUnsafeOffheapSwap.d/
>> --add-exports=java.base/jdk.internal.misc=ALL-UNNAMED
>> -XX:-UseOnStackReplacement -XX:-BackgroundCompilation
>> -XX:-TieredCompilation  -XX:+UnlockExperimentalVMOptions
>> -XX:+UseShenandoahGC -XX:+PrintCompilation -XX:CICompilerCount=1
>> -XX:CompileCommand=quiet
>> -XX:CompileCommand=compileonly,*::testUnsafeHelper
>> -XX:CompileCommand=print,*::testUnsafeHelper -XX:PrintIdealGraphLevel=0
>> -XX:-VerifyOops -XX:-UseCompressedOops TestUnsafeOffheapSwap
>>
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 29.11.2019 20:18, Vladimir Ivanov wrote:
>>>
>>>> Does it affect this:
>>>> https://bugs.openjdk.java.net/browse/JDK-8220714
>>>
>>> Good point, Roman. Proposed patch breaks the fix for JDK-8220714.
>>>
>>> I'll investigate what happens there and come back with a revised fix.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>>>> http://cr.openjdk.java.net/~vlivanov/8226411/webrev.00/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8226411
>>>>>
>>>>> There were a number of fixes in C2 support for unsafe accesses recently
>>>>> which led to additional memory barriers around them. It improved
>>>>> stability, but in some cases it was redundant. One of important use
>>>>> cases which regressed is off-heap accesses [1]. The barriers around
>>>>> them
>>>>> are redundant because they are serialized on raw memory and don't
>>>>> intersect with any on-heap accesses.
>>>>>
>>>>> Proposed fix skips memory barriers around unsafe accesses which are
>>>>> provably off-heap (base == NULL).
>>>>>
>>>>> It (almost completely) recovers performance on the microbenchmark
>>>>> provided in JDK-8224182 [1].
>>>>>
>>>>> Testing: tier1-6.
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov
>>>>>
>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8224182
>>>>>
>>>>
>>
>