RFR: 8213754: PPC64: Add Intrinsics for isDigit/isLowerCase/isUpperCase/isWhitespace
Michihiro Horie
HORIE at jp.ibm.com
Tue Dec 11 15:48:27 UTC 2018
Hi Martin,
Thanks a lot! I think Gustavo is now testing.
>Note that changes pushed before Thursday 16:00 UTC will go into jdk12.
Also thanks for this information.
Best regards,
--
Michihiro,
IBM Research - Tokyo
From: "Doerr, Martin" <martin.doerr at sap.com>
To: Michihiro Horie <HORIE at jp.ibm.com>
Cc: "core-libs-dev at openjdk.java.net"
<core-libs-dev at openjdk.java.net>, Gustavo Romero
<gromero at linux.vnet.ibm.com>,
"hotspot-compiler-dev at openjdk.java.net"
<hotspot-compiler-dev at openjdk.java.net>, Roger Riggs
<Roger.Riggs at oracle.com>, Vladimir Kozlov
<vladimir.kozlov at oracle.com>, "Simonis, Volker"
<volker.simonis at sap.com>
Date: 2018/12/12 00:34
Subject: RE: RFR: 8213754: PPC64: Add Intrinsics for
isDigit/isLowerCase/isUpperCase/isWhitespace
Hi Michihiro,
thanks for updating. I think it can get shipped when Gustavo is fine with
it and jdk/submit tests have passed.
Note that changes pushed before Thursday 16:00 UTC will go into jdk12.
Best regards,
Martin
From: Michihiro Horie <HORIE at jp.ibm.com>
Sent: Dienstag, 11. Dezember 2018 14:08
To: Doerr, Martin <martin.doerr at sap.com>
Cc: core-libs-dev at openjdk.java.net; Gustavo Romero
<gromero at linux.vnet.ibm.com>; hotspot-compiler-dev at openjdk.java.net; Roger
Riggs <Roger.Riggs at oracle.com>; Vladimir Kozlov
<vladimir.kozlov at oracle.com>; Simonis, Volker <volker.simonis at sap.com>
Subject: RE: RFR: 8213754: PPC64: Add Intrinsics for
isDigit/isLowerCase/isUpperCase/isWhitespace
Hi Martin,
Thank you so much for your review and pointing out the issue on flag
enablement on PPC64.
Latest webrev enables the flag on PPC64 using has_darn in vm_version_ppc
and it is used in match_rule_supported in the ad file.
http://cr.openjdk.java.net/~mhorie/8213754/webrev.06/
Best regards,
--
Michihiro,
IBM Research - Tokyo
Inactive hide details for "Doerr, Martin" ---2018/12/11 20:31:40---Hi
Michihiro, the shared code looks good to me, now."Doerr, Martin"
---2018/12/11 20:31:40---Hi Michihiro, the shared code looks good to me,
now.
From: "Doerr, Martin" <martin.doerr at sap.com>
To: Vladimir Kozlov <vladimir.kozlov at oracle.com>, Michihiro Horie <
HORIE at jp.ibm.com>, Gustavo Romero <gromero at linux.vnet.ibm.com>
Cc: "core-libs-dev at openjdk.java.net" <core-libs-dev at openjdk.java.net>, "
hotspot-compiler-dev at openjdk.java.net" <
hotspot-compiler-dev at openjdk.java.net>, Roger Riggs <Roger.Riggs at oracle.com
>, "Simonis, Volker" <volker.simonis at sap.com>
Date: 2018/12/11 20:31
Subject: RE: RFR: 8213754: PPC64: Add Intrinsics for
isDigit/isLowerCase/isUpperCase/isWhitespace
Hi Michihiro,
the shared code looks good to me, now.
Looks like the PPC64 is not consistent any more.
Where do you enable UseCharacterCompareIntrinsics on PPC64?
Why aren't you using it for match_rule_supported in the ad file?
I think has_darn could be used to enable it in vm_version_ppc.
Best regards,
Martin
-----Original Message-----
From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
Sent: Montag, 10. Dezember 2018 20:33
To: Michihiro Horie <HORIE at jp.ibm.com>; Gustavo Romero <
gromero at linux.vnet.ibm.com>
Cc: core-libs-dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net;
Doerr, Martin <martin.doerr at sap.com>; Roger Riggs <Roger.Riggs at oracle.com>;
Simonis, Volker <volker.simonis at sap.com>
Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for
isDigit/isLowerCase/isUpperCase/isWhitespace
On 12/9/18 5:28 PM, Michihiro Horie wrote:
> Hi Vladimir,
>
> Thanks a lot for your review. I also fixed the copyright in
intrinsicnode.hpp.
>
> >Did you look on code generated by C2 with your latest changes?
> Yes,I usually check generated code with Oprofile and they were as
expected like:
> :
> 90 0.0905 : 3fff64c27654: rlwinm r12,r9,24,8,31
> 12 0.0121 : 3fff64c27658: li r11,14640
> 15 0.0151 : 3fff64c2765c: cmprb cr5,0,r31,r11
> 5527 5.5587 : 3fff64c27660: setb r11,cr5
> 36010 36.2164 : 3fff64c27664: stb r11,16(r15)
Good.
> :
>
> I found a CSR FAQ that mentions adding a diagnostic flag do not need CSR
process. This time I do not need CSR.
> https://wiki.openjdk.java.net/display/csr/CSR+FAQs
Thank is correct.
>
> Hi Gustavo,
> Could I ask you to sponsor the latest change webrev.05? I would like you
to test it on your P9 node too.
Thanks,
Vladimir
>
>
> Best regards,
> --
> Michihiro,
> IBM Research - Tokyo
>
> Inactive hide details for Vladimir Kozlov ---2018/12/08 03:48:04---From:
Vladimir Kozlov <vladimir.kozlov at oracle.com>
> To: RogerVladimir Kozlov ---2018/12/08 03:48:04---From: Vladimir Kozlov <
vladimir.kozlov at oracle.com> To: Roger Riggs
> <Roger.Riggs at oracle.com>, Michihiro Horie <HORIE at jp.ibm.com>
>
> From: Vladimir Kozlov <vladimir.kozlov at oracle.com>
> To: Roger Riggs <Roger.Riggs at oracle.com>, 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, volker.simonis at sap.com
> Date: 2018/12/08 03:48
> Subject: Re: RFR: 8213754: PPC64: Add Intrinsics for
isDigit/isLowerCase/isUpperCase/isWhitespace
>
>
------------------------------------------------------------------------------------------------------------------------
>
>
>
> Hi Michihiro,
>
> Hotspot changes looks fine.
> Did you look on code generated by C2 with your latest changes?
>
> And Copyright year change in intrinsicnode.hpp is incorrect:
>
> - * Copyright (c) 2015, Oracle and/or its affiliates. All rights
reserved.
> + * Copyright (c) 2018, Oracle and/or its affiliates. All rights
reserved.
>
> Should be
>
> * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights
reserved.
>
>
> Thanks,
> Vladimir
>
> On 12/7/18 10:08 AM, Roger Riggs wrote:
> > Hi Michihiro,
> >
> > Looks fine with the updates.
> >
> > Its great that the change to isDigit is beneficial on other platforms
too.
> >
> > A very small editorial:
> > There should be a comma "," after the 2018 in the copyright of:
> > test/micro/org/openjdk/bench/java/lang/Characters.java
> >
> > Thanks, Roger
> >
> >
> > On 12/07/2018 03:13 AM, Michihiro Horie wrote:
> >>
> >> Hi Roger,
> >>
> >> I updated my webrev.
> >> http://cr.openjdk.java.net/~mhorie/8213754/webrev.04/ <
http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.04/>
> >> <http://cr.openjdk.java.net/%7Emhorie/8213754/webrev.04/>
> >>
> >> >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
> >> > >>
> >> > >>
> >> >
> >> >
> >> >
> >> >
> >> >
> >>
> >>
> >>
> >>
> >>
> >>
> >
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20181212/f235b93f/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20181212/f235b93f/graycol-0001.gif>
More information about the hotspot-compiler-dev
mailing list