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

[Vladimir Kozlov]

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