[12] RFR(S) 8215317: [GRAAL] unit test CheckGraalIntrinsics failed after 8213754

[Gustavo Romero]

On 01/17/2019 04:37 PM, Vladimir Kozlov wrote:
> On 1/17/19 6:07 AM, Gustavo Romero wrote:
>> Hi Vladimir,
>>
>> On 01/16/2019 11:40 PM, Vladimir Kozlov wrote:
>>> You should combine both changes and request them at the same time. Changeset will have to list both changes. Originally your changes should include CheckGraalIntrinsics.java fix.
>>
>> Thanks a lot for advising. Just one question: by "Changeset will have to
>> list both changes" you mean the commit title (or body) must say explicitly
>> the change is a combination of 8213754 + 821531?
> 
> Separate lines per bug subject. Here is example:
> 
> http://hg.openjdk.java.net/jdk/jdk/rev/c139884bd80e
> 
> 8213348: jdk.internal.vm.compiler.management service providers missing in module descriptor
> 8211781: re-building fails after changing Graal sources
> Reviewed-by: erikj, mchung

Got it. I'll send the change to jdk-updates for review before
tagging the bugs with "jdk11u-fix-request" so.

Thanks, Vladimir.

Regards,
Gustavo
  
> Vladimir
> 
>>
>>
>>> Note, for 8213754 corresponding Graal test fix is 8215317 (and not 8215687, that one is for 8212043 Math.min/max).
>>
>> Thanks for the detailed note. I commented on the right thread but pasted
>> the wrong bug!
>>
>> Thank you and best regards,
>> Gustavo
>>
>>> Yes, Graal fix in jdk11u is different - new intrinsics should be listed under existing condition isJDK11OrHigher():
>>>
>>> http://hg.openjdk.java.net/jdk-updates/jdk11u/file/5fc74655f16d/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/CheckGraalIntrinsics.java#l371
>>>
>>> Regards,
>>> Vladimir
>>>
>>> On 1/16/19 1:57 PM, Gustavo Romero wrote:
>>>> Hi Vladimir,
>>>>
>>>> I would like to request the approval to backport the change:
>>>>
>>>> 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace
>>>> https://bugs.openjdk.java.net/browse/JDK-8213754
>>>>
>>>> to jdk11u, but if it gets integrated before 8215687 it will break Graal
>>>> test HotspotTest.java/CheckGraalIntrinsics.java again, as expected.
>>>>
>>>> Are you fine if I request the approval to backport first this change, i.e.
>>>> 8215687?
>>>>
>>>> Actually I'll have to tweak a bit and s/isJDK12OrHigher/isJDK11OrHigher/,
>>>> right?
>>>>
>>>> Thank you.
>>>>
>>>> Best regards,
>>>> Gustavo
>>>>
>>>> On 12/13/2018 02:10 AM, Vladimir Kozlov wrote:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8215317
>>>>>
>>>>> JDK-8213754 added new intrinsics which cause Graal's unit test failure.
>>>>>
>>>>> CheckGraalIntrinsics test is adjusted for new intrinsics:
>>>>>
>>>>> src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/CheckGraalIntrinsics.java
>>>>> @@ -376,6 +376,14 @@
>>>>>                               "jdk/jfr/internal/JVM.getEventWriter()Ljava/lang/Object;");
>>>>>           }
>>>>>
>>>>> +        if (isJDK12OrHigher()) {
>>>>> +            add(toBeInvestigated,
>>>>> +                            "java/lang/CharacterDataLatin1.isDigit(I)Z",
>>>>> +                            "java/lang/CharacterDataLatin1.isLowerCase(I)Z",
>>>>> +                            "java/lang/CharacterDataLatin1.isUpperCase(I)Z",
>>>>> +                            "java/lang/CharacterDataLatin1.isWhitespace(I)Z");
>>>>> +        }
>>>>> +
>>>>>           if (!config.inlineNotify()) {
>>>>>               add(ignore, "java/lang/Object.notify()V");
>>>>>           }
>>>>>
>>>>> Tested tier1 and tier3-graal (where test is run).
>>>>>
>>>>> I also pushed changes into Lab's Graal repo so this test will be updated during next sync.
>>>>> But I want to push fix into JDK because JDK 12 will be forked very soon.
>>>>>
>>>>
>>>
>>
>