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>
To: RogerVladimir Kozlov ---2018/12/08 03:48:04---From: Vladimir Kozlov <vladimir.koz...@oracle.com> To: Roger Riggs
<roger.ri...@oracle.com>, Michihiro Horie <ho...@jp.ibm.com>
From: Vladimir Kozlov <vladimir.koz...@oracle.com>
To: Roger Riggs <roger.ri...@oracle.com>, Michihiro Horie <ho...@jp.ibm.com>
Cc: core-libs-dev@openjdk.java.net, hotspot-compiler-...@openjdk.java.net,
martin.do...@sap.com, 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/>
>>
>> >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>
>> To: Michihiro Horie <ho...@jp.ibm.com>
>> Cc: core-libs-dev@openjdk.java.net, hotspot-compiler-...@openjdk.java.net,
martin.do...@sap.com,
>> vladimir.koz...@oracle.com, 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>
>> To: Michihiro Horie _<ho...@jp.ibm.com>_ <mailto:ho...@jp.ibm.com>,
>> _vladimir.kozlov@oracle.com_ <mailto:vladimir.koz...@oracle.com>
>> Cc: _core-libs-...@openjdk.java.net_
<mailto:core-libs-dev@openjdk.java.net>,
>> _hotspot-compiler-...@openjdk.java.net_
<mailto:hotspot-compiler-...@openjdk.java.net>,
>> _martin.doerr@sap.com_ <mailto:martin.do...@sap.com>,
_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>
>> Cc: core-libs-dev _<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.doerr@sap.com_
>> <mailto:martin.do...@sap.com>, Roger Riggs
_<roger.ri...@oracle.com>_
>> <mailto:roger.ri...@oracle.com>,
_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>
>> To: Michihiro Horie _<ho...@jp.ibm.com>_
<mailto:ho...@jp.ibm.com>, Roger
>> Riggs _<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>,
>> _hotspot-compiler-...@openjdk.java.net_
>> <mailto:hotspot-compiler-...@openjdk.java.net>,
_martin.doerr@sap.com_
>> <mailto:martin.do...@sap.com>,
_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>
>> > To: Vladimir Kozlov _<vladimir.koz...@oracle.com>_
>> <mailto:vladimir.koz...@oracle.com>, Michihiro Horie
_<ho...@jp.ibm.com>_
>> <mailto:ho...@jp.ibm.com>,
_core-libs-...@openjdk.java.net_
>> <mailto:core-libs-dev@openjdk.java.net>
>> > Cc: _volker.simonis@sap.com_
<mailto:volker.simo...@sap.com>,
>> _hotspot-compiler-...@openjdk.java.net_
>> <mailto:hotspot-compiler-...@openjdk.java.net>,
_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>
>> > >> Sent by: "hotspot-compiler-dev"
>> > >>
_<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>
>> > >> Cc: "Simonis, Volker"
_<volker.simo...@sap.com>_
>> <mailto:volker.simo...@sap.com>,
>> > >> _"hotspot-compiler-...@openjdk.java.net"_
>>
<mailto:hotspot-compiler-...@openjdk.java.net>_<hotspot-compiler-...@openjdk.java.net>_
>> <mailto:hotspot-compiler-...@openjdk.java.net>,
>> > >> _"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>
>> > >> 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>
>> > >> To: Michihiro Horie _<ho...@jp.ibm.com>_
<mailto:ho...@jp.ibm.com>,
>> > >> _"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>
>> > >> Cc: _"hotspot-compiler-...@openjdk.java.net"_
>> <mailto:hotspot-compiler-...@openjdk.java.net>
>> > >> _<hotspot-compiler-...@openjdk.java.net>_
>> <mailto:hotspot-compiler-...@openjdk.java.net>,
"Simonis,
>> > >> Volker"_<volker.simo...@sap.com>_
<mailto:volker.simo...@sap.com>,
>> Gustavo Romero
>> > >> _<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>*
>> > >> **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>*
>> > >> **Cc:*hotspot-compiler-...@openjdk.java.net;
Simonis, Volker
>> > >> _<volker.simo...@sap.com>_
<mailto:volker.simo...@sap.com>; Gustavo
>> Romero_<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_>>
>> > >> Sent by: "hotspot-compiler-dev"
>> > >>
>>
<_hotspot-compiler-dev-boun...@openjdk.java.net_<_mailto:hotspot-compiler-dev-boun...@openjdk.java.net_>>
>> > >> To: "Doerr, Martin" <_martin.doerr@sap.com_
>> > >> <_mailto:martin.doerr@sap.com_>>
>> > >> Cc: "Simonis, Volker"
<_volker.simonis@sap.com_
>> > >> <_mailto:volker.simonis@sap.com_>>,
>> > >>
>>
"_ppc-aix-port-...@openjdk.java.net_<_mailto:ppc-aix-port-...@openjdk.java.net_>"
>> > >>
>>
<_ppc-aix-port-...@openjdk.java.net_<_mailto:ppc-aix-port-...@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_>>
>> > >>
>> > >> 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_>>
>> > >> To: Michihiro Horie <_ho...@jp.ibm.com_
<_mailto:ho...@jp.ibm.com_>>,
>> > >>
>>
"_hotspot-compiler-...@openjdk.java.net_<_mailto:hotspot-compiler-...@openjdk.java.net_>"
>> > >>
>>
<_hotspot-compiler-...@openjdk.java.net_<_mailto:hotspot-compiler-...@openjdk.java.net_>>,
>> > >>
>>
"_ppc-aix-port-...@openjdk.java.net_<_mailto:ppc-aix-port-...@openjdk.java.net_>"
>> > >> <_ppc-aix-port-...@openjdk.java.net_
>> > >> <_mailto:ppc-aix-port-...@openjdk.java.net_>>
>> > >> Cc: "Simonis, Volker"
<_volker.simonis@sap.com_
>> > >> <_mailto:volker.simonis@sap.com_>>,
"Lindenmaier,
>> > >> Goetz"<_goetz.lindenmaier@sap.com_
>> > >> <_mailto:goetz.lindenmaier@sap.com_>>,
Gustavo Romero
>> > >>
<_grom...@linux.vnet.ibm.com_<_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_>>*
>> > >> **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_>>; Simonis,
Volker
>> > >>
<_volker.simonis@sap.com_<_mailto:volker.simonis@sap.com_>>;
>> > >> Lindenmaier, Goetz
<_goetz.lindenmaier@sap.com_
>> > >> <_mailto:goetz.lindenmaier@sap.com_>>;Gustavo
Romero
>> > >> <_grom...@linux.vnet.ibm.com_
<_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
>> > >>
>> > >>
>> >
>> >
>> >
>> >
>> >
>>
>>
>>
>>
>>
>>
>
