RFR (S) [8u] 8155635: C2: Mixed unsafe oop accesses break alias analysis

[Vladimir Kozlov]

Good.

Thanks,
Vladimir

On 10/19/18 8:52 PM, David Buck wrote:
> Hi Vladimir!
> 
>  > Yes, leaving comment will be good but you need to use @comment command
>  > for that. Otherwise it is treated as continuation of previous line.
> 
> Once again, thank you for the catch. It seems that I need to find a chunk of time and reread some of the jtreg docs.
> 
> Hopefully I now have all of my ducks in a row for this fix:
> 
> http://cr.openjdk.java.net/~dbuck/jdk8155635_jdk8_ver01/
> 
> Thanks again for looking at this for me.
> 
> Cheers,
> -Buck
> 
> 
> On 2018/10/20 10:17, Vladimir Kozlov wrote:
>> Yes, leaving comment will be good but you need to use @comment command for that. Otherwise it is treated as 
>> continuation of previous line. Or add comment in a separate comment block if jtreg for jdk8 does not support @comment.
>>
>> Regards,
>> Vladimir
>>
>> On 10/19/18 6:01 PM, David Buck wrote:
>>> Hi Vladimir!
>>>
>>> Thanks for taking a look at this.
>>>
>>>  > And I did not find in Bug or Review
>>>  > that it is G1 specific problem.
>>>
>>> Yeah, I'm confident that it is not G1-specific, but I failed in coming up with an easy way to reproduce the issue 
>>> without G1. Only after a frustrated afternoon of trying to get the testcase to reproduce on JDK 8 did I stumble on 
>>> Mikael's comment (2014-08-26 20:58) in JDK-8056058 [0] that G1 seemed to help trigger the issue.
>>>
>>>  > Specifying GC in test is 'no-no'. You can have conflict when testing
>>>  > environment pass different GC flag.
>>>
>>> I did not know about this rule. Thanks for catching this. How about, after removing the flag from the command line, I 
>>> add a comment saying that running with G1 seems to help trigger the issue? Something like this perhaps?
>>>
>>> ===
>>> /*
>>>   * @test
>>>   * @bug 8155635
>>>   * @library /testlibrary
>>>   * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -Xbatch -XX:-TieredCompilation 
>>> compiler.unsafe.MixedUnsafeStoreObject
>>>   * @run main/othervm -Xbatch compiler.unsafe.MixedUnsafeStoreObject
>>>   * Testcase currently only known to reproduce when run with -XX:+UseG1GC.
>>>   */
>>> ===
>>>
>>> BTW, this seems true for JDK 9 as well. For example, specifying UseParallelGC or UseConcMarkSweepGC with JDK 9 makes 
>>> the testcase pass.
>>>
>>> Cheers,
>>> -Buck
>>>
>>> [0] https://bugs.openjdk.java.net/browse/JDK-8056058
>>>
>>> On 2018/10/20 2:01, Vladimir Kozlov wrote:
>>>> Specifying GC in test is 'no-no'. You can have conflict when testing environment pass different GC flag. And I did 
>>>> not find in Bug or Review that it is G1 specific problem.
>>>>
>>>> Otherwise looks good.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 10/19/18 1:15 AM, David Buck wrote:
>>>>> Hi!
>>>>>
>>>>> May I please get a review of this straightforward backport:
>>>>>
>>>>> bug report:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8155635
>>>>>
>>>>> JDK 9 code review thread:
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-August/024250.html
>>>>>
>>>>> JDK 9 changeset:
>>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/bde4bcd58309
>>>>>
>>>>> JDK 8 webrev (for review):
>>>>> http://cr.openjdk.java.net/~dbuck/jdk8155635_jdk8_ver00/
>>>>>
>>>>> The only minor differences from the JDK 9 fix are the different assert message formatting [0] and the following 
>>>>> changes to the testcase:
>>>>>
>>>>> - use of com.oracle.java.testlibrary.Utils to get Unsafe
>>>>> - addition of -XX:+UseG1GC to command line (this is default in JDK 9, so explicit flag was not needed)
>>>>>
>>>>> Cheers,
>>>>> -Buck
>>>>>
>>>>> [0] https://bugs.openjdk.java.net/browse/JDK-8080775