Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]
On Mon, 20 May 2024 10:41:36 GMT, Bhavana Kilambi wrote: >> @dafedafe @dean-long please take a look and let me know if there are further >> issues, thanks! > > Hi @mur47x111, do you happen to have any performance results with this patch? @Bhavana-Kilambi the performance result for x86 is at https://github.com/openjdk/jdk/pull/18226#issuecomment-2007922439 . It is expected to be negligible. - PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2124984579
Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]
On Wed, 17 Apr 2024 12:37:01 GMT, Yudi Zheng wrote: >> `multiply_to_len` seems to be used by `generate_squareToLen` as well for >> aarch64 and riscv but `zlen` is still passed in a register. >> >> https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp#L4710 >> https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/riscv/stubGenerator_riscv.cpp#L2881 >> >> I think it might work anyway but it might be better to adapt them if only >> for completeness. > > @dafedafe @dean-long please take a look and let me know if there are further > issues, thanks! Hi @mur47x111, do you happen to have any performance results with this patch? - PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2120179701
Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]
On Tue, 26 Mar 2024 15:59:33 GMT, Damon Fenacci wrote: >> Yudi Zheng has updated the pull request incrementally with one additional >> commit since the last revision: >> >> address comment. > > `multiply_to_len` seems to be used by `generate_squareToLen` as well for > aarch64 and riscv but `zlen` is still passed in a register. > > https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp#L4710 > https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/riscv/stubGenerator_riscv.cpp#L2881 > > I think it might work anyway but it might be better to adapt them if only for > completeness. > @dafedafe @dean-long please take a look and let me know if there are further > issues, thanks! Thanks @mur47x111! I noticed that you found even a few more `zlen` usages Did you test the change against all affected platforms? - PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2061301421
Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]
On Tue, 26 Mar 2024 15:59:33 GMT, Damon Fenacci wrote: >> Yudi Zheng has updated the pull request incrementally with one additional >> commit since the last revision: >> >> address comment. > > `multiply_to_len` seems to be used by `generate_squareToLen` as well for > aarch64 and riscv but `zlen` is still passed in a register. > > https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp#L4710 > https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/riscv/stubGenerator_riscv.cpp#L2881 > > I think it might work anyway but it might be better to adapt them if only for > completeness. @dafedafe @dean-long please take a look and let me know if there are further issues, thanks! - PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2061162283
Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]
On Tue, 19 Mar 2024 21:09:31 GMT, Yudi Zheng wrote: >> Moving array construction within BigInteger.implMultiplyToLen intrinsic >> candidate to its caller simplifies the intrinsic implementation in JIT >> compiler. > > Yudi Zheng has updated the pull request incrementally with one additional > commit since the last revision: > > address comment. `multiply_to_len` seems to be used by `generate_squareToLen` as well for aarch64 and riscv but `zlen` is still passed in a register. https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp#L4710 https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/riscv/stubGenerator_riscv.cpp#L2881 I think it might work anyway but it might be better to adapt them if only for completeness. - PR Review: https://git.openjdk.org/jdk/pull/18226#pullrequestreview-1960906919
Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]
> Moving array construction within BigInteger.implMultiplyToLen intrinsic > candidate to its caller simplifies the intrinsic implementation in JIT > compiler. Yudi Zheng has updated the pull request incrementally with one additional commit since the last revision: address comment. - Changes: - all: https://git.openjdk.org/jdk/pull/18226/files - new: https://git.openjdk.org/jdk/pull/18226/files/bfc323b7..870a6127 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18226=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18226=01-02 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18226.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18226/head:pull/18226 PR: https://git.openjdk.org/jdk/pull/18226
