[8u] RFR for JDK-8134918 - C2: Type speculation produces mismatched unsafe accesses

Mon Nov 14 17:50:01 UTC 2016

On 11/14/16 1:03 AM, Shafi Ahmad wrote:
> Hi Vladimir,
>
> Thanks for the review.
>
> Please find updated webrevs.
>
> All webrevs are with respect to the base changes on JDK-8140309.
> http://cr.openjdk.java.net/~shshahma/8140309/webrev.00/

Why you kept unaligned parameter in changes?

The test TestUnsafeUnalignedMismatchedAccesses.java will not work since 
since Unsafe class in jdk8 does not have unaligned methods.
Hot did you run it?

> http://cr.openjdk.java.net/~shshahma/8134918/webrev.01/

Good. Did you run new UnsafeAccess.java test?

> http://cr.openjdk.java.net/~shshahma/8162101/webrev.01/

Good.

Thanks,
Vladimir

>
> Regards,
> Shafi
>
>
>
>> -----Original Message-----
>> From: Vladimir Kozlov
>> Sent: Friday, November 11, 2016 1:26 AM
>> To: Shafi Ahmad; hotspot-dev at openjdk.java.net
>> Subject: Re: [8u] RFR for JDK-8134918 - C2: Type speculation produces
>> mismatched unsafe accesses
>>
>> On 11/9/16 10:42 PM, Shafi Ahmad wrote:
>>> Hi,
>>>
>>> Please review the backport of following dependent backports.
>>>
>>> jdk9 bug link: https://bugs.openjdk.java.net/browse/JDK-8136473
>>> Conflict in file src/share/vm/opto/memnode.cpp due to 1.
>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/fe311de64c61 [JDK-
>> 8080289]. Manual merge is not done as the corresponding code is not there
>> in jdk8u-dev.
>>> Multiple conflicts in file src/share/vm/opto/library_call.cpp and manual
>> merge is done.
>>> webrev link: http://cr.openjdk.java.net/~shshahma/8136473/webrev.00/
>>
>> unaligned unsafe access methods were added in jdk 9 only. In your changes
>> unaligned argument is always false. You can simplify changes.
>>
>> Also you should base changes on JDK-8140309 (original 8136473 changes
>> were backout by 8140267):
>>
>> On 11/4/15 10:21 PM, Roland Westrelin wrote:
>>  > http://cr.openjdk.java.net/~roland/8140309/webrev.00/
>>  >
>>  > Same as 8136473 with only the following change:
>>  >
>>  > diff --git a/src/share/vm/opto/library_call.cpp
>> b/src/share/vm/opto/library_call.cpp
>>  > --- a/src/share/vm/opto/library_call.cpp
>>  > +++ b/src/share/vm/opto/library_call.cpp
>>  > @@ -2527,7 +2527,7 @@
>>  >     // of safe & unsafe memory.
>>  >     if (need_mem_bar) insert_mem_bar(Op_MemBarCPUOrder);
>>  >
>>  > -  assert(is_native_ptr || alias_type->adr_type() == TypeOopPtr::BOTTOM
>> ||  > +  assert(alias_type->adr_type() == TypeRawPtr::BOTTOM ||
>> alias_type->adr_type() == TypeOopPtr::BOTTOM ||
>>  >            alias_type->field() != NULL || alias_type->element() !=
>> NULL, "field, array element or unknown");
>>  >     bool mismatched = false;
>>  >     if (alias_type->element() != NULL || alias_type->field() != NULL) {
>>  >
>>  > alias_type->adr_type() == TypeRawPtr::BOTTOM covers the is_native_ptr
>> case and the case where the unsafe method is called with a null object.
>>
>>> jdk9 changeset:
>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/9108fab781a4
>>>
>>> jdk9 bug link: https://bugs.openjdk.java.net/browse/JDK-8134918
>>> Conflict in file src/share/vm/opto/library_call.cpp due to 1.
>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/4bee38ba018c#l5.165
>> [JDK-8140309]. Manual merge is not done as the corresponding code is not
>> there in jdk8u-dev.
>>
>> I explained situation with this line above.
>>
>>> webrev link: http://cr.openjdk.java.net/~shshahma/8134918/webrev.00/
>>
>> This webrev is not incremental for your 8136473 changes - library_call.cpp has
>> part from 8136473 changes.
>>
>>> jdk9 changeset:
>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/79dae2cd00ef
>>>
>>> jdk9 bug link: https://bugs.openjdk.java.net/browse/JDK-8155781
>>> Clean merge
>>> webrev link: http://cr.openjdk.java.net/~shshahma/8155781/webrev.00/
>>
>> Thanks seems fine.
>>
>>> jdk9 changeset:
>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/cde17b3e2e70
>>>
>>> jdk9 bug link: https://bugs.openjdk.java.net/browse/JDK-8162101
>>> Conflict in file src/share/vm/opto/library_call.cpp due to 1.
>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/4be0cada20ad#l1.7
>>> [JDK-8160360] - Resolved 2.
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/8b9fdaeb8c57#l10.273
>> [JDK-8148146] - Manual merge is not done as the corresponding code is not
>> there in jdk8u-dev.
>>> webrev link: http://cr.openjdk.java.net/~shshahma/8162101/webrev.00/
>>
>> This webrev is not incremental in library_call.cpp. Difficult to see this part of
>> changes.
>>
>> Thanks,
>> Vladimir
>>
>>> jdk9 changeset:
>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/10dad1d40843
>>>
>>> Testing: jprt and jtreg
>>>
>>> Regards,
>>> Shafi
>>>
>>>> -----Original Message-----
>>>> From: Shafi Ahmad
>>>> Sent: Thursday, October 20, 2016 10:08 AM
>>>> To: Vladimir Kozlov; hotspot-dev at openjdk.java.net
>>>> Subject: RE: [8u] RFR for JDK-8134918 - C2: Type speculation produces
>>>> mismatched unsafe accesses
>>>>
>>>> Thanks Vladimir.
>>>>
>>>> I will create dependent  backport of
>>>> 1. https://bugs.openjdk.java.net/browse/JDK-8136473
>>>> 2. https://bugs.openjdk.java.net/browse/JDK-8155781
>>>> 3. https://bugs.openjdk.java.net/browse/JDK-8162101
>>>>
>>>> Regards,
>>>> Shafi
>>>>
>>>>> -----Original Message-----
>>>>> From: Vladimir Kozlov
>>>>> Sent: Wednesday, October 19, 2016 8:27 AM
>>>>> To: Shafi Ahmad; hotspot-dev at openjdk.java.net
>>>>> Subject: Re: [8u] RFR for JDK-8134918 - C2: Type speculation
>>>>> produces mismatched unsafe accesses
>>>>>
>>>>> Hi Shafi,
>>>>>
>>>>> You should also consider backporting following related fixes:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8155781
>>>>> https://bugs.openjdk.java.net/browse/JDK-8162101
>>>>>
>>>>> Otherwise you may hit asserts added by 8134918 changes.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 10/17/16 3:12 AM, Shafi Ahmad wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> Please review the backport of JDK-8134918 - C2: Type speculation
>>>>>> produces
>>>>> mismatched unsafe accesses to jdk8u-dev.
>>>>>>
>>>>>> Please note that backport is not clean and the conflict is due to:
>>>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/9108fab781a4#l5.1
>>>>>> 65
>>>>>>
>>>>>>  Getting debug build failure because of:
>>>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/9108fab781a4#l5.1
>>>>>> 55
>>>>>>
>>>>>> The above changes are done under bug# 'JDK-8136473: failed: no
>>>>> mismatched stores, except on raw memory: StoreB StoreI' which is not
>>>>> back ported to jdk8u and the current backport is on top of above
>> change.
>>>>>>
>>>>>>  Please note that I am not sure if there is any dependency between
>>>>>> these
>>>>> two changesets.
>>>>>>
>>>>>> open webrev:
>>>> http://cr.openjdk.java.net/~shshahma/8134918/webrev.00/
>>>>>> jdk9 bug link: https://bugs.openjdk.java.net/browse/JDK-8134918
>>>>>> jdk9 changeset:
>>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/79dae2cd00ef
>>>>>>
>>>>>> testing: Passes JPRT, jtreg not completed
>>>>>>
>>>>>> Regards,
>>>>>> Shafi
>>>>>>