Re: RFR: 8264109: Add vectorized implementation for VectorMask.andNot() [v2]
On Thu, 1 Apr 2021 03:39:43 GMT, Xiaohong Gong wrote: >> Currently "VectorMask.andNot()" is not vectorized: >> public VectorMask andNot(VectorMask m) { >> // FIXME: Generate good code here. >> return bOp(m, (i, a, b) -> a && !b); >> } >> This can be implemented with` "and(m.not())" `directly. Since >> `"VectorMask.and()/not()" `have been vectorized in hotspot, `"andNot"` >> can be vectorized as well by calling them. >> >> The performance gain is >100% for such a simple JMH: >> @Benchmark >> public Object andNot(Blackhole bh) { >> boolean[] mask = fm.apply(SPECIES.length()); >> boolean[] r = fmt.apply(SPECIES.length()); >> VectorMask rm = VectorMask.fromArray(SPECIES, r, 0); >> >> for (int ic = 0; ic < INVOC_COUNT; ic++) { >> for (int i = 0; i < mask.length; i += SPECIES.length()) { >> VectorMask vmask = VectorMask.fromArray(SPECIES, mask, i); >> rm = rm.andNot(vmask); >> } >> } >> return rm; >> } > > Xiaohong Gong has updated the pull request incrementally with one additional > commit since the last revision: > > Move the changing to AbstractMask.andNot and revert changes in VectorMask > > Change-Id: Ie3ec8f53cb24526c8e1ccc68038355d024910818 > CustomizedGitHooks: yes LGTM - Marked as reviewed by njian (Committer). PR: https://git.openjdk.java.net/jdk/pull/3211
Re: RFR: 8264109: Add vectorized implementation for VectorMask.andNot() [v2]
On Fri, 2 Apr 2021 09:59:31 GMT, Ningsheng Jian wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Move the changing to AbstractMask.andNot and revert changes in VectorMask >> >> Change-Id: Ie3ec8f53cb24526c8e1ccc68038355d024910818 >> CustomizedGitHooks: yes > > LGTM Thanks for the review @nsjian ! - PR: https://git.openjdk.java.net/jdk/pull/3211
Re: RFR: 8264109: Add vectorized implementation for VectorMask.andNot() [v2]
On Thu, 1 Apr 2021 15:13:31 GMT, Paul Sandoz wrote: >> Xiaohong Gong has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Move the changing to AbstractMask.andNot and revert changes in VectorMask >> >> Change-Id: Ie3ec8f53cb24526c8e1ccc68038355d024910818 >> CustomizedGitHooks: yes > > Looks good. Thanks for your review @PaulSandoz ! - PR: https://git.openjdk.java.net/jdk/pull/3211
Re: RFR: 8264109: Add vectorized implementation for VectorMask.andNot() [v2]
On Thu, 1 Apr 2021 03:39:43 GMT, Xiaohong Gong wrote: >> Currently "VectorMask.andNot()" is not vectorized: >> public VectorMask andNot(VectorMask m) { >> // FIXME: Generate good code here. >> return bOp(m, (i, a, b) -> a && !b); >> } >> This can be implemented with` "and(m.not())" `directly. Since >> `"VectorMask.and()/not()" `have been vectorized in hotspot, `"andNot"` >> can be vectorized as well by calling them. >> >> The performance gain is >100% for such a simple JMH: >> @Benchmark >> public Object andNot(Blackhole bh) { >> boolean[] mask = fm.apply(SPECIES.length()); >> boolean[] r = fmt.apply(SPECIES.length()); >> VectorMask rm = VectorMask.fromArray(SPECIES, r, 0); >> >> for (int ic = 0; ic < INVOC_COUNT; ic++) { >> for (int i = 0; i < mask.length; i += SPECIES.length()) { >> VectorMask vmask = VectorMask.fromArray(SPECIES, mask, i); >> rm = rm.andNot(vmask); >> } >> } >> return rm; >> } > > Xiaohong Gong has updated the pull request incrementally with one additional > commit since the last revision: > > Move the changing to AbstractMask.andNot and revert changes in VectorMask > > Change-Id: Ie3ec8f53cb24526c8e1ccc68038355d024910818 > CustomizedGitHooks: yes Looks good. - Marked as reviewed by psandoz (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3211
Re: RFR: 8264109: Add vectorized implementation for VectorMask.andNot() [v2]
On Wed, 31 Mar 2021 16:42:09 GMT, Paul Sandoz wrote: > Would you mind updating the existing `AbstractMask.andNot` implementation? > rather than changing `VectorMask.andNot`. That fits in with the current > implementation pattern. Hi @PaulSandoz , thanks for looking at this PR. I'v updated the patch according to your comments. Would you mind having look at it again? Thanks so much! - PR: https://git.openjdk.java.net/jdk/pull/3211
Re: RFR: 8264109: Add vectorized implementation for VectorMask.andNot() [v2]
> Currently "VectorMask.andNot()" is not vectorized: > public VectorMask andNot(VectorMask m) { > // FIXME: Generate good code here. > return bOp(m, (i, a, b) -> a && !b); > } > This can be implemented with` "and(m.not())" `directly. Since > `"VectorMask.and()/not()" `have been vectorized in hotspot, `"andNot"` > can be vectorized as well by calling them. > > The performance gain is >100% for such a simple JMH: > @Benchmark > public Object andNot(Blackhole bh) { > boolean[] mask = fm.apply(SPECIES.length()); > boolean[] r = fmt.apply(SPECIES.length()); > VectorMask rm = VectorMask.fromArray(SPECIES, r, 0); > > for (int ic = 0; ic < INVOC_COUNT; ic++) { > for (int i = 0; i < mask.length; i += SPECIES.length()) { > VectorMask vmask = VectorMask.fromArray(SPECIES, mask, i); > rm = rm.andNot(vmask); > } > } > return rm; > } Xiaohong Gong has updated the pull request incrementally with one additional commit since the last revision: Move the changing to AbstractMask.andNot and revert changes in VectorMask Change-Id: Ie3ec8f53cb24526c8e1ccc68038355d024910818 CustomizedGitHooks: yes - Changes: - all: https://git.openjdk.java.net/jdk/pull/3211/files - new: https://git.openjdk.java.net/jdk/pull/3211/files/ccb73d92..33ac041e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3211&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3211&range=00-01 Stats: 9 lines in 2 files changed: 5 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3211.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3211/head:pull/3211 PR: https://git.openjdk.java.net/jdk/pull/3211