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

[David Buck]

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