Re: RFR: 8274242: Implement fast-path for ASCII-compatible CharsetEncoders on x86 [v3]
On Mon, 27 Sep 2021 12:11:22 GMT, Claes Redestad wrote: >> src/hotspot/share/opto/c2compiler.cpp line 222: >> >>> 220: #if !defined(X86) >>> 221: return false; // not yet implemented >>> 222: #endif >> >> It might be a little more work, but I think it's cleaner to move the >> decision whether the intrinisc is supported into the Matcher like for most >> other intrinsics and keep this code here platform independent. Otherwise we >> will get an increasing cascade of ifdefs as people start implementing this >> for other platforms. > > Not too much work. I recently introduced platform-specific `matcher_*.hpp` > files, so since then adding a boolean constant is easy (no need to muck with > the .ad files). Does the changes in 9800a99 resolve your concerns? - PR: https://git.openjdk.java.net/jdk/pull/5621
Re: RFR: 8274242: Implement fast-path for ASCII-compatible CharsetEncoders on x86 [v3]
> This patch extends the `ISO_8859_1.implEncodeISOArray` intrinsic on x86 to > work also for ASCII encoding, which makes for example the `UTF_8$Encoder` > perform on par with (or outperform) similarly getting charset encoded bytes > from a String. The former took a small performance hit in JDK 9, and the > latter improved greatly in the same release. > > Extending the `EncodeIsoArray` intrinsics on other platforms should be > possible, but I'm unfamiliar with the macro assembler in general and unlike > the x86 intrinsic they don't use a simple vectorized mask to implement the > latin-1 check. For example aarch64 seem to filter out the low bytes and then > check if there's any bits set in the high bytes. Clever, but very different > to the 0xFF80 2-byte mask that an ASCII test wants. Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Add Matcher predicate to avoid changing shared code as non-x86 platforms implements support for the _encodeAsciiArray intrinsic - Changes: - all: https://git.openjdk.java.net/jdk/pull/5621/files - new: https://git.openjdk.java.net/jdk/pull/5621/files/12ab6ff5..9800a99a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5621=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5621=01-02 Stats: 17 lines in 6 files changed: 14 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5621.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5621/head:pull/5621 PR: https://git.openjdk.java.net/jdk/pull/5621
Re: RFR: 8274242: Implement fast-path for ASCII-compatible CharsetEncoders on x86 [v3]
On Mon, 27 Sep 2021 11:59:54 GMT, Claes Redestad wrote: > > Should we remove the "iso" part from the method/class names? > > I'm open to suggestions, but I've not been able to think of anything better. > `encodeISOOrASCII` doesn't seem helpful and since ASCII is a subset of the > ISO-8859-1 encoding referred to by the "iso" moniker then the ASCII-only > variant is technically encoding chars to valid ISO-8859-1. Okay, that's fine with me. - PR: https://git.openjdk.java.net/jdk/pull/5621
Re: RFR: 8274242: Implement fast-path for ASCII-compatible CharsetEncoders on x86 [v3]
On Mon, 27 Sep 2021 09:34:21 GMT, Volker Simonis wrote: >> Claes Redestad has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add Matcher predicate to avoid changing shared code as non-x86 platforms >> implements support for the _encodeAsciiArray intrinsic > > src/hotspot/share/opto/c2compiler.cpp line 222: > >> 220: #if !defined(X86) >> 221: return false; // not yet implemented >> 222: #endif > > It might be a little more work, but I think it's cleaner to move the decision > whether the intrinisc is supported into the Matcher like for most other > intrinsics and keep this code here platform independent. Otherwise we will > get an increasing cascade of ifdefs as people start implementing this for > other platforms. Not too much work. I recently introduced platform-specific `matcher_*.hpp` files, so since then adding a boolean constant is easy (no need to muck with the .ad files). - PR: https://git.openjdk.java.net/jdk/pull/5621
