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 This change includes the intrinsics of Character isDigit, isLowerCase, isUpperCase, and isWhitespace to support the Latin1 block using POWER9’s instructions cmprb and cmpeqb. The cmprb enables to compare a character with 1 or 2 ranged bytes, 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
Hi Michihiro, first of all, thanks for working on Power9 optimizations. Please note that we don't have a machine, yet. So other people will have to test. I think it may be problematic to insert a slow path by "generate_method_call_static". This may be a performance disadvantage 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@jp.ibm.com> Sent: Freitag, 16. November 2018 12:53 To: hotspot-compiler-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net Cc: Doerr, Martin <martin.doerr@sap.com>; Simonis, Volker <volker.simonis@sap.com>; Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; Gustavo Romero <gromero@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 This change includes the intrinsics of Character isDigit, isLowerCase, isUpperCase, and isWhitespace to support the Latin1 block using POWER9's instructions cmprb and cmpeqb. The cmprb enables to compare a character with 1 or 2 ranged bytes, 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
Hi Martin, Thank you for giving your helpful comments. I did not recognize “ generate_method_call_static” prevents any optimizations, 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 From: "Doerr, Martin" <martin.doerr@sap.com> To: Michihiro Horie <HORIE@jp.ibm.com>, "hotspot-compiler-dev@openjdk.java.net" <hotspot-compiler-dev@openjdk.java.net>, "ppc-aix-port-dev@openjdk.java.net" <ppc-aix-port-dev@openjdk.java.net> Cc: "Simonis, Volker" <volker.simonis@sap.com>, "Lindenmaier, Goetz" <goetz.lindenmaier@sap.com>, Gustavo Romero <gromero@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 people will have to test. I think it may be problematic to insert a slow path by “generate_method_call_static”. This may be a performance disadvantage 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@jp.ibm.com> Sent: Freitag, 16. November 2018 12:53 To: hotspot-compiler-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net Cc: Doerr, Martin <martin.doerr@sap.com>; Simonis, Volker <volker.simonis@sap.com>; Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; Gustavo Romero <gromero@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 This change includes the intrinsics of Character isDigit, isLowerCase, isUpperCase, and isWhitespace to support the Latin1 block using POWER9’s instructions cmprb and cmpeqb. The cmprb enables to compare a character with 1 or 2 ranged bytes, 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
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
participants (3)
-
Doerr, Martin
-
Gustavo Romero
-
Michihiro Horie