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

Tue Nov 15 19:29:51 UTC 2016

Hi Shafi

You should not backport tests which use only new JDK 9 APIs. Like TestUnsafeUnalignedMismatchedAccesses.java test.

But it is perfectly fine to modify backport by removing part of changes which use a new API. For example,  8162101 changes in OpaqueAccesses.java test which use getIntUnaligned() method.

It is unfortunate that 8140309 changes include also code which process new Unsafe Unaligned intrinsics from JDK 9. It should not be backported but it will simplify this and following backports. So I 
agree with changes you did for 8140309 backport.

Thanks,
Vladimir

On 11/14/16 10:34 PM, Shafi Ahmad wrote:
> 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;hotspot-dev at openjdk.java.net <mailto: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 <mailto: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 <mailto: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
>
>> >>>>>>
>