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

Michihiro Horie HORIE at jp.ibm.com
Thu Nov 29 06:31:23 UTC 2018 

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 for your 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



From:	Roger Riggs <roger.riggs at oracle.com>
To:	Vladimir Kozlov <vladimir.kozlov at oracle.com>, Michihiro Horie
            <HORIE at jp.ibm.com>, core-libs-dev at openjdk.java.net
Cc:	volker.simonis at sap.com, hotspot-compiler-dev at openjdk.java.net,
            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>
>> Sent by: "hotspot-compiler-dev"
>> <hotspot-compiler-dev-bounces at openjdk.java.net>
>> To: "Doerr, Martin" <martin.doerr at sap.com>
>> Cc: "Simonis, Volker" <volker.simonis at sap.com>,
>>
"hotspot-compiler-dev at openjdk.java.net"<hotspot-compiler-dev at openjdk.java.net>,

>> "core-libs-dev at openjdk.java.net" <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>
>> To: Michihiro Horie <HORIE at jp.ibm.com>,
>> "core-libs-dev at openjdk.java.net" <core-libs-dev at openjdk.java.net>
>> Cc: "hotspot-compiler-dev at openjdk.java.net"
>> <hotspot-compiler-dev at openjdk.java.net>, "Simonis,
>> Volker"<volker.simonis at sap.com>, Gustavo Romero
>> <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>*
>> **Sent:*Mittwoch, 21. November 2018 17:14*
>> **To:*core-libs-dev at openjdk.java.net; Doerr, Martin
>> <martin.doerr at sap.com>*
>> **Cc:*hotspot-compiler-dev at openjdk.java.net; Simonis, Volker
>> <volker.simonis at sap.com>; Gustavo Romero<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
>_/_
>>
>> 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
>
>>
>> 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/20181129/144cc6fd/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/20181129/144cc6fd/graycol-0001.gif>

More information about the hotspot-compiler-dev mailing list