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

Vladimir Kozlov vladimir.kozlov at oracle.com
Fri Dec 7 18:49:28 UTC 2018


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/>
>>
>> >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