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" <grom...@linux.vnet.ibm.com> To: 
"Do"Gustavo Romero" ---2018/12/12 07:57:21---From: "Gustavo Romero" <grom...@linux.vnet.ibm.com> To: "Doerr, 
Martin" <martin.do...@sap.com>, Michihiro Horie/Japan/IBM@IBMJP

From: "Gustavo Romero" <grom...@linux.vnet.ibm.com>
To: "Doerr, Martin" <martin.do...@sap.com>, Michihiro Horie/Japan/IBM@IBMJP
Cc: "core-libs-dev@openjdk.java.net" <core-libs-dev@openjdk.java.net>, "hotspot-compiler-...@openjdk.java.net" 
<hotspot-compiler-...@openjdk.java.net>, "Roger Riggs" <roger.ri...@oracle.com>, "Vladimir Kozlov" 
<vladimir.koz...@oracle.com>, "Simonis, Volker" <volker.simo...@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 <ho...@jp.ibm.com>
 > *Sent:* Dienstag, 11. Dezember 2018 14:08
 > *To:* Doerr, Martin <martin.do...@sap.com>
 > *Cc:* core-libs-dev@openjdk.java.net; Gustavo Romero <grom...@linux.vnet.ibm.com>; 
hotspot-compiler-...@openjdk.java.net; Roger Riggs <roger.ri...@oracle.com>; Vladimir Kozlov 
<vladimir.koz...@oracle.com>; Simonis, Volker <volker.simo...@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.do...@sap.com <mailto:martin.do...@sap.com>>
 > To: Vladimir Kozlov <vladimir.koz...@oracle.com <mailto:vladimir.koz...@oracle.com>>, Michihiro 
Horie <ho...@jp.ibm.com <mailto:ho...@jp.ibm.com>>, Gustavo Romero <grom...@linux.vnet.ibm.com 
<mailto:grom...@linux.vnet.ibm.com>>
 > Cc: "core-libs-dev@openjdk.java.net <mailto:core-libs-dev@openjdk.java.net>" <core-libs-dev@openjdk.java.net 
<mailto:core-libs-dev@openjdk.java.net>>, "hotspot-compiler-...@openjdk.java.net <mailto:hotspot-compiler-...@openjdk.java.net>" 
<hotspot-compiler-...@openjdk.java.net <mailto:hotspot-compiler-...@openjdk.java.net>>, Roger Riggs <roger.ri...@oracle.com 
<mailto:roger.ri...@oracle.com>>, "Simonis, Volker" <volker.simo...@sap.com <mailto:volker.simo...@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.koz...@oracle.com 
<mailto:vladimir.koz...@oracle.com>>
 > Sent: Montag, 10. Dezember 2018 20:33
 > To: Michihiro Horie <ho...@jp.ibm.com <mailto:ho...@jp.ibm.com>>; Gustavo Romero 
<grom...@linux.vnet.ibm.com <mailto:grom...@linux.vnet.ibm.com>>
 > Cc: core-libs-dev@openjdk.java.net <mailto:core-libs-dev@openjdk.java.net>; hotspot-compiler-...@openjdk.java.net 
<mailto:hotspot-compiler-...@openjdk.java.net>; Doerr, Martin <martin.do...@sap.com 
<mailto:martin.do...@sap.com>>; Roger Riggs <roger.ri...@oracle.com <mailto:roger.ri...@oracle.com>>; 
Simonis, Volker <volker.simo...@sap.com <mailto:volker.simo...@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.koz...@oracle.com <mailto:vladimir.koz...@oracle.com>>
 >> To: RogerVladimir Kozlov ---2018/12/08 03:48:04---From: Vladimir Kozlov 
<vladimir.koz...@oracle.com <mailto:vladimir.koz...@oracle.com>> To: Roger Riggs
 >> <roger.ri...@oracle.com <mailto:roger.ri...@oracle.com>>, Michihiro Horie 
<ho...@jp.ibm.com <mailto:ho...@jp.ibm.com>>
 >>
 >> From: Vladimir Kozlov <vladimir.koz...@oracle.com 
<mailto:vladimir.koz...@oracle.com>>
 >> To: Roger Riggs <roger.ri...@oracle.com <mailto:roger.ri...@oracle.com>>, Michihiro 
Horie <ho...@jp.ibm.com <mailto:ho...@jp.ibm.com>>
 >> Cc: core-libs-dev@openjdk.java.net <mailto:core-libs-dev@openjdk.java.net>, 
hotspot-compiler-...@openjdk.java.net <mailto:hotspot-compiler-...@openjdk.java.net>, 
martin.do...@sap.com <mailto:martin.do...@sap.com>, volker.simo...@sap.com 
<mailto:volker.simo...@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.ri...@oracle.com 
<mailto:roger.ri...@oracle.com>>
 >>  >> To: Michihiro Horie <ho...@jp.ibm.com <mailto:ho...@jp.ibm.com>>
 >>  >> Cc: core-libs-dev@openjdk.java.net <mailto:core-libs-dev@openjdk.java.net>, 
hotspot-compiler-...@openjdk.java.net <mailto:hotspot-compiler-...@openjdk.java.net>, 
martin.do...@sap.com <mailto:martin.do...@sap.com>,
 >>  >> vladimir.koz...@oracle.com <mailto:vladimir.koz...@oracle.com>, 
volker.simo...@sap.com <mailto:volker.simo...@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.ri...@oracle.com 
<mailto:roger.ri...@oracle.com>>_ <mailto:roger.ri...@oracle.com>
 >>  >>         To: Michihiro Horie _<ho...@jp.ibm.com <mailto:ho...@jp.ibm.com>>_ 
<mailto:ho...@jp.ibm.com>,
 >>  >>         _vladimir.kozlov@oracle.com_ 
<mailto:_vladimir.kozlov@oracle.com_><mailto:vladimir.koz...@oracle.com>
 >>  >>         Cc: _core-libs-...@openjdk.java.net_ 
<mailto:_core-libs-...@openjdk.java.net_><mailto:core-libs-dev@openjdk.java.net>,
 >>  >>         _hotspot-compiler-...@openjdk.java.net_ 
<mailto:_hotspot-compiler-...@openjdk.java.net_><mailto:hotspot-compiler-...@openjdk.java.net>,
 >>  >>         _martin.doerr@sap.com_ 
<mailto:_martin.doerr@sap.com_><mailto:martin.do...@sap.com>, _volker.simonis@sap.com_ 
<mailto:_volker.simonis@sap.com_>
 >>  >>         <mailto:volker.simo...@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.koz...@oracle.com 
<mailto:vladimir.koz...@oracle.com>>_
 >>  >>                         <mailto:vladimir.koz...@oracle.com>
 >>  >>                         Cc: core-libs-dev _<core-libs-dev@openjdk.java.net 
<mailto:core-libs-dev@openjdk.java.net>>_
 >>  >>                         <mailto:core-libs-dev@openjdk.java.net>,
 >>  >>                         _hotspot-compiler-...@openjdk.java.net_ 
<mailto:_hotspot-compiler-...@openjdk.java.net_>
 >>  >>                         <mailto:hotspot-compiler-...@openjdk.java.net>, 
_martin.doerr@sap.com_ <mailto:_martin.doerr@sap.com_>
 >>  >>                         <mailto:martin.do...@sap.com>, Roger Riggs 
_<roger.ri...@oracle.com <mailto:roger.ri...@oracle.com>&g t;_
 >>  >>                         <mailto:roger.ri...@oracle.com>, 
_volker.simonis@sap.com_ <mailto:_volker.simonis@sap.com_>
 >>  >>                         <mailto:volker.simo...@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.koz...@oracle.com 
<mailto:vladimir.koz...@oracle.com>>_
 >>  >>                         <mailto:vladimir.koz...@oracle.com>
 >>  >>                         To: Michihiro Horie _<ho...@jp.ibm.com 
<mailto:ho...@jp.ibm.com>>_ <mailto:ho...@jp.ibm.com>, Roger
 >>  >>                         Riggs _<roger.ri...@oracle.com 
<mailto:roger.ri...@oracle.com>>_ <mailto:roger.ri...@oracle.com>
 >>  >>                         Cc: core-libs-dev _<core-libs-dev@openjdk.java.net 
<mailto:core-libs-dev@openjdk.java.net>>_
 >>  >>                         <mailto:core-libs-dev@openjdk.java.net>,
 >>  >>                         _hotspot-compiler-...@openjdk.java.net_ 
<mailto:_hotspot-compiler-...@openjdk.java.net_>
 >>  >>                         <mailto:hotspot-compiler-...@openjdk.java.net>, 
_martin.doerr@sap.com_ <mailto:_martin.doerr@sap.com_>
 >>  >>                         <mailto:martin.do...@sap.com>, 
_volker.simonis@sap.com_ <mailto:_volker.simonis@sap.com_>
 >>  >>                         <mailto:volker.simo...@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.ri...@oracle.com 
<mailto:roger.ri...@oracle.com>>_ <mailto:roger.ri...@oracle.com>
 >>  >>                         > To: Vladimir Kozlov _<vladimir.koz...@oracle.com 
<mailto:vladimir.koz...@oracle.com>>_
 >>  >>                         <mailto:vladimir.koz...@oracle.com>, Michihiro Horie 
_<ho...@jp.ibm.com <mailto:ho...@jp.ibm.com>>_
 >>  >>                         <mailto:ho...@jp.ibm.com>, 
_core-libs-...@openjdk.java.net_ <mailto:_core-libs-...@openjdk.java.net_>
 >>  >>                         <mailto:core-libs-dev@openjdk.java.net>
 >>  >>                         > Cc: _volker.simonis@sap.com_ 
<mailto:_volker.simonis@sap.com_><mailto:volker.simo...@sap.com>,
 >>  >>                         _hotspot-compiler-...@openjdk.java.net_ 
<mailto:_hotspot-compiler-...@openjdk.java.net_>
 >>  >>                         <mailto:hotspot-compiler-...@openjdk.java.net>, 
_martin.doerr@sap.com_ <mailto:_martin.doerr@sap.com_>
 >>  >>                         <mailto:martin.do...@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" _<ho...@jp.ibm.com 
<mailto:ho...@jp.ibm.com>>_ <mailto:ho...@jp.ibm.com>
 >>  >>                         >  >> Sent by: "hotspot-compiler-dev"
 >>  >>                         >  >> _<hotspot-compiler-dev-boun...@openjdk.java.net 
<mailto:hotspot-compiler-dev-boun...@openjdk.java.net>>_
 >>  >>                         
<mailto:hotspot-compiler-dev-boun...@openjdk.java.net>
 >>  >>                         >  >> To: "Doerr, Martin" _<martin.do...@sap.com 
<mailto:martin.do...@sap.com>>_
 >>  >>                         <mailto:martin.do...@sap.com>
 >>  >>                         >  >> Cc: "Simonis, Volker" _<volker.simo...@sap.com 
<mailto:volker.simo...@sap.com>>_
 >>  >>                         <mailto:volker.simo...@sap.com>,
 >>  >>                         >  >> _"hotspot-compiler-...@openjdk.java.net 
<mailto:hotspot-compiler-...@openjdk.java.net>"_
 >>  >>                         
<mailto:hotspot-compiler-...@openjdk.java.net>_<hotspot-compiler-...@openjdk.java.net 
<mailto:hotspot-compiler-...@openjdk.java.net>>_
 >>  >>                         <mailto:hotspot-compiler-...@openjdk.java.net>,
 >>  >>                         >  >> _"core-libs-dev@openjdk.java.net 
<mailto:core-libs-dev@openjdk.java.net>"_
 >>  >>                         
<mailto:core-libs-dev@openjdk.java.net>_<core-libs-dev@openjdk.java.net 
<mailto:core-libs-dev@openjdk.java.net>>_
 >>  >>                         <mailto:core-libs-dev@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.do...@sap.com 
<mailto:martin.do...@sap.com>>_
 >>  >>                         <mailto:martin.do...@sap.com>
 >>  >>                         >  >> To: Michihiro Horie _<ho...@jp.ibm.com 
<mailto:ho...@jp.ibm.com>>_ <mailto:ho...@jp.ibm.com>,
 >>  >>                         >  >> _"core-libs-dev@openjdk.java.net 
<mailto:core-libs-dev@openjdk.java.net>"_
 >>  >>                         
<mailto:core-libs-dev@openjdk.java.net>_<core-libs-dev@openjdk.java.net 
<mailto:core-libs-dev@openjdk.java.net>>_
 >>  >>                         <mailto:core-libs-dev@openjdk.java.net>
 >>  >>                         >  >> Cc: _"hotspot-compiler-...@openjdk.java.net 
<mailto:hotspot-compiler-...@openjdk.java.net>"_
 >>  >>                         <mailto:hotspot-compiler-...@openjdk.java.net>
 >>  >>                         >  >> _<hotspot-compiler-...@openjdk.java.net 
<mailto:hotspot-compiler-...@openjdk.java.net>>_
 >>  >>       <mailto:hotspot-compiler-...@openjdk.java.net>, "Simonis,
 >>  >>                         >  >> Volker"_<volker.simo...@sap.com 
<mailto:volker.simo...@sap.com>>_ <mailto:volker.simo...@sap.com>,
 >>  >>                         Gustavo Romero
 >>  >>                         >  >> _<grom...@linux.vnet.ibm.com 
<mailto:grom...@linux.vnet.ibm.com>>_ <mailto:grom...@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 _<ho...@jp.ibm.com 
<mailto:ho...@jp.ibm.com>>_ <mailto:ho...@jp.ibm.com>*
 >>  >>                         >  >> **Sent:*Mittwoch, 21. November 2018 17:14*
 >>  >>                         >  >> **To:*core-libs-dev@openjdk.java.net; 
Doerr, Martin
 >>  >>                         >  >> _<martin.do...@sap.com 
<mailto:martin.do...@sap.com>>_ <mailto:martin.do...@sap.com>*
 >>  >>                         >  >> 
**Cc:*hotspot-compiler-...@openjdk.java.net; Simonis, Volker
 >>  >>                         >  >> _<volker.simo...@sap.com 
<mailto:volker.simo...@sap.com>>_ <mailto:volker.simo...@sap.com>; Gustavo
 >>  >>                         Romero_<grom...@linux.vnet.ibm.com 
<mailto:grom...@linux.vnet.ibm.com>>_ <mailto:grom...@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" <_ho...@jp.ibm.com_ 
<_mailto:ho...@jp.ibm.com_ <mailto:_ho...@jp.ibm.com_%20%3c_mailto:ho...@jp.ibm.com_>>>
 >>  >>                         >  >> Sent by: "hotspot-compiler-dev"
 >>  >>                         >  >>
 >>  >>
 >> 
<_hotspot-compiler-dev-boun...@openjdk.java.net_<_mailto:hotspot-compiler-dev-boun...@openjdk.java.net_
 
<mailto:_hotspot-compiler-dev-boun...@openjdk.java.net_%3c_mailto:hotspot-compiler-dev-boun...@openjdk.java.net_>>>
 >>  >>                         >  >> To: "Doerr, Martin" 
<_martin.doerr@sap.com_
 > <mailto:_martin.doerr@sap.com_%0b>>  >>                         >  >> 
<_mailto:martin.doerr@sap.com_>>
 >>  >>                         >  >> Cc: "Simonis, Volker" 
<_volker.simonis@sap.com_
 > <mailto:_volker.simonis@sap.com_%0b>>  >>                         >  >> 
<_mailto:volker.simonis@sap.com_>>,
 >>  >>                         >  >>
 >>  >>                         
"_ppc-aix-port-...@openjdk.java.net_<_mailto:ppc-aix-port-...@openjdk.java.net_> 
<mailto:_ppc-aix-port-...@openjdk.java.net_%3c_mailto:ppc-aix-port-...@openjdk.java.net_%3e>"
 >>  >>                         >  >>
 >>  >>                         
<_ppc-aix-port-...@openjdk.java.net_<_mailto:ppc-aix-port-...@openjdk.java.net_ 
<mailto:_ppc-aix-port-...@openjdk.java.net_%3c_mailto:ppc-aix-port-...@openjdk.java.net_>>>,
 >>  >>                         >  >>
 >>  >>                         
"_hotspot-compiler-...@openjdk.java.net_<_mailto:hotspot-compiler-...@openjdk.java.net_> 
<mailto:_hotspot-compiler-...@openjdk.java.net_%3c_mailto:hotspot-compiler-...@openjdk.java.net_%3e>"
 >>  >>                         >  >>
 >>  >>                         
<_hotspot-compiler-...@openjdk.java.net_<_mailto:hotspot-compiler-...@openjdk.java.net_ 
<mailto:_hotspot-compiler-...@openjdk.java.net_%3c_mailto:hotspot-compiler-...@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@sap.com_
 > <mailto:_martin.doerr@sap.com_%0b>>  >>                         >  >> 
<_mailto:martin.doerr@sap.com_>>
 >>  >>                         >  >> To: Michihiro Horie <_ho...@jp.ibm.com_ 
<_mailto:ho...@jp.ibm.com_ <mailto:_ho...@jp.ibm.com_%20%3c_mailto:ho...@jp.ibm.com_>>>,
 >>  >>                         >  >>
 >>  >>                         
"_hotspot-compiler-...@openjdk.java.net_<_mailto:hotspot-compiler-...@openjdk.java.net_> 
<mailto:_hotspot-compiler-...@openjdk.java.net_%3c_mailto:hotspot-compiler-...@openjdk.java.net_%3e>"
 >>  >>                         >  >>
 >>  >>                         
<_hotspot-compiler-...@openjdk.java.net_<_mailto:hotspot-compiler-...@openjdk.java.net_ 
<mailto:_hotspot-compiler-...@openjdk.java.net_%3c_mailto:hotspot-compiler-...@openjdk.java.net_>>>,
 >>  >>                         >  >>
 >>  >>                         
"_ppc-aix-port-...@openjdk.java.net_<_mailto:ppc-aix-port-...@openjdk.java.net_> 
<mailto:_ppc-aix-port-...@openjdk.java.net_%3c_mailto:ppc-aix-port-...@openjdk.java.net_%3e>"
 >>  >>                         >  >> <_ppc-aix-port-...@openjdk.java.net_
 > <mailto:_ppc-aix-port-...@openjdk.java.net_%0b>>  >>                         >  >> 
<_mailto:ppc-aix-port-...@openjdk.java.net_>>
 >>  >>                         >  >> Cc: "Simonis, Volker" 
<_volker.simonis@sap.com_
 > <mailto:_volker.simonis@sap.com_%0b>>  >>                         >  >> 
<_mailto:volker.simonis@sap.com_>>, "Lindenmaier,
 >>  >>                         >  >> Goetz"<_goetz.lindenmaier@sap.com_
 > <mailto:_goetz.lindenmaier@sap.com_%0b>>  >>                         >  >> 
<_mailto:goetz.lindenmaier@sap.com_>>, Gustavo Romero
 >>  >>                         >  >> 
<_grom...@linux.vnet.ibm.com_<_mailto:grom...@linux.vnet.ibm.com_ 
<mailto:_grom...@linux.vnet.ibm.com_%3c_mailto:grom...@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 
<_ho...@jp.ibm.com_
 > <mailto:_ho...@jp.ibm.com_%0b>>  >>                         
<_mailto:ho...@jp.ibm.com_>>*
 >>  >>                         >  >> **Sent:*Freitag, 16. November 2018 12:53*
 >>  >>                         >  >> 
**To:*_hotspot-compiler-...@openjdk.java.net_
 >>  >>                         >  >>
 >>  >>                         
<_mailto:hotspot-compiler-...@openjdk.java.net_>;_ppc-aix-port-...@openjdk.java.net_
 >>  >>                         >  >> 
<_mailto:ppc-aix-port-...@openjdk.java.net_>*
 >>  >>                         >  >> **Cc:*Doerr, Martin 
<_martin.doerr@sap.com_
 > <mailto:_martin.doerr@sap.com_%0b>>  >>                         >  >> 
<_mailto:martin.doerr@sap.com_>>; Simonis, Volker
 >>  >>                         >  >> 
<_volker.simonis@sap.com_<_mailto:volker.simonis@sap.com_ 
<mailto:_volker.simonis@sap.com_%3c_mailto:volker.simonis@sap.com_>>>;
 >>  >>                         >  >> Lindenmaier, Goetz 
<_goetz.lindenmaier@sap.com_
 > <mailto:_goetz.lindenmaier@sap.com_%0b>>  >>                         >  >> 
<_mailto:goetz.lindenmaier@sap.com_>>;Gustavo Romero
 >>  >>                         >  >> <_grom...@linux.vnet.ibm.com_ 
<_mailto:grom...@linux.vnet.ibm.com_ 
<mailto:_grom...@linux.vnet.ibm.com_%20%3c_mailto:grom...@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
 >>  >>                         >  >>
 >>  >>                         >  >>
 >>  >>                         >
 >>  >>                 >
 >>  >>                         >
 >>  >>                         >
 >>  >>                         >
 >>  >>
 >>  >>
 >>  >>
 >>  >>
 >>  >>
 >>  >>
 >>  >
 >>
 >>
 >>
 >>
 >
 >
 >



Reply via email to