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

[Shafi Ahmad]

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/
http://cr.openjdk.java.net/~shshahma/8134918/webrev.01/
http://cr.openjdk.java.net/~shshahma/8162101/webrev.01/

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
> >>>>