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

Roger Riggs Roger.Riggs at oracle.com
Fri Dec 7 18:08:00 UTC 2018


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/>
>
> >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>
> To: Michihiro Horie <HORIE at jp.ibm.com>
> Cc: core-libs-dev at openjdk.java.net, 
> hotspot-compiler-dev at openjdk.java.net, martin.doerr at sap.com, 
> vladimir.kozlov at oracle.com, 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._
>         __http://cr.openjdk.java.net/~mhorie/8213754/webrev.03/_
>         <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>
>         To: Michihiro Horie _<HORIE at jp.ibm.com>_
>         <mailto:HORIE at jp.ibm.com>, _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>,
>         _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/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._
>                         __http://cr.openjdk.java.net/~mhorie/8213754/jmh-webrev.00/_
>                         <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._
>                         __http://cr.openjdk.java.net/~mhorie/8213754/webrev.02/_
>                         <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>
>                         Cc: core-libs-dev
>                         _<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>, Roger Riggs
>                         _<roger.riggs at oracle.com>_
>                         <mailto:roger.riggs at oracle.com>,
>                         _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>
>                         To: Michihiro Horie _<HORIE at jp.ibm.com>_
>                         <mailto:HORIE at jp.ibm.com>, Roger Riggs
>                         _<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>,
>                         _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/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>
>                         > 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>,
>                         _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>,
>                         _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>
>                         > 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>
>                         >  >> Sent by: "hotspot-compiler-dev"
>                         >  >>
>                         _<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>
>                         >  >> Cc: "Simonis, Volker"
>                         _<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>_<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>_<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>
>                         >  >> To: Michihiro Horie _<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>_<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>
>                         >  >>
>                         _<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>, Gustavo Romero
>                         >  >> _<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>*
>                         >  >> **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>*
>                         >  >>
>                         **Cc:*hotspot-compiler-dev at openjdk.java.net;
>                         Simonis, Volker
>                         >  >> _<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>*
>                         >  >> **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 _
>                         >  >>
>                         >
>                         ___http://cr.openjdk.java.net/~mhorie/8213754/webrev.01__
>                         <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.01_>
>                         >
>                         >  >>
>                         <_http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.01_>_/_
>                         >  >>
>                         >  >> 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_>>
>                         >  >> Sent by: "hotspot-compiler-dev"
>                         >  >>
>                         <_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_>>
>                         >  >> Cc: "Simonis, Volker"
>                         <_volker.simonis at sap.com_
>                         >  >> <_mailto:volker.simonis at sap.com_>>,
>                         >  >>
>                         "_ppc-aix-port-dev at openjdk.java.net_<_mailto:ppc-aix-port-dev at openjdk.java.net_>"
>                         >  >>
>                         <_ppc-aix-port-dev at openjdk.java.net_<_mailto:ppc-aix-port-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_>>
>                         >  >>
>                         >  >> 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_>>
>                         >  >> To: Michihiro Horie <_HORIE at jp.ibm.com_
>                         <_mailto:HORIE at jp.ibm.com_>>,
>                         >  >>
>                         "_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_>>,
>                         >  >>
>                         "_ppc-aix-port-dev at openjdk.java.net_<_mailto:ppc-aix-port-dev at openjdk.java.net_>"
>                         >  >> <_ppc-aix-port-dev at openjdk.java.net_
>                         >  >>
>                         <_mailto:ppc-aix-port-dev at openjdk.java.net_>>
>                         >  >> Cc: "Simonis, Volker"
>                         <_volker.simonis at sap.com_
>                         >  >> <_mailto:volker.simonis at sap.com_>>,
>                         "Lindenmaier,
>                         >  >> Goetz"<_goetz.lindenmaier at sap.com_
>                         >  >> <_mailto:goetz.lindenmaier at sap.com_>>,
>                         Gustavo Romero
>                         >  >>
>                         <_gromero at linux.vnet.ibm.com_<_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_>>*
>                         >  >> **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_>>;
>                         Simonis, Volker
>                         >  >>
>                         <_volker.simonis at sap.com_<_mailto:volker.simonis at sap.com_>>;
>                         >  >> Lindenmaier, Goetz
>                         <_goetz.lindenmaier at sap.com_
>                         >  >>
>                         <_mailto:goetz.lindenmaier at sap.com_>>;Gustavo
>                         Romero
>                         >  >> <_gromero at linux.vnet.ibm.com_
>                         <_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://bugs.openjdk.java.net/browse/JDK-8213754__
>                         >  >> Webrev:
>                         >
>                         __http://cr.openjdk.java.net/~mhorie/8213754/webrev.00__
>                         <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.00_>
>                         >
>                         >  >>
>                         <_http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.00_>
>                         >  >>
>                         >  >> 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