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

Thu Nov 29 02:21:18 UTC 2018

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