RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace

Gustavo Romero gromero at linux.vnet.ibm.com
Wed Dec 12 02:07:59 UTC 2018


Hi Michi,

On 12/11/2018 11:12 PM, Michihiro Horie wrote:
> Thank you for finding the issue on Power8. You do not need a check with has_darn in the ppc.ad. It is better to add a check in vm_versoin_ppc.

I agree.

> I uploaded webrev.08 based on your webrev.07. (Thanks for the enhancement of opto assembly and removing trailing spaces!)
> http://cr.openjdk.java.net/~mhorie/8213754/webrev.08/ <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.08/>

Thanks for the updated webrev. Looks good!
I've just pushed webrev.08 to jdk/submit expecting no failures as .07 passed fine.

Once I get the jdk/submit results tomorrow I'll push.

Best regards,
Gustavo

> 
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
> 
> Inactive hide details for "Gustavo Romero" ---2018/12/12 07:57:21---From: "Gustavo Romero" <gromero at linux.vnet.ibm.com> To: "Do"Gustavo Romero" ---2018/12/12 07:57:21---From: "Gustavo Romero" <gromero at linux.vnet.ibm.com> To: "Doerr, Martin" <martin.doerr at sap.com>, Michihiro Horie/Japan/IBM at IBMJP
> 
> From: "Gustavo Romero" <gromero at linux.vnet.ibm.com>
> To: "Doerr, Martin" <martin.doerr at sap.com>, Michihiro Horie/Japan/IBM at IBMJP
> Cc: "core-libs-dev at openjdk.java.net" <core-libs-dev at openjdk.java.net>, "hotspot-compiler-dev at openjdk.java.net" <hotspot-compiler-dev at openjdk.java.net>, "Roger Riggs" <Roger.Riggs at oracle.com>, "Vladimir Kozlov" <vladimir.kozlov at oracle.com>, "Simonis, Volker" <volker.simonis at sap.com>
> Date: 2018/12/12 07:57
> Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace
> 
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> 
> 
> 
> Hi Michi and Martin,
> 
> On 12/11/2018 01:30 PM, Doerr, Martin wrote:
>  > thanks for updating. I think it can get shipped when Gustavo is fine with it and jdk/submit tests have passed.
> 
> webrev.06 removed has_darn from match_rule_supported and the JVM now crashes
> with SIGILL (cmprb) on CPUs <= POWER8. I think that what we want is both
> UseCharacterCompareIntrinsics and has_darn in the predicate, so:
> 
> diff -r 01b519fcb8a8 -r f3bd7a422a6c src/hotspot/cpu/ppc/ppc.ad
> --- a/src/hotspot/cpu/ppc/ppc.ad        Tue Dec 11 11:01:02 2018 -0500
> +++ b/src/hotspot/cpu/ppc/ppc.ad        Tue Dec 11 12:29:41 2018 -0500
> @@ -2257,6 +2257,11 @@
>       return SuperwordUseVSX;
>     case Op_PopCountVI:
>       return (SuperwordUseVSX && UsePopCountInstruction);
> +  case Op_Digit:
> +  case Op_LowerCase:
> +  case Op_UpperCase:
> +  case Op_Whitespace:
> +    return (UseCharacterCompareIntrinsics && VM_Version::has_darn());
>     }
> 
> Martin, is what you meant on your last comment about it?
> 
> I tested the change on POWER9 and it looks good (from webrev.06 and also with
> webrev.06 + the fix above)!
> 
> I think that the Opto assembly could be improved a little bit around 'done:'
> label, like:
> 
> diff -r 89aab62d10cd src/hotspot/cpu/ppc/ppc.ad
> --- a/src/hotspot/cpu/ppc/ppc.ad        Tue Dec 11 12:29:41 2018 -0500
> +++ b/src/hotspot/cpu/ppc/ppc.ad        Tue Dec 11 15:55:34 2018 -0500
> @@ -12440,7 +12440,8 @@
>               "LIS     $src2, (signed short)0xAAB5\n\t"
>               "ORI     $src2, $src2, 0xBABA\n\t"
>               "INSRDI  $src2, $src2, 32, 0\n\t"
> -            "CMPEQB  $crx, 1, $src1, $src2\n\t"
> +            "CMPEQB  $crx, 1, $src1, $src2\n"
> +            "done:\n\t"
>               "SETB    $dst, $crx" %}
> 
>     size(48);
> @@ -12484,7 +12485,8 @@
>               "BGT     $crx, done\n\t"
>               "LIS     $src2, (signed short)0xD6C0\n\t"
>               "ORI     $src2, $src2, 0xDED8\n\t"
> -            "CMPRB   $crx, 1, $src1, $src2\n\t"
> +            "CMPRB   $crx, 1, $src1, $src2\n"
> +            "done:\n\t"
>               "SETB    $dst, $crx" %}
> 
> so the output would be:
> 
> 024   B2: #     B5 B3 <- B1  Freq: 1
> 024     LI      R14, 0x5A41
>          CMPRB   CCR6, 0, R3, R14
>          BGT     CCR6, done
>          LIS     R14, (signed short)0xD6C0
>          ORI     R14, R14, 0xDED8
>          CMPRB   CCR6, 1, R3, R14
> done:
>          SETB    R15, CCR6
> 040     CMPWI   CCR6, R15, #0
> 044     Beq     CCR6, B5  P=0.000000 C=5377.000000
> 
> instead of:
> 
> 024   B2: #     B5 B3 <- B1  Freq: 1
> 024     LI      R14, 0x5A41
>          CMPRB   CCR6, 0, R3, R14
>          BGT     CCR6, done
>          LIS     R14, (signed short)0xD6C0
>          ORI     R14, R14, 0xDED8
>          CMPRB   CCR6, 1, R3, R14
>          SETB    R15, CCR6
> 040     CMPWI   CCR6, R15, #0
> 044     Beq     CCR6, B5  P=0.000000 C=5379.000000
> 
> If nobody opposes to that tiny enhancement I would like to keep it.
> 
> ... and a nit: several trailing spaces here and there :)
> 
> I generated a webrev with has_darn in match_rule_supported, with the change to
> the Opto asm, and with the trailing spaces removed. I uploaded it to:
> 
> http://cr.openjdk.java.net/~gromero/8213754/webrev.07/ <http://cr.openjdk.java.net/%7Egromero/8213754/webrev.07/>
> 
> I pushed that same version to jdk/submit [1] not expecting any failure and once
> I get the fine results and if you, Martin are ok with the version in webrev.07
> I'll push it to jdk/jdk.
> 
> I'll update this thread a soon as I get the results back from jdk/submit repo.
> I think we have time until Thurday 16:00 UTC, even taking into account the
> 3+ different timezones working on that change :)
> 
> Thank you and best regards,
> Gustavo
> 
> [1] http://mail.openjdk.java.net/pipermail/jdk-submit-changes/2018-December/004418.html
> 
>  > Note that changes pushed before Thursday 16:00 UTC will go into jdk12.
>  >
>  > Best regards,
>  >
>  > Martin
>  >
>  > *From:*Michihiro Horie <HORIE at jp.ibm.com>
>  > *Sent:* Dienstag, 11. Dezember 2018 14:08
>  > *To:* Doerr, Martin <martin.doerr at sap.com>
>  > *Cc:* core-libs-dev at openjdk.java.net; Gustavo Romero <gromero at linux.vnet.ibm.com>; hotspot-compiler-dev at openjdk.java.net; Roger Riggs <Roger.Riggs at oracle.com>; Vladimir Kozlov <vladimir.kozlov at oracle.com>; Simonis, Volker <volker.simonis at sap.com>
>  > *Subject:* RE: RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace
>  >
>  > Hi Martin,
>  >
>  > Thank you so much for your review and pointing out the issue on flag enablement on PPC64.
>  >
>  > Latest webrev enables the flag on PPC64 using has_darn in vm_version_ppc and it is used in match_rule_supported in the ad file.
>  > http://cr.openjdk.java.net/~mhorie/8213754/webrev.06/ <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.06/> <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.06/>
>  >
>  >
>  > Best regards,
>  > --
>  > Michihiro,
>  > IBM Research - Tokyo
>  >
>  > Inactive hide details for "Doerr, Martin" ---2018/12/11 20:31:40---Hi Michihiro, the shared code looks good to me, now."Doerr, Martin" ---2018/12/11 20:31:40---Hi Michihiro, the shared code looks good to me, now.
>  >
>  > From: "Doerr, Martin" <martin.doerr at sap.com <mailto:martin.doerr at sap.com>>
>  > To: Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>, Michihiro Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>, Gustavo Romero <gromero at linux.vnet.ibm.com <mailto:gromero at linux.vnet.ibm.com>>
>  > Cc: "core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>" <core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>>, "hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>" <hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>>, Roger Riggs <Roger.Riggs at oracle.com <mailto:Roger.Riggs at oracle.com>>, "Simonis, Volker" <volker.simonis at sap.com <mailto:volker.simonis at sap.com>>
>  > Date: 2018/12/11 20:31
>  > Subject: RE: RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace
>  >
>  > ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>  >
>  >
>  >
>  >
>  > Hi Michihiro,
>  >
>  > the shared code looks good to me, now.
>  >
>  > Looks like the PPC64 is not consistent any more.
>  > Where do you enable UseCharacterCompareIntrinsics on PPC64?
>  > Why aren't you using it for match_rule_supported in the ad file?
>  > I think has_darn could be used to enable it in vm_version_ppc.
>  >
>  > Best regards,
>  > Martin
>  >
>  >
>  > -----Original Message-----
>  > From: Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>
>  > Sent: Montag, 10. Dezember 2018 20:33
>  > To: Michihiro Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>; Gustavo Romero <gromero at linux.vnet.ibm.com <mailto:gromero at linux.vnet.ibm.com>>
>  > Cc: core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>; hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>; Doerr, Martin <martin.doerr at sap.com <mailto:martin.doerr at sap.com>>; Roger Riggs <Roger.Riggs at oracle.com <mailto:Roger.Riggs at oracle.com>>; Simonis, Volker <volker.simonis at sap.com <mailto:volker.simonis at sap.com>>
>  > Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace
>  >
>  > On 12/9/18 5:28 PM, Michihiro Horie wrote:
>  >> Hi Vladimir,
>  >>
>  >> Thanks a lot for your review. I also fixed the copyright in intrinsicnode.hpp.
>  >>
>  >>  >Did you look on code generated by C2 with your latest changes?
>  >> Yes,I usually check generated code with Oprofile and they were as expected like:
>  >> :
>  >> 90 0.0905 : 3fff64c27654: rlwinm r12,r9,24,8,31
>  >> 12 0.0121 : 3fff64c27658: li r11,14640
>  >> 15 0.0151 : 3fff64c2765c: cmprb cr5,0,r31,r11
>  >> 5527 5.5587 : 3fff64c27660: setb r11,cr5
>  >> 36010 36.2164 : 3fff64c27664: stb r11,16(r15)
>  >
>  > Good.
>  >
>  >> :
>  >>
>  >> I found a CSR FAQ that mentions adding a diagnostic flag do not need CSR process. This time I do not need CSR.
>  >> https://wiki.openjdk.java.net/display/csr/CSR+FAQs
>  >
>  > Thank is correct.
>  >
>  >>
>  >> Hi Gustavo,
>  >> Could I ask you to sponsor the latest change webrev.05? I would like you to test it on your P9 node too.
>  >
>  > Thanks,
>  > Vladimir
>  >
>  >>
>  >>
>  >> Best regards,
>  >> --
>  >> Michihiro,
>  >> IBM Research - Tokyo
>  >>
>  >> Inactive hide details for Vladimir Kozlov ---2018/12/08 03:48:04---From: Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>
>  >> To: RogerVladimir Kozlov ---2018/12/08 03:48:04---From: Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> To: Roger Riggs
>  >> <Roger.Riggs at oracle.com <mailto:Roger.Riggs at oracle.com>>, Michihiro Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>
>  >>
>  >> From: Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>
>  >> To: Roger Riggs <Roger.Riggs at oracle.com <mailto:Roger.Riggs at oracle.com>>, Michihiro Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>
>  >> Cc: core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>, hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>, martin.doerr at sap.com <mailto:martin.doerr at sap.com>, volker.simonis at sap.com <mailto:volker.simonis at sap.com>
>  >> Date: 2018/12/08 03:48
>  >> Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>
>  >> ------------------------------------------------------------------------------------------------------------------------
>  >>
>  >>
>  >>
>  >> Hi Michihiro,
>  >>
>  >> Hotspot changes looks fine.
>  >> Did you look on code generated by C2 with your latest changes?
>  >>
>  >> And Copyright year change in intrinsicnode.hpp is incorrect:
>  >>
>  >> - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
>  >> + * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
>  >>
>  >> Should be
>  >>
>  >> * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
>  >>
>  >>
>  >> Thanks,
>  >> Vladimir
>  >>
>  >> On 12/7/18 10:08 AM, Roger Riggs wrote:
>  >>  > Hi Michihiro,
>  >>  >
>  >>  > Looks fine with the updates.
>  >>  >
>  >>  > Its great that the change to isDigit is beneficial on other platforms too.
>  >>  >
>  >>  > A very small editorial:
>  >>  >    There should be a comma  "," after the 2018 in the copyright of:
>  >>  >    test/micro/org/openjdk/bench/java/lang/Characters.java
>  >>  >
>  >>  > Thanks, Roger
>  >>  >
>  >>  >
>  >>  > On 12/07/2018 03:13 AM, Michihiro Horie wrote:
>  >>  >>
>  >>  >> Hi Roger,
>  >>  >>
>  >>  >> I updated my webrev.
>  >>  >> http://cr.openjdk.java.net/~mhorie/8213754/webrev.04/ <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.04/> <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.04/> <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.04/>
>  >>  >> <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.04/>
>  >>  >>
>  >>  >> >0x80 might be a good choice or 0xff.
>  >>  >> I agree,thanks.
>  >>  >>
>  >>  >> >I was wondering about the performance without the PPC specific intrinsic but with the
>  >>  >> >CharacterDataLatin1.java.template code for isDigit being:
>  >>  >> >        return '0' <= ch && ch <= '9';
>  >>  >> I now understand your point,thanks for your explanation. Both on Power9 and Skylake, the
>  >>  >> isDigit(ch)using ‘0’…’9’explicitly in CharacterDataLatin1.java.template without PPC specific
>  >>  >> intrinsicwas around 15% faster than the original code that does not include my changes. I fixed my
>  >>  >> change using ‘0’…’9’for this isDigit(ch).
>  >>  >>
>  >>  >>
>  >>  >> Best regards,
>  >>  >> --
>  >>  >> Michihiro,
>  >>  >> IBM Research - Tokyo
>  >>  >>
>  >>  >> Inactive hide details for Roger Riggs ---2018/12/07 01:23:39---Hi Michihiro, On 12/06/2018 02:38
>  >>  >> AM, Michihiro Horie wrote:Roger Riggs ---2018/12/07 01:23:39---Hi Michihiro, On 12/06/2018 02:38
>  >>  >> AM, Michihiro Horie wrote:
>  >>  >>
>  >>  >> From: Roger Riggs <Roger.Riggs at oracle.com <mailto:Roger.Riggs at oracle.com>>
>  >>  >> To: Michihiro Horie <HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>
>  >>  >> Cc: core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>, hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>, martin.doerr at sap.com <mailto:martin.doerr at sap.com>,
>  >>  >> vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>, volker.simonis at sap.com <mailto:volker.simonis at sap.com>
>  >>  >> Date: 2018/12/07 01:23
>  >>  >> Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>
>  >>  >> ----------------------------------------------------------------------------------------------------
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >> Hi Michihiro,
>  >>  >>
>  >>  >> On 12/06/2018 02:38 AM, Michihiro Horie wrote:
>  >>  >>
>  >>  >>         Hi Roger,
>  >>  >>
>  >>  >>         Thank you for your helpful comments. I updated new webrev, adding a parameter in the jmh
>  >>  >>         test to measure ‘other’ characters and moving the file to an appropriate location, also
>  >>  >>         fixing the indents and copyright year._
>  >>  >>
>  >> __https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Emhorie_8213754_webrev.03_-5F&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=sNklBozv1ZztDu7rnM5SUgqTwPn4CPlp4Gp0uGwfVhs&s=aKLGn7JZawWsl9UR7H-PyYoTpBc23BAKMqGScywbC5U&e=
>  >>  >>         <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.03/>
>  >>  >>
>  >>  >> The choice of 256 for other is outside the range of Latin1 which is 0..255.
>  >>  >> 0x80 might be a good choice or 0xff.
>  >>  >>
>  >>  >>
>  >>  >>         >Is there an opportunity to improve the performance of isDigit using explicit '0'.. '9'
>  >>  >>         >in CharacterDataLatin1.java.template?  The performance would need to be measured and
>  >>  >>         compared.
>  >>  >>         Yes, there is an opportunity: intrinsic performed 25% better than the original on Power9.
>  >>  >>
>  >>  >> I was wondering about the performance without the PPC specific intrinsic but with the
>  >>  >> CharacterDataLatin1.java.template code for isDigit being:
>  >>  >>         return '0' <= ch && ch <= '9';
>  >>  >>
>  >>  >>
>  >>  >>         >Is the profile of in-lining methods changed in any measurable way now that
>  >>  >>         >there is an extra level of method invocation?
>  >>  >>         >    Character.isLowerCase(ch) -> CharacterData.isLowerCase(ch) -> getType(ch) ==
>  >>  >>         LOWERCASE_LETTER;
>  >>  >>         >
>  >>  >>         >instead of:
>  >>  >>         >    Character.isLowerCase(ch) -> getType(ch) == LOWERCASE_LETTER;
>  >>  >>         I missed this point, thanks. I tried jstack but could not find additional level.
>  >>  >>
>  >>  >>         LogCompilation shows CharacterDataLatin1.isLowerCase(ch) is compiled in c1:
>  >>  >>         <nmethod compile_id='71' compiler='c1' level='3' entry='0x00003fff5911cd00' size='1472'
>  >>  >>         address='0x00003fff5911cb90' relocation_offset='344' insts_offset='368' stub_offset='1136'
>  >>  >>         scopes_data_offset='1248' scopes_pcs_offset='1336' dependencies_offset='1448'
>  >>  >>         nul_chk_table_offset='1456' oops_offset='1184' metadata_offset='1192'
>  >>  >>         method='java.lang.CharacterDataLatin1 isLowerCase (I)Z' bytes='15' count='1024'
>  >>  >>         iicount='1025' stamp='0.058'/>
>  >>  >>
>  >>  >> Supposing some existing complex code was already taking advantage of the full allowed inline depth.
>  >>  >> Then adding an extra level might change the inlining behavior.
>  >>  >>
>  >>  >>
>  >>  >>         Existing methods in CharacterDataLatin1.java.template etc. directly invoke
>  >>  >>         getProperties(ch), so isLowerCase(ch) would be more aligned with other methods if it looks
>  >>  >>         like as follows?
>  >>  >>         + @HotSpotIntrinsicCandidate
>  >>  >>         + boolean isLowerCase(int ch) {
>  >>  >>         + int props = getProperties(ch);
>  >>  >>         + return (props & $$maskType) == Character.LOWERCASE_LETTER;
>  >>  >>         + }
>  >>  >>
>  >>  >> Yes, that would alleviate my concern.
>  >>  >>
>  >>  >> Thanks, Roger
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >>         Best regards,
>  >>  >>         --
>  >>  >>         Michihiro,
>  >>  >>         IBM Research - Tokyo
>  >>  >>
>  >>  >>         Inactive hide details for Roger Riggs ---2018/12/06 05:09:36---Hi Michihiro, On 12/05/2018
>  >>  >>         07:21 AM, Michihiro Horie wrote:Roger Riggs ---2018/12/06 05:09:36---Hi Michihiro, On
>  >>  >>         12/05/2018 07:21 AM, Michihiro Horie wrote:
>  >>  >>
>  >>  >>         From: Roger Riggs _<Roger.Riggs at oracle.com <mailto:Roger.Riggs at oracle.com>>_ <mailto:Roger.Riggs at oracle.com>
>  >>  >>         To: Michihiro Horie _<HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>_ <mailto:HORIE at jp.ibm.com>,
>  >>  >>         _vladimir.kozlov at oracle.com_ <mailto:_vladimir.kozlov at oracle.com_><mailto:vladimir.kozlov at oracle.com>
>  >>  >>         Cc: _core-libs-dev at openjdk.java.net_ <mailto:_core-libs-dev at openjdk.java.net_><mailto:core-libs-dev at openjdk.java.net>,
>  >>  >>         _hotspot-compiler-dev at openjdk.java.net_ <mailto:_hotspot-compiler-dev at openjdk.java.net_><mailto:hotspot-compiler-dev at openjdk.java.net>,
>  >>  >>         _martin.doerr at sap.com_ <mailto:_martin.doerr at sap.com_><mailto:martin.doerr at sap.com>, _volker.simonis at sap.com_ <mailto:_volker.simonis at sap.com_>
>  >>  >>         <mailto:volker.simonis at sap.com>
>  >>  >>         Date: 2018/12/06 05:09
>  >>  >>         Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for
>  >>  >>         isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>         ----------------------------------------------------------------------------------------------------
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >>         Hi Michihiro,
>  >>  >>
>  >>  >>         On 12/05/2018 07:21 AM, Michihiro Horie wrote:
>  >>  >>                         >There are a few JMH tests for upper and lower in the
>  >>  >>                         jmh-jdk-microbenchmarks repo. [1]
>  >>  >>                         Here is a jmh webrev for the Character methods._
>  >>  >>
>  >> __https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Emhorie_8213754_jmh-2Dwebrev.00_-5F&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=sNklBozv1ZztDu7rnM5SUgqTwPn4CPlp4Gp0uGwfVhs&s=v22au5r5gvv48ufyda1poelXBjJWwuotSFrv-M2AlRY&e=
>  >>  >>                         <http://cr.openjdk.java.net/%7Emhorie/8213754/jmh-webrev.00/> Please include at least one
>  >> character value that is not a member of any of the classes.
>  >>  >>         The performance impact for 'other' characters is important also.
>  >>  >>
>  >>  >>         The JMH tests have been moved to the OpenJDK jdk/jdk repo in the directory:
>  >>  >>            test/micro/org/openjdk/bench/java/lang/
>  >>  >>
>  >>  >>         Now that they are in that repo, they can be included with the rest of the changeset.
>  >>  >>                         Also, I updated C2 changes in the latest webrev. (Thank you for giving
>  >>  >>                         valuable comments off-list, Vladimir and Martin!)
>  >>  >>                         With the change in is_disabled_by_flags, I added a JVM flag that will need
>  >>  >>                         another review request. I would proceed for it if this RFR is accepted._
>  >>  >>
>  >> __https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Emhorie_8213754_webrev.02_-5F&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=sNklBozv1ZztDu7rnM5SUgqTwPn4CPlp4Gp0uGwfVhs&s=UXQByb5dFxfAwCCppkizqG_-RWf6DhE_PFkr9TsyzKo&e=
>  >>  >>                         <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.02/> The indent in the Java code should
>  >> be 4 spaces. (Native lib code is 4 spaces, Hotspot is 2
>  >>  >>         spaces)
>  >>  >>
>  >>  >>         Please update the copyrights to include 2018.
>  >>  >>
>  >>  >>         Is there an opportunity to improve the performance of isDigit using explicit '0'.. '9'
>  >>  >>         in CharacterDataLatin1.java.template?  The performance would need to be measured and compared.
>  >>  >>
>  >>  >>         Is the profile of in-lining methods changed in any measurable way now that
>  >>  >>         there is an extra level of method invocation?
>  >>  >>             Character.isLowerCase(ch) -> CharacterData.isLowerCase(ch) -> getType(ch) ==
>  >>  >>         LOWERCASE_LETTER;
>  >>  >>
>  >>  >>         instead of:
>  >>  >>             Character.isLowerCase(ch) -> getType(ch) == LOWERCASE_LETTER;
>  >>  >>
>  >>  >>         Regards, Roger
>  >>  >>                         I need a review on changes in the class library, anyway. Would be grateful
>  >>  >>                         if I could have one.
>  >>  >>
>  >>  >>
>  >>  >>                         Best regards,
>  >>  >>                         --
>  >>  >>                         Michihiro,
>  >>  >>                         IBM Research - Tokyo
>  >>  >>
>  >>  >>
>  >>  >>                         ----- Original message -----
>  >>  >>                         From: Michihiro Horie/Japan/IBM
>  >>  >>                         To: Vladimir Kozlov _<vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>_
>  >>  >>                         <mailto:vladimir.kozlov at oracle.com>
>  >>  >>                         Cc: core-libs-dev _<core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>>_
>  >>  >>                         <mailto:core-libs-dev at openjdk.java.net>,
>  >>  >>                         _hotspot-compiler-dev at openjdk.java.net_ <mailto:_hotspot-compiler-dev at openjdk.java.net_>
>  >>  >>                         <mailto:hotspot-compiler-dev at openjdk.java.net>, _martin.doerr at sap.com_ <mailto:_martin.doerr at sap.com_>
>  >>  >>                         <mailto:martin.doerr at sap.com>, Roger Riggs _<roger.riggs at oracle.com <mailto:roger.riggs at oracle.com>&g t;_
>  >>  >>                         <mailto:roger.riggs at oracle.com>, _volker.simonis at sap.com_ <mailto:_volker.simonis at sap.com_>
>  >>  >>                         <mailto:volker.simonis at sap.com>
>  >>  >>                         Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>                         Date: Fri, Nov 30, 2018 1:01 PM
>  >>  >>
>  >>  >>                         Hi Vladimir,
>  >>  >>
>  >>  >>                         Thank you for your further comments. I fixed my code, but I’m afraid
>  >>  >>                         discussing such a basic topic involving the two big mailing lists is not
>  >>  >>                         good. Please let me have a chance to talk on this off-list.
>  >>  >>
>  >>  >>
>  >>  >>                         Best regards,
>  >>  >>                         --
>  >>  >>                         Michihiro,
>  >>  >>                         IBM Research - Tokyo
>  >>  >>
>  >>  >>                         Vladimir Kozlov ---2018/11/30 03:01:42---Hi Michihiro, I still do not
>  >>  >>                         understand which Region node you are swapping. Where it is coming from?
>  >>  >>
>  >>  >>                         From: Vladimir Kozlov _<vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>_
>  >>  >>                         <mailto:vladimir.kozlov at oracle.com>
>  >>  >>                         To: Michihiro Horie _<HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>_ <mailto:HORIE at jp.ibm.com>, Roger
>  >>  >>                         Riggs _<roger.riggs at oracle.com <mailto:roger.riggs at oracle.com>>_ <mailto:roger.riggs at oracle.com>
>  >>  >>                         Cc: core-libs-dev _<core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>>_
>  >>  >>                         <mailto:core-libs-dev at openjdk.java.net>,
>  >>  >>                         _hotspot-compiler-dev at openjdk.java.net_ <mailto:_hotspot-compiler-dev at openjdk.java.net_>
>  >>  >>                         <mailto:hotspot-compiler-dev at openjdk.java.net>, _martin.doerr at sap.com_ <mailto:_martin.doerr at sap.com_>
>  >>  >>                         <mailto:martin.doerr at sap.com>, _volker.simonis at sap.com_ <mailto:_volker.simonis at sap.com_>
>  >>  >>                         <mailto:volker.simonis at sap.com>
>  >>  >>                         Date: 2018/11/30 03:01
>  >>  >>                         Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>
>  >> ----------------------------------------------------------------------------------------------------
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >>                         Hi Michihiro,
>  >>  >>
>  >>  >>                         I still do not understand which Region node you are swapping. Where it is
>  >>  >>                         coming from?
>  >>  >>
>  >>  >>                         > + // Swap current RegionNode with new one
>  >>  >>
>  >>  >>                         Regards,
>  >>  >>                         Vladimir
>  >>  >>
>  >>  >>                         On 11/28/18 10:31 PM, Michihiro Horie wrote:
>  >>  >>                         > Vladimir,Roger,
>  >>  >>                         > Thank you so much for your responses.
>  >>  >>                         >
>  >>  >>                         > Vladimir,
>  >>  >>                         > Regarding the hotspot change,I’m new in changing around
>  >>  >>                         library_call.cpp,so your comments are very helpful. I will fix
>  >>  >>                         > my code,especially inline_character_compare()would be like:
>  >>  >>                         >
>  >>  >>                         > +bool LibraryCallKit::inline_character_compare(vmIntrinsics::ID id){
>  >>  >>                         > + RegionNode* old_rgn = control()->as_Region();
>  >>  >>                         > + RegionNode* new_rgn = new RegionNode(1);
>  >>  >>                         > + record_for_igvn(new_rgn);
>  >>  >>                         > +
>  >>  >>                         > + // Swap current RegionNode with new one
>  >>  >>                         > + Node* new_ctrl = old_rgn->in(1);
>  >>  >>                         > + old_rgn->del_req(1);
>  >>  >>                         > + new_rgn->add_req(new_ctrl);
>  >>  >>                         > +
>  >>  >>                         > + set_control(_gvn.transform(new_rgn));
>  >>  >>                         > +
>  >>  >>                         > + // argument(0)is receiver
>  >>  >>                         > + Node* codePoint = argument(1);
>  >>  >>                         > + Node* n = NULL;
>  >>  >>                         > +
>  >>  >>                         > + switch (id){
>  >>  >>                         > + case vmIntrinsics::_isDigit_c : n = new
>  >>  >>                         DigitCNode(control(),codePoint);break;
>  >>  >>                         > + case vmIntrinsics::_isLowerCase_c : n = new
>  >>  >>                         LowerCaseCNode(control(),codePoint);break;
>  >>  >>                         > + case vmIntrinsics::_isUpperCase_c : n = new
>  >>  >>                         UpperCaseCNode(control(),codePoint);break;
>  >>  >>                         > + case vmIntrinsics::_isWhitespace_c : n = new
>  >>  >>                         WhitespaceCNode(control(),codePoint);break;
>  >>  >>                         > + default:
>  >>  >>                         > + fatal_unexpected_iid(id);
>  >>  >>                         > + }
>  >>  >>                         > +
>  >>  >>                         > + set_result(_gvn.transform(n));
>  >>  >>                         > + return true;
>  >>  >>                         > +}
>  >>  >>                         >
>  >>  >>                         > Microbenchmark showed ~40% improvements.
>  >>  >>                         >
>  >>  >>                         > Roger,
>  >>  >>                         > I would wait foryour review,thanks. I understood the discussion had not
>  >>  >>                         been recognized from people in core-libs-dev,and
>  >>  >>                         > checked it was not actually archived there….
>  >>  >>                         >
>  >>  >>                         > Best regards,
>  >>  >>                         > --
>  >>  >>                         > Michihiro,
>  >>  >>                         > IBM Research - Tokyo
>  >>  >>                         >
>  >>  >>                         > Inactive hide details for Roger Riggs ---2018/11/29 11:21:26---Hi, I'll
>  >>  >>                         look at it on Thursday.Roger Riggs ---2018/11/29
>  >>  >>                         > 11:21:26---Hi, I'll look at it on Thursday.
>  >>  >>                         >
>  >>  >>                         > From: Roger Riggs _<roger.riggs at oracle.com <mailto:roger.riggs at oracle.com>>_ <mailto:roger.riggs at oracle.com>
>  >>  >>                         > To: Vladimir Kozlov _<vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>_
>  >>  >>                         <mailto:vladimir.kozlov at oracle.com>, Michihiro Horie _<HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>_
>  >>  >>                         <mailto:HORIE at jp.ibm.com>, _core-libs-dev at openjdk.java.net_ <mailto:_core-libs-dev at openjdk.java.net_>
>  >>  >>                         <mailto:core-libs-dev at openjdk.java.net>
>  >>  >>                         > Cc: _volker.simonis at sap.com_ <mailto:_volker.simonis at sap.com_><mailto:volker.simonis at sap.com>,
>  >>  >>                         _hotspot-compiler-dev at openjdk.java.net_ <mailto:_hotspot-compiler-dev at openjdk.java.net_>
>  >>  >>                         <mailto:hotspot-compiler-dev at openjdk.java.net>, _martin.doerr at sap.com_ <mailto:_martin.doerr at sap.com_>
>  >>  >>                         <mailto:martin.doerr at sap.com>
>  >>  >>                         > Date: 2018/11/29 11:21
>  >>  >>                         > Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>                         >
>  >>  >>                         >
>  >>  >>
>  >> ------------------------------------------------------------------------------------------------------------------------
>  >>  >>                         >
>  >>  >>                         >
>  >>  >>                         >
>  >>  >>                         > Hi,
>  >>  >>                         >
>  >>  >>                         > I'll look at it on Thursday.
>  >>  >>                         >
>  >>  >>                         > In spite of it looking like the email was sent to core-lib-dev, I have
>  >>  >>                         > not seen it before
>  >>  >>                         > and its not in the core-libs-dev archive.
>  >>  >>                         >
>  >>  >>                         > $.02, Roger
>  >>  >>                         >
>  >>  >>                         > On 11/28/18 7:15 PM, Vladimir Kozlov wrote:
>  >>  >>                         >  > You still need review from core-libs.
>  >>  >>                         >  >
>  >>  >>                         >  > About your hotspot changes. You need to add intrinsics to
>  >>  >>                         >  > is_disabled_by_flags() to be able switch them off.
>  >>  >>                         >  >
>  >>  >>                         >  > Note, match_rule_supported() calls in
>  >>  >>                         >  > C2Compiler::is_intrinsic_supported() executed before intrinsics in C2
>  >>  >>                         >  > are registered. So they will not be available if they are not
>  >>  >>                         >  > supported. match_rule_supported() checks in
>  >>  >>                         >  > LibraryCallKit::inline_character_compare() are not needed.
>  >>  >>                         >  >
>  >>  >>                         >  > I don't get what you code in
>  >>  >>                         >  > LibraryCallKit::inline_character_compare() is doing. Why you check
>  >>  >>                         >  > Region node at the beginning and why you add Region and Phi at the end
>  >>  >>                         >  > if you have only one path?
>  >>  >>                         >  > You are creating code for whole method which should return boolean result
>  >>  >>                         >  >
>  >>  >>                         >  > +    boolean isDigit(int ch) {
>  >>  >>                         >  > +      return getType(ch) == Character.DECIMAL_DIGIT_NUMBER;
>  >>  >>                         >  > +    }
>  >>  >>                         >  >
>  >>  >>                         >  > but your code produce TypeInt::CHAR. why?
>  >>  >>                         >  >
>  >>  >>                         >  > An finally. Did you compare code generated by default and with you
>  >>  >>                         >  > changes? what improvement you see?
>  >>  >>                         >  >
>  >>  >>                         >  > Thanks,
>  >>  >>                         >  > Vladimir
>  >>  >>                         >  >
>  >>  >>                         >  > On 11/28/18 3:07 PM, Michihiro Horie wrote:
>  >>  >>                         >  >> Is there any response from core-libs-dev?
>  >>  >>                         >  >>
>  >>  >>                         >  >> Vladimir,
>  >>  >>                         >  >> Could you give your suggestion on how to proceed?
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> Best regards,
>  >>  >>                         >  >> --
>  >>  >>                         >  >> Michihiro,
>  >>  >>                         >  >> IBM Research - Tokyo
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> ----- Original message -----
>  >>  >>                         >  >> From: "Michihiro Horie" _<HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>_ <mailto:HORIE at jp.ibm.com>
>  >>  >>                         >  >> Sent by: "hotspot-compiler-dev"
>  >>  >>                         >  >> _<hotspot-compiler-dev-bounces at openjdk.java.net <mailto:hotspot-compiler-dev-bounces at openjdk.java.net>>_
>  >>  >>                         <mailto:hotspot-compiler-dev-bounces at openjdk.java.net>
>  >>  >>                         >  >> To: "Doerr, Martin" _<martin.doerr at sap.com <mailto:martin.doerr at sap.com>>_
>  >>  >>                         <mailto:martin.doerr at sap.com>
>  >>  >>                         >  >> Cc: "Simonis, Volker" _<volker.simonis at sap.com <mailto:volker.simonis at sap.com>>_
>  >>  >>                         <mailto:volker.simonis at sap.com>,
>  >>  >>                         >  >> _"hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>"_
>  >>  >>                         <mailto:hotspot-compiler-dev at openjdk.java.net>_<hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>>_
>  >>  >>                         <mailto:hotspot-compiler-dev at openjdk.java.net>,
>  >>  >>                         >  >> _"core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>"_
>  >>  >>                         <mailto:core-libs-dev at openjdk.java.net>_<core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>>_
>  >>  >>                         <mailto:core-libs-dev at openjdk.java.net>
>  >>  >>                         >  >> Subject: RE: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         >  >> isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>                         >  >> Date: Thu, Nov 22, 2018 11:28 AM
>  >>  >>                         >  >>
>  >>  >>                         >  >> Hi Martin,
>  >>  >>                         >  >>
>  >>  >>                         >  >> Yes, we should wait for the feedback on class library change.
>  >>  >>                         >  >>
>  >>  >>                         >  >>  >Btw. I think you can further simplify the code in library_call.cpp
>  >>  >>                         >  >> (no phi). But we can discuss details when thedirection regarding the
>  >>  >>                         >  >> java classes is clear.
>  >>  >>                         >  >> Thank you for pointing out it, I think I understand how to simplify
>  >>  >>                         >  >> code.
>  >>  >>                         >  >>
>  >>  >>                         >  >>  >Hi Michi,
>  >>  >>                         >  >>  >
>  >>  >>                         >  >>  >On 11/20/2018 02:52 PM, Michihiro Horie wrote:
>  >>  >>                         >  >>  >> >Please note that we don’t have a machine, yet. So other people
>  >>  >>                         >  >> will have to test.
>  >>  >>                         >  >>  >> I think Gustavo can help testing this change when its' ready.
>  >>  >>                         >  >>  >Sure :)
>  >>  >>                         >  >>  >
>  >>  >>                         >  >>  >Best regards,
>  >>  >>                         >  >>  >Gustavo
>  >>  >>                         >  >> Thank you, Gustavo.
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> Best regards,
>  >>  >>                         >  >> --
>  >>  >>                         >  >> Michihiro,
>  >>  >>                         >  >> IBM Research - Tokyo
>  >>  >>                         >  >>
>  >>  >>                         >  >> Inactive hide details for "Doerr, Martin" ---2018/11/22 01:33:34---Hi
>  >>  >>                         >  >> Michihiro, thanks. This proposal makes the javacode much"Doerr,
>  >>  >>                         >  >> Martin" ---2018/11/22 01:33:34---Hi Michihiro, thanks. This proposal
>  >>  >>                         >  >> makes the java code much moreintrinsics friendly.
>  >>  >>                         >  >>
>  >>  >>                         >  >> From: "Doerr, Martin" _<martin.doerr at sap.com <mailto:martin.doerr at sap.com>>_
>  >>  >>                         <mailto:martin.doerr at sap.com>
>  >>  >>                         >  >> To: Michihiro Horie _<HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>_ <mailto:HORIE at jp.ibm.com>,
>  >>  >>                         >  >> _"core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>"_
>  >>  >>                         <mailto:core-libs-dev at openjdk.java.net>_<core-libs-dev at openjdk.java.net <mailto:core-libs-dev at openjdk.java.net>>_
>  >>  >>                         <mailto:core-libs-dev at openjdk.java.net>
>  >>  >>                         >  >> Cc: _"hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>"_
>  >>  >>                         <mailto:hotspot-compiler-dev at openjdk.java.net>
>  >>  >>                         >  >> _<hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>>_
>  >>  >>       <mailto:hotspot-compiler-dev at openjdk.java.net>, "Simonis,
>  >>  >>                         >  >> Volker"_<volker.simonis at sap.com <mailto:volker.simonis at sap.com>>_ <mailto:volker.simonis at sap.com>,
>  >>  >>                         Gustavo Romero
>  >>  >>                         >  >> _<gromero at linux.vnet.ibm.com <mailto:gromero at linux.vnet.ibm.com>>_ <mailto:gromero at linux.vnet.ibm.com>
>  >>  >>                         >  >> Date: 2018/11/22 01:33
>  >>  >>                         >  >> Subject: RE: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         >  >> isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >
>  >>  >>
>  >> ------------------------------------------------------------------------------------------------------------------------
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> Hi Michihiro,
>  >>  >>                         >  >>
>  >>  >>                         >  >> thanks. This proposal makes the java code much more intrinsics friendly.
>  >>  >>                         >  >> We should wait for feedback from the core lib folks. Maybe they have
>  >>  >>                         >  >> some requirements or other proposals.
>  >>  >>                         >  >>
>  >>  >>                         >  >> I think these intrinsics should be interesting for Oracle, Intel and
>  >>  >>                         >  >> others, too.
>  >>  >>                         >  >>
>  >>  >>                         >  >> Btw. I think you can further simplify the code in library_call.cpp
>  >>  >>                         >  >> (no phi). But we can discuss details when thedirection regarding the
>  >>  >>                         >  >> java classes is clear.
>  >>  >>                         >  >>
>  >>  >>                         >  >> Best regards,
>  >>  >>                         >  >> Martin
>  >>  >>                         >  >>
>  >>  >>                         >  >> *
>  >>  >>                         >  >> **From:*Michihiro Horie _<HORIE at jp.ibm.com <mailto:HORIE at jp.ibm.com>>_ <mailto:HORIE at jp.ibm.com>*
>  >>  >>                         >  >> **Sent:*Mittwoch, 21. November 2018 17:14*
>  >>  >>                         >  >> **To:*core-libs-dev at openjdk.java.net; Doerr, Martin
>  >>  >>                         >  >> _<martin.doerr at sap.com <mailto:martin.doerr at sap.com>>_ <mailto:martin.doerr at sap.com>*
>  >>  >>                         >  >> **Cc:*hotspot-compiler-dev at openjdk.java.net; Simonis, Volker
>  >>  >>                         >  >> _<volker.simonis at sap.com <mailto:volker.simonis at sap.com>>_ <mailto:volker.simonis at sap.com>; Gustavo
>  >>  >>                         Romero_<gromero at linux.vnet.ibm.com <mailto:gromero at linux.vnet.ibm.com>>_ <mailto:gromero at linux.vnet.ibm.com>*
>  >>  >>                         >  >> **Subject:*RE: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         >  >> isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>                         >  >>
>  >>  >>                         >  >> Hi Martin,
>  >>  >>                         >  >>
>  >>  >>                         >  >> I send this RFR to core-libs-dev too, because it changes the class
>  >>  >>                         >  >> library.
>  >>  >>                         >  >>
>  >>  >>                         >  >> Through trial and error, I separated Latin1 block as in the _
>  >>  >>                         >  >>
>  >>  >>                         >
>  >> ___https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Emhorie_8213754_webrev.01-5F-5F&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=sNklBozv1ZztDu7rnM5SUgqTwPn4CPlp4Gp0uGwfVhs&s=co8xQFD19i2JBuYtSh2KKr-qUmwPdt6MEpJErzBfsd0&e=
>  >>  >>                         <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.01_>
>  >>  >>                         >
>  >>  >>                         >  >>
>  >> <_https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-257Emhorie_8213754_webrev.01-5F&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=sNklBozv1ZztDu7rnM5SUgqTwPn4CPlp4Gp0uGwfVhs&s=w78kALiRtp6DIYAfslpK_TGpubqajF0h32O_z_ITAF4&e=>_/_
>  >>  >>                         >  >>
>  >>  >>                         >  >> I followed the coding way of Character.isWhitespace() that invokes
>  >>  >>                         >  >> each ChracterData’s isWhitespace() to refactorisDigit(),
>  >>  >>                         >  >> isLowerCase(), and isUpperCase().
>  >>  >>                         >  >>
>  >>  >>                         >  >> I think this change is also useful on x86, using STTNI.
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> Best regards,
>  >>  >>                         >  >> --
>  >>  >>                         >  >> Michihiro,
>  >>  >>                         >  >> IBM Research - Tokyo
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> ----- Original message -----
>  >>  >>                         >  >> From: "Michihiro Horie" <_HORIE at jp.ibm.com_ <_mailto:HORIE at jp.ibm.com_ <mailto:_HORIE at jp.ibm.com_%20%3c_mailto:HORIE at jp.ibm.com_>>>
>  >>  >>                         >  >> Sent by: "hotspot-compiler-dev"
>  >>  >>                         >  >>
>  >>  >>
>  >> <_hotspot-compiler-dev-bounces at openjdk.java.net_<_mailto:hotspot-compiler-dev-bounces at openjdk.java.net_ <mailto:_hotspot-compiler-dev-bounces at openjdk.java.net_%3c_mailto:hotspot-compiler-dev-bounces at openjdk.java.net_>>>
>  >>  >>                         >  >> To: "Doerr, Martin" <_martin.doerr at sap.com_
>  > <mailto:_martin.doerr at sap.com_%0b>>  >>                         >  >> <_mailto:martin.doerr at sap.com_>>
>  >>  >>                         >  >> Cc: "Simonis, Volker" <_volker.simonis at sap.com_
>  > <mailto:_volker.simonis at sap.com_%0b>>  >>                         >  >> <_mailto:volker.simonis at sap.com_>>,
>  >>  >>                         >  >>
>  >>  >>                         "_ppc-aix-port-dev at openjdk.java.net_<_mailto:ppc-aix-port-dev at openjdk.java.net_> <mailto:_ppc-aix-port-dev at openjdk.java.net_%3c_mailto:ppc-aix-port-dev at openjdk.java.net_%3e>"
>  >>  >>                         >  >>
>  >>  >>                         <_ppc-aix-port-dev at openjdk.java.net_<_mailto:ppc-aix-port-dev at openjdk.java.net_ <mailto:_ppc-aix-port-dev at openjdk.java.net_%3c_mailto:ppc-aix-port-dev at openjdk.java.net_>>>,
>  >>  >>                         >  >>
>  >>  >>                         "_hotspot-compiler-dev at openjdk.java.net_<_mailto:hotspot-compiler-dev at openjdk.java.net_> <mailto:_hotspot-compiler-dev at openjdk.java.net_%3c_mailto:hotspot-compiler-dev at openjdk.java.net_%3e>"
>  >>  >>                         >  >>
>  >>  >>                         <_hotspot-compiler-dev at openjdk.java.net_<_mailto:hotspot-compiler-dev at openjdk.java.net_ <mailto:_hotspot-compiler-dev at openjdk.java.net_%3c_mailto:hotspot-compiler-dev at openjdk.java.net_>>>
>  >>  >>                         >  >>
>  >>  >>                         >  >> Subject: RE: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         >  >> isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>                         >  >> Date: Wed, Nov 21, 2018 1:53 AM
>  >>  >>                         >  >>
>  >>  >>                         >  >> Hi Martin,
>  >>  >>                         >  >>
>  >>  >>                         >  >> Thank you for giving your helpful comments. I did not recognize
>  >>  >>                         >  >> “generate_method_call_static” prevents anyoptimizations, but I now
>  >>  >>                         >  >> checked it actually degraded the performance, thanks.
>  >>  >>                         >  >>
>  >>  >>                         >  >>  >Please note that we don’t have a machine, yet. So other people will
>  >>  >>                         >  >> have to test.
>  >>  >>                         >  >> I think Gustavo can help testing this change when its' ready.
>  >>  >>                         >  >>
>  >>  >>                         >  >>  >Would it be possible to introduce more fine-grained intrinsics such
>  >>  >>                         >  >> that the “slow” path is outside of them?
>  >>  >>                         >  >>  >
>  >>  >>                         >  >>  >Maybe you can factor out as in the following example?
>  >>  >>                         >  >>  >if (latin1) return isLatin1Digit(codePoint);
>  >>  >>                         >  >>  >with isLatin1Digit as HotSpotIntrinsicCandidate.
>  >>  >>                         >  >> Thanks for an example, please let me try to separate the Latin block
>  >>  >>                         >  >> from other blocks for some time.
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> Best regards,
>  >>  >>                         >  >> --
>  >>  >>                         >  >> Michihiro,
>  >>  >>                         >  >> IBM Research - Tokyo
>  >>  >>                         >  >>
>  >>  >>                         >  >> Inactive hide details for "Doerr, Martin" ---2018/11/20 01:55:27---Hi
>  >>  >>                         >  >> Michihiro, first of all, thanks for working onPower9 opt"Doerr,
>  >>  >>                         >  >> Martin" ---2018/11/20 01:55:27---Hi Michihiro, first of all, thanks
>  >>  >>                         >  >> for working on Power9optimizations. Please note that we don't ha
>  >>  >>                         >  >>
>  >>  >>                         >  >> From: "Doerr, Martin" <_martin.doerr at sap.com_
>  > <mailto:_martin.doerr at sap.com_%0b>>  >>                         >  >> <_mailto:martin.doerr at sap.com_>>
>  >>  >>                         >  >> To: Michihiro Horie <_HORIE at jp.ibm.com_ <_mailto:HORIE at jp.ibm.com_ <mailto:_HORIE at jp.ibm.com_%20%3c_mailto:HORIE at jp.ibm.com_>>>,
>  >>  >>                         >  >>
>  >>  >>                         "_hotspot-compiler-dev at openjdk.java.net_<_mailto:hotspot-compiler-dev at openjdk.java.net_> <mailto:_hotspot-compiler-dev at openjdk.java.net_%3c_mailto:hotspot-compiler-dev at openjdk.java.net_%3e>"
>  >>  >>                         >  >>
>  >>  >>                         <_hotspot-compiler-dev at openjdk.java.net_<_mailto:hotspot-compiler-dev at openjdk.java.net_ <mailto:_hotspot-compiler-dev at openjdk.java.net_%3c_mailto:hotspot-compiler-dev at openjdk.java.net_>>>,
>  >>  >>                         >  >>
>  >>  >>                         "_ppc-aix-port-dev at openjdk.java.net_<_mailto:ppc-aix-port-dev at openjdk.java.net_> <mailto:_ppc-aix-port-dev at openjdk.java.net_%3c_mailto:ppc-aix-port-dev at openjdk.java.net_%3e>"
>  >>  >>                         >  >> <_ppc-aix-port-dev at openjdk.java.net_
>  > <mailto:_ppc-aix-port-dev at openjdk.java.net_%0b>>  >>                         >  >> <_mailto:ppc-aix-port-dev at openjdk.java.net_>>
>  >>  >>                         >  >> Cc: "Simonis, Volker" <_volker.simonis at sap.com_
>  > <mailto:_volker.simonis at sap.com_%0b>>  >>                         >  >> <_mailto:volker.simonis at sap.com_>>, "Lindenmaier,
>  >>  >>                         >  >> Goetz"<_goetz.lindenmaier at sap.com_
>  > <mailto:_goetz.lindenmaier at sap.com_%0b>>  >>                         >  >> <_mailto:goetz.lindenmaier at sap.com_>>, Gustavo Romero
>  >>  >>                         >  >> <_gromero at linux.vnet.ibm.com_<_mailto:gromero at linux.vnet.ibm.com_ <mailto:_gromero at linux.vnet.ibm.com_%3c_mailto:gromero at linux.vnet.ibm.com_>>>
>  >>  >>                         >  >> Date: 2018/11/20 01:55
>  >>  >>                         >  >> Subject: RE: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         >  >> isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >
>  >>  >>
>  >> ------------------------------------------------------------------------------------------------------------------------
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> Hi Michihiro,
>  >>  >>                         >  >>
>  >>  >>                         >  >> first of all, thanks for working on Power9 optimizations. Please note
>  >>  >>                         >  >> that we don’t have a machine, yet. So other peoplewill have to test.
>  >>  >>                         >  >>
>  >>  >>                         >  >> I think it may be problematic to insert a slow path by
>  >>  >>                         >  >> “generate_method_call_static”. This may be a performancedisadvantage
>  >>  >>                         >  >> for some users of other encodings because your intrinsics prevent
>  >>  >>                         >  >> inlining and further optimizations.
>  >>  >>                         >  >> Would it be possible to introduce more fine-grained intrinsics such
>  >>  >>                         >  >> that the “slow” path is outside of them?
>  >>  >>                         >  >>
>  >>  >>                         >  >> Maybe you can factor out as in the following example?
>  >>  >>                         >  >> if (latin1) return isLatin1Digit(codePoint);
>  >>  >>                         >  >> with isLatin1Digit as HotSpotIntrinsicCandidate.
>  >>  >>                         >  >>
>  >>  >>                         >  >> I can’t judge if this is needed, but I think this should be discussed
>  >>  >>                         >  >> first before going into the details.
>  >>  >>                         >  >>
>  >>  >>                         >  >> Best regards,
>  >>  >>                         >  >> Martin
>  >>  >>                         >  >>
>  >>  >>                         >  >> *
>  >>  >>                         >  >> **From:*Michihiro Horie <_HORIE at jp.ibm.com_
>  > <mailto:_HORIE at jp.ibm.com_%0b>>  >>                         <_mailto:HORIE at jp.ibm.com_>>*
>  >>  >>                         >  >> **Sent:*Freitag, 16. November 2018 12:53*
>  >>  >>                         >  >> **To:*_hotspot-compiler-dev at openjdk.java.net_
>  >>  >>                         >  >>
>  >>  >>                         <_mailto:hotspot-compiler-dev at openjdk.java.net_>;_ppc-aix-port-dev at openjdk.java.net_
>  >>  >>                         >  >> <_mailto:ppc-aix-port-dev at openjdk.java.net_>*
>  >>  >>                         >  >> **Cc:*Doerr, Martin <_martin.doerr at sap.com_
>  > <mailto:_martin.doerr at sap.com_%0b>>  >>                         >  >> <_mailto:martin.doerr at sap.com_>>; Simonis, Volker
>  >>  >>                         >  >> <_volker.simonis at sap.com_<_mailto:volker.simonis at sap.com_ <mailto:_volker.simonis at sap.com_%3c_mailto:volker.simonis at sap.com_>>>;
>  >>  >>                         >  >> Lindenmaier, Goetz <_goetz.lindenmaier at sap.com_
>  > <mailto:_goetz.lindenmaier at sap.com_%0b>>  >>                         >  >> <_mailto:goetz.lindenmaier at sap.com_>>;Gustavo Romero
>  >>  >>                         >  >> <_gromero at linux.vnet.ibm.com_ <_mailto:gromero at linux.vnet.ibm.com_ <mailto:_gromero at linux.vnet.ibm.com_%20%3c_mailto:gromero at linux.vnet.ibm.com_>>>*
>  >>  >>                         >  >> **Subject:*RFR: 8213754: PPC64: Add Intrinsics for
>  >>  >>                         >  >> isDigit/isLowerCase/isUpperCase/isWhitespace
>  >>  >>                         >  >>
>  >>  >>                         >  >> Dear all,
>  >>  >>                         >  >>
>  >>  >>                         >  >> Would you please review following change?
>  >>  >>                         >  >>
>  >>  >>                         >  >> Bug:
>  >>  >>                         >
>  >> __https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8213754-5F-5F&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=sNklBozv1ZztDu7rnM5SUgqTwPn4CPlp4Gp0uGwfVhs&s=yTUTl4D2EPdboqkkAHXYtHx5KijWhAzXOXPBqwME0iQ&e=
>  >>  >>                         >  >> Webrev:
>  >>  >>                         >
>  >> __https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Emhorie_8213754_webrev.00-5F-5F&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=sNklBozv1ZztDu7rnM5SUgqTwPn4CPlp4Gp0uGwfVhs&s=88Ms75RO8511eAgWMsvWrTDLmRRcxcKiEs_DkSZmVlc&e=
>  >>  >>                         <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.00_>
>  >>  >>                         >
>  >>  >>                         >  >>
>  >> <_https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-257Emhorie_8213754_webrev.00-5F&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=oecsIpYF-cifqq2i1JEH0Q&m=sNklBozv1ZztDu7rnM5SUgqTwPn4CPlp4Gp0uGwfVhs&s=zZl2pxTY0Dn-5RZHkTZSnIRpYzb-w9T_4d8cV7gU3iw&e=>
>  >>  >>                         >  >>
>  >>  >>                         >  >> This change includes the intrinsics of Character isDigit,
>  >>  >>                         >  >> isLowerCase, isUpperCase, and isWhitespace to support theLatin1 block
>  >>  >>                         >  >> using POWER9’s instructions cmprb and cmpeqb. The cmprb enables to
>  >>  >>                         >  >> compare a character with 1 or 2 rangedbytes, while the cmpeqb
>  >>  >>                         >  >> compares one with 1 to 8 values. Simple micro benchmark attached
>  >>  >>                         >  >> showed improvements by 20-40%.
>  >>  >>                         >  >> /
>  >>  >>                         >  >> //(See attached file: Latin1Test.java)/
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >  >> Best regards,
>  >>  >>                         >  >> --
>  >>  >>                         >  >> Michihiro,
>  >>  >>                         >  >> IBM Research - Tokyo
>  >>  >>                         >  >>
>  >>  >>                         >  >>
>  >>  >>                         >
>  >>  >>                 >
>  >>  >>                         >
>  >>  >>                         >
>  >>  >>                         >
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >>
>  >>  >
>  >>
>  >>
>  >>
>  >>
>  >
>  >
>  >
> 
> 
> 


More information about the core-libs-dev mailing list