[8u] RFR for JDK-8134918 - C2: Type speculation produces mismatched unsafe accesses
Shafi Ahmad
shafi.s.ahmad at oracle.com
Tue Nov 15 06:34:42 UTC 2016
Hi Vladimir,
Thanks for the review.
> -----Original Message-----
> From: Vladimir Kozlov
> Sent: Monday, November 14, 2016 11:20 PM
> 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/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 fix of JDK-8136473 caused many problems after integration (see JDK-8140267).
The fix was backed out and re-implemented with JDK-8140309 by slightly changing the assert:
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-November/019696.html
The code change for the fix of JDK-8140309 is code changes for JDK-8136473 by slightly changing one assert.
jdk9 original changeset is http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/4bee38ba018c
As this is a backport so I keep the changes as it is.
>
> The test TestUnsafeUnalignedMismatchedAccesses.java will not work since
> since Unsafe class in jdk8 does not have unaligned methods.
> Hot did you run it?
I am sorry, looks there is some issue with my testing.
I have run jtreg test after merging the changes but somehow the test does not run and I verified only the failing list of jtreg result.
When I run the test case separately it is failing as you already pointed out the same.
$java -jar ~/Tools/jtreg/lib/jtreg.jar -jdk:build/linux-x86_64-normal-server-slowdebug/jdk/ hotspot/test/compiler/intrinsics/unsafe/TestUnsafeUnalignedMismatchedAccesses.java
Test results: failed: 1
Report written to /scratch/shshahma/Java/jdk8u-dev-8140309_01/JTreport/html/report.html
Results written to /scratch/shshahma/Java/jdk8u-dev-8140309_01/JTwork
Error:
/scratch/shshahma/Java/jdk8u-dev-8140309_01/hotspot/test/compiler/intrinsics/unsafe/TestUnsafeUnalignedMismatchedAccesses.java:92: error: cannot find symbol
UNSAFE.putIntUnaligned(array, UNSAFE.ARRAY_BYTE_BASE_OFFSET+1, -1);
Not sure if we should push without the test case.
>
> > http://cr.openjdk.java.net/~shshahma/8134918/webrev.01/
>
> Good. Did you run new UnsafeAccess.java test?
Due to same process issue the test case is not run and when I run it separately it fails.
It passes after doing below changes:
1. Added /othervm
2. replaced import statement 'import jdk.internal.misc.Unsafe;' by 'import sun.misc.Unsafe;'
Updated webrev: http://cr.openjdk.java.net/~shshahma/8134918/webrev.02/
>
> > http://cr.openjdk.java.net/~shshahma/8162101/webrev.01/
I am getting the similar compilation error as above for added test case. Not sure if we can push without the test case.
Regards,
Shafi
>
> Good.
>
> Thanks,
> Vladimir
>
> >
> > Regards,
> > Shafi
> >
> >
> >
> >> -----Original Message-----
> >> From: Vladimir Kozlov
> >> Sent: Friday, November 11, 2016 1:26 AM
> >> To: Shafi Ahmad; HYPERLINK "mailto:hotspot-dev at openjdk.java.net"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; HYPERLINK "mailto:hotspot-dev at openjdk.java.net"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; HYPERLINK "mailto:hotspot-dev at openjdk.java.net"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
> >>>>>>
More information about the hotspot-dev
mailing list