[riscv-port] RFR: 8278994: riscv: RVC: basic instruction set
Hi team, This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu. Thanks, Xiaolin ------------- Commit messages: - RVC: basic instruction set Changes: https://git.openjdk.java.net/riscv-port/pull/34/files Webrev: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278994 Stats: 627 lines in 8 files changed: 607 ins; 3 del; 17 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 20 Dec 2021 08:10:10 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Thanks, Xiaolin
Looks like the previous discussion about compressible region is not reflected here in this PR. Could you please explain your approach? ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Tue, 21 Dec 2021 01:45:25 GMT, Fei Yang <fyang@openjdk.org> wrote:
Looks like the previous discussion about compressible region is not reflected here in this PR. Could you please explain your approach?
Thanks for the reviews, Felix. The Region things are at [here](https://github.com/openjdk/riscv-port/pull/34/files#diff-d09fa54666353b5d9df...) and [here](https://github.com/openjdk/riscv-port/pull/34/files#diff-d09fa54666353b5d9df...), and currently, there is no usage of them. Besides, I sent messages to Yadong privately to discuss the previous design `RVC_Assembler` things. Maybe this patch could hold for a little moment after the discussion. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 20 Dec 2021 08:10:10 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Thanks, Xiaolin
I have discussed with Yadong and we may hold the optional `RVC_Assembler` implementation temporarily. This will not affect the `CompressibleRegion` implementation so please feel safe. Now this patch might be ready to review because it seems less possible to change in design, and thank you for your patience. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 20 Dec 2021 08:10:10 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Thanks, Xiaolin
Changes requested by fyang (Lead). src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 27:
25: 26: #ifndef CPU_RISCV_ASSEMBLER_RISCV_CEXT_HPP 27: #define CPU_RISCV_ASSEMBLER_RISCV_CEXT_HPP
This should corresponds to the real file name src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 30:
28: 29: private: 30: bool _in_compressible_region;
So where will this variable be used then? src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 36:
34: public: 35: 36: // C-Ext: If an instruction is compressible, then
I would suggest we change "C-Ext" into something like "RVC". src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 67:
65: 66: // C-Ext: extract a 16-bit instruction. 67: static inline uint16_t c_extract(uint16_t val, unsigned msb, unsigned lsb) {
We definitely need test coverage for those newly added assember functions for this PR. src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 571:
569: 570: // C-Ext: an uncompressible region 571: class UncompressibleRegion : public AbstractCompressibleRegion {
It looks strange that UncompressibleRegion extends the abstract 'compressible' region here. src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1321:
1319: #ifdef ASSERT 1320: tty->print_cr("pd_patch_instruction_size: instruction 0x%x at " INTPTR_FORMAT " could not be patched!\n", *(unsigned*)branch, p2i(branch)); 1321: Disassembler::decode(branch - 10, branch + 10);
How is this change tested? ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Fri, 24 Dec 2021 07:13:09 GMT, Fei Yang <fyang@openjdk.org> wrote:
Xiaolin Zheng has updated the pull request incrementally with three additional commits since the last revision:
- Cover most RVC instructions by using CompressibleRegion to cover minimal functions in C2 - Revise as proposed comments, including - Fix macros in assembler_riscv_c.hpp - Remove UncompressibleRegion - Modify comments - Change names: C-Ext to RVC - Enable RVC instructions (based on the basic patch)
src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1321:
1319: #ifdef ASSERT 1320: tty->print_cr("pd_patch_instruction_size: instruction 0x%x at " INTPTR_FORMAT " could not be patched!\n", *(unsigned*)branch, p2i(branch)); 1321: Disassembler::decode(branch - 10, branch + 10);
How is this change tested?
This is a pretty print and I deliberately violated one `movptr` under relocation to test it. If it looks not good I could remove that. Other comments are fixed in the new patches. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Fri, 24 Dec 2021 07:08:31 GMT, Fei Yang <fyang@openjdk.org> wrote:
Xiaolin Zheng has updated the pull request incrementally with three additional commits since the last revision:
- Cover most RVC instructions by using CompressibleRegion to cover minimal functions in C2 - Revise as proposed comments, including - Fix macros in assembler_riscv_c.hpp - Remove UncompressibleRegion - Modify comments - Change names: C-Ext to RVC - Enable RVC instructions (based on the basic patch)
src/hotspot/cpu/riscv/assembler_riscv_c.hpp line 30:
28: 29: private: 30: bool _in_compressible_region;
So where will this variable be used then?
Sorry I missed this comment. This variable `_in_compressible_region` is used in `CompressibleRegion cr(&_masm)`. During [RTTI](https://github.com/openjdk/riscv-port/blob/dbc763edfbee277125283fdd0cf98f46d...) this variable is set, and when emitting instructions inside the region, instructions inside the region will be considered qualified as safe to be emitted as RVC instructions -- please see the [macros](https://github.com/openjdk/riscv-port/blob/dbc763edfbee277125283fdd0cf98f46d...) and [an example usage](https://github.com/openjdk/riscv-port/blob/dbc763edfbee277125283fdd0cf98f46d...). ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with three additional commits since the last revision: - Cover most RVC instructions by using CompressibleRegion to cover minimal functions in C2 - Revise as proposed comments, including - Fix macros in assembler_riscv_c.hpp - Remove UncompressibleRegion - Modify comments - Change names: C-Ext to RVC - Enable RVC instructions (based on the basic patch) ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/41ec7bd1..dbc763ed Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=01 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=00-01 Stats: 622 lines in 13 files changed: 479 ins; 32 del; 111 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Fri, 24 Dec 2021 07:55:24 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with three additional commits since the last revision:
- Cover most RVC instructions by using CompressibleRegion to cover minimal functions in C2 - Revise as proposed comments, including - Fix macros in assembler_riscv_c.hpp - Remove UncompressibleRegion - Modify comments - Change names: C-Ext to RVC - Enable RVC instructions (based on the basic patch)
Thanks for your reviews, Felix. In the morning Yadong discussed with me your review comments, so I have pushed additional patches to ensure nearly all functions I defined have been properly 'use'd. I think that might increase the burden to review this patch because of the new changes, and I feel quite sorry for that. Now excluding `c.beqz`, `c.bnez` and `c.j`, which need to be supported in `MachBranchNode`s afterward by Wei Kuai, all RVC instructions have been covered in usage by adding `CompressibleRegion`s in the ad file. Also, I revised the comments in the code based on your suggestions and hope they look good. This may need another period to review so no rush. Also, due to [Wei Kuai's code](https://github.com/openjdk/riscv-port/blob/dbc763edfbee277125283fdd0cf98f46d...) is added into this patch now, I assume I should add him as a co-author of this patch. Hmm. Hope this time it could succeed. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Fri, 24 Dec 2021 07:55:24 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with three additional commits since the last revision:
- Cover most RVC instructions by using CompressibleRegion to cover minimal functions in C2 - Revise as proposed comments, including - Fix macros in assembler_riscv_c.hpp - Remove UncompressibleRegion - Modify comments - Change names: C-Ext to RVC - Enable RVC instructions (based on the basic patch)
-- Quite feel sorry for the disturbance :-( ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Fri, 24 Dec 2021 07:55:24 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with three additional commits since the last revision:
- Cover most RVC instructions by using CompressibleRegion to cover minimal functions in C2 - Revise as proposed comments, including - Fix macros in assembler_riscv_c.hpp - Remove UncompressibleRegion - Modify comments - Change names: C-Ext to RVC - Enable RVC instructions (based on the basic patch)
Rebased and fixed conflicts with RVB(#37)'s initial load. Tested build and a simple spring-boot application with/without RVC. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Cover most RVC instructions by using CompressibleRegion to cover minimal functions in C2 - Revise as proposed comments, including - Fix macros in assembler_riscv_c.hpp - Remove UncompressibleRegion - Modify comments - Change names: C-Ext to RVC - Enable RVC instructions (based on the basic patch) - RVC: basic instruction set ------------- Changes: https://git.openjdk.java.net/riscv-port/pull/34/files Webrev: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=02 Stats: 1177 lines in 15 files changed: 1061 ins; 10 del; 106 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Update licenses to the new year ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/5df07a18..2accc5b5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=03 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=02-03 Stats: 37 lines in 15 files changed: 0 ins; 0 del; 37 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: Update licenses to the new year ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/2accc5b5..84de97e9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=04 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=03-04 Stats: 37 lines in 15 files changed: 0 ins; 0 del; 37 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Tue, 4 Jan 2022 04:34:11 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR.
src/hotspot/cpu/riscv/assembler_riscv.hpp line 431:
429: INSN(remu, 0b0110011, 0b111, 0b0000001, NOT_COMPRESSIBLE); 430: INSN(remw, 0b0111011, 0b110, 0b0000001, NOT_COMPRESSIBLE); 431: INSN(remuw, 0b0111011, 0b111, 0b0000001, NOT_COMPRESSIBLE);
Shall we arrange the instructions to 2 groups (compressible and non-compressible), instead of crossing each other? src/hotspot/cpu/riscv/c2_globals_riscv.hpp line 49:
47: define_pd_global(intx, FreqInlineSize, 325); 48: define_pd_global(intx, MinJumpTableSize, 10); 49: define_pd_global(intx, InteriorEntryAlignment, 4);
It's best to keep InteriorEntryAlignment and CodeEntryAlignment unchanged if there's no obvious benefits. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Tue, 4 Jan 2022 11:23:52 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
Xiaolin Zheng has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR.
src/hotspot/cpu/riscv/assembler_riscv.hpp line 431:
429: INSN(remu, 0b0110011, 0b111, 0b0000001, NOT_COMPRESSIBLE); 430: INSN(remw, 0b0111011, 0b110, 0b0000001, NOT_COMPRESSIBLE); 431: INSN(remuw, 0b0111011, 0b111, 0b0000001, NOT_COMPRESSIBLE);
Shall we arrange the instructions to 2 groups (compressible and non-compressible), instead of crossing each other?
Thanks for your reviews Yadong. These macros are not existed anymore - I have separated them off and hope you like it. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Wed, 5 Jan 2022 03:47:42 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
Xiaolin Zheng has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR.
src/hotspot/cpu/riscv/c2_globals_riscv.hpp line 49:
47: define_pd_global(intx, FreqInlineSize, 325); 48: define_pd_global(intx, MinJumpTableSize, 10); 49: define_pd_global(intx, InteriorEntryAlignment, 4);
It's best to keep InteriorEntryAlignment and CodeEntryAlignment unchanged if there's no obvious benefits.
These changes are made for shrinking the code size to a minimum level. Though regarding them doing no harm and as an enhancement, I consider it also great to remain them unchanged. Need some testing work after the changes and if there are other suggestions I feel pleased to change. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with five additional commits since the last revision: - Manually inline all macros into functions as discussions - Remove assembler_riscv_c.hpp as discussions - Remove COMPRESSIBLE & NOT_COMPRESSIBLE macros by adding one layer as discussions - Fix remaining CEXT -> RVC - Remove Alignment-related changes as discussions ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/84de97e9..e0de8c92 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=05 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=04-05 Stats: 2004 lines in 5 files changed: 1016 ins; 902 del; 86 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Remain an 'minimum_alignment' unchanged ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/e0de8c92..42abb1b8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=06 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Remove remaining macros as discussions ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/42abb1b8..2ecdc92b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=07 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=06-07 Stats: 154 lines in 1 file changed: 15 ins; 15 del; 124 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Thu, 6 Jan 2022 09:30:19 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
Remove remaining macros as discussions
src/hotspot/cpu/riscv/assembler_riscv.hpp line 1345:
1343: protected: 1344: Assembler *_masm; 1345: bool _prev_in_compressible_region;
suggestion: rename to "_saved_in_compressible_region" src/hotspot/cpu/riscv/assembler_riscv.hpp line 1858:
1856: 1857: // -------------------------- 1858: // sub/subw -> c.sub/c.subw
It's better to move this comment before the if block which does compression. src/hotspot/cpu/riscv/assembler_riscv.hpp line 1898:
1896: private: 1897: // some helper functions 1898: bool check_rvc() const {
maybe rename to "bool do_compress"? ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Thu, 6 Jan 2022 09:46:25 GMT, Fei Yang <fyang@openjdk.org> wrote:
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
Remove remaining macros as discussions
src/hotspot/cpu/riscv/assembler_riscv.hpp line 1858:
1856: 1857: // -------------------------- 1858: // sub/subw -> c.sub/c.subw
It's better to move this comment before the if block which does compression.
Thanks for pointing out this and I have moved them before every if block. A trivial detail might be that we need to use `/**/` inside macros instead of `//`. Also fixed other comments. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Thu, 6 Jan 2022 09:30:19 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
Remove remaining macros as discussions
src/hotspot/cpu/riscv/globals_riscv.hpp line 95:
93: product(bool, UseRVV, false, EXPERIMENTAL, "Use RVV instructions") \ 94: product(bool, UseRVB, false, EXPERIMENTAL, "Use RVB instructions") \ 95: product(bool, UseRVC, false, EXPERIMENTAL, "Use RVC instructions") \
redundant '' at end of line, we should remove it. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Rename misc functions and change the positions of some comments ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/2ecdc92b..6102220b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=08 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=07-08 Stats: 68 lines in 2 files changed: 21 ins; 20 del; 27 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits: - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - Remove assembler_riscv_c.hpp as discussions - Remove COMPRESSIBLE & NOT_COMPRESSIBLE macros by adding one layer as discussions - Fix remaining CEXT -> RVC - Remove Alignment-related changes as discussions - Update licenses to the new year - ... and 4 more: https://git.openjdk.java.net/riscv-port/compare/c7944edf...2a6ff151 ------------- Changes: https://git.openjdk.java.net/riscv-port/pull/34/files Webrev: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=09 Stats: 1255 lines in 12 files changed: 1171 ins; 10 del; 74 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Fri, 7 Jan 2022 08:36:47 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
- Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - Remove assembler_riscv_c.hpp as discussions - Remove COMPRESSIBLE & NOT_COMPRESSIBLE macros by adding one layer as discussions - Fix remaining CEXT -> RVC - Remove Alignment-related changes as discussions - Update licenses to the new year - ... and 4 more: https://git.openjdk.java.net/riscv-port/compare/c7944edf...2a6ff151
src/hotspot/cpu/riscv/assembler_riscv.hpp line 2053:
2051: 2052: // RVC: extract a 16-bit instruction. 2053: static inline uint16_t c_extract(uint16_t val, unsigned msb, unsigned lsb) {
Looks like c_extract and c_sextract are not used? src/hotspot/cpu/riscv/assembler_riscv.hpp line 2409:
2407: } 2408: 2409: #define IF(BOOL, ...) IF_##BOOL(__VA_ARGS__)
maybe better to expand those macros directly? src/hotspot/cpu/riscv/riscv.ad line 1194:
1192: 1193: // RVC: With RVC a call may get 2-byte aligned. 1194: // The offset encoding in jal ranges bits [12, 31], which could span the cache line.
Maybe rephrasing like this: // The offset encoding in jal ranges bits [12, 31] could span the cache line. src/hotspot/cpu/riscv/riscv.ad line 1204:
1202: } 1203: 1204: // RVC: With RVC a call may get 2-byte aligned.
Looks like this comment dupliates the one for CallStaticJavaDirectNode::compute_padding. src/hotspot/cpu/riscv/vm_version_riscv.cpp line 109:
107: } 108: 109: // compressed instruction extension
I think this should better to place this one after UseRVV and UseRVB. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Sat, 8 Jan 2022 11:09:59 GMT, Fei Yang <fyang@openjdk.org> wrote:
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
- Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - Remove assembler_riscv_c.hpp as discussions - Remove COMPRESSIBLE & NOT_COMPRESSIBLE macros by adding one layer as discussions - Fix remaining CEXT -> RVC - Remove Alignment-related changes as discussions - Update licenses to the new year - ... and 4 more: https://git.openjdk.java.net/riscv-port/compare/c7944edf...2a6ff151
src/hotspot/cpu/riscv/assembler_riscv.hpp line 2053:
2051: 2052: // RVC: extract a 16-bit instruction. 2053: static inline uint16_t c_extract(uint16_t val, unsigned msb, unsigned lsb) {
Looks like c_extract and c_sextract are not used?
Thanks for pointing out this - this seems to be inherited from the original patch (the version that we decode instructions in `emit()`) and they seem to be not used anymore and I have removed them.
src/hotspot/cpu/riscv/riscv.ad line 1194:
1192: 1193: // RVC: With RVC a call may get 2-byte aligned. 1194: // The offset encoding in jal ranges bits [12, 31], which could span the cache line.
Maybe rephrasing like this: // The offset encoding in jal ranges bits [12, 31] could span the cache line.
done. Thanks for polishing the sentence.
src/hotspot/cpu/riscv/riscv.ad line 1204:
1202: } 1203: 1204: // RVC: With RVC a call may get 2-byte aligned.
Looks like this comment dupliates the one for CallStaticJavaDirectNode::compute_padding.
done.
src/hotspot/cpu/riscv/vm_version_riscv.cpp line 109:
107: } 108: 109: // compressed instruction extension
I think this should better to place this one after UseRVV and UseRVB.
done. Thanks for pointing out this. It is inherited from my original patch and it indeed needs to be moved downward to keep the consistency. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Sat, 8 Jan 2022 11:11:21 GMT, Fei Yang <fyang@openjdk.org> wrote:
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
- Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - Remove assembler_riscv_c.hpp as discussions - Remove COMPRESSIBLE & NOT_COMPRESSIBLE macros by adding one layer as discussions - Fix remaining CEXT -> RVC - Remove Alignment-related changes as discussions - Update licenses to the new year - ... and 4 more: https://git.openjdk.java.net/riscv-port/compare/c7944edf...2a6ff151
src/hotspot/cpu/riscv/assembler_riscv.hpp line 2409:
2407: } 2408: 2409: #define IF(BOOL, ...) IF_##BOOL(__VA_ARGS__)
maybe better to expand those macros directly?
Unfortunately, this could not be easily done by substituting by a simple `assert_cond(!CHECK_RD || Rd != x0);` because of some details: `c_ldsp` and `c_fldsp` use `Register` and `FloatRegister` separately so there is a type-related problem: x0 is only one `Register` and only `c_ldsp` needs this sanity check. So these macros here are trying to make sure this check isn't generated by `c_fldsp` at all because checking `Rd(a float register) != x0` will result in compilation errors. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 03:04:25 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
src/hotspot/cpu/riscv/assembler_riscv.hpp line 2409:
2407: } 2408: 2409: #define IF(BOOL, ...) IF_##BOOL(__VA_ARGS__)
maybe better to expand those macros directly?
Unfortunately, this could not be easily done by substituting by a simple `assert_cond(!CHECK_RD || Rd != x0);` because of some details: `c_ldsp` and `c_fldsp` use `Register` and `FloatRegister` separately so there is a type-related problem: x0 is only one `Register` and only `c_ldsp` needs this sanity check. So these macros here are trying to make sure this check isn't generated by `c_fldsp` at all because checking `Rd(a float register) != x0` will result in compilation errors.
Yes, I know the issue you are trying to resolve. Actually, I prefer to have two INSN macro definitions here: one for the integer case and one for the floating point case. That will be cleaner. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 03:17:48 GMT, Fei Yang <fyang@openjdk.org> wrote:
Unfortunately, this could not be easily done by substituting by a simple `assert_cond(!CHECK_RD || Rd != x0);` because of some details: `c_ldsp` and `c_fldsp` use `Register` and `FloatRegister` separately so there is a type-related problem: x0 is only one `Register` and only `c_ldsp` needs this sanity check. So these macros here are trying to make sure this check isn't generated by `c_fldsp` at all because checking `Rd(a float register) != x0` will result in compilation errors.
Yes, I know the issue you are trying to resolve. Actually, I prefer to have two INSN macro definitions here: one for the integer case and one for the floating point case. That will be cleaner.
done - thanks for the suggestions of splitting the two macros. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Fri, 7 Jan 2022 08:36:47 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
- Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - Remove assembler_riscv_c.hpp as discussions - Remove COMPRESSIBLE & NOT_COMPRESSIBLE macros by adding one layer as discussions - Fix remaining CEXT -> RVC - Remove Alignment-related changes as discussions - Update licenses to the new year - ... and 4 more: https://git.openjdk.java.net/riscv-port/compare/c7944edf...2a6ff151
src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp line 245:
243: // RVC: RISCV's amoswap instructions need an alignment for the memory address it swaps 244: // when we reach here we may get a 2-byte alignment so need to align it 245: __ align(4, nmethod_barrier_guard_offset());
Is it necessary to pass nmethod_barrier_guard_offset() as the "extra_offset" parameter here since this function will return (12 * instruction_size) which is multiple of 4 ? src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 2954:
2952: // 8 byte aligned after 3 intructions 2953: // RVC: when we reach here we may get a 2-byte alignment so need to align it 2954: align(wordSize, NativeCallTrampolineStub::data_offset);
Same question: Is there a reason to pass "NativeCallTrampolineStub::data_offset" as the "extra_offset" parameter here since this function will return (3 * NativeInstruction::instruction_size) which is multiple of 4 ? ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 01:20:58 GMT, Fei Yang <fyang@openjdk.org> wrote:
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
- Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - Remove assembler_riscv_c.hpp as discussions - Remove COMPRESSIBLE & NOT_COMPRESSIBLE macros by adding one layer as discussions - Fix remaining CEXT -> RVC - Remove Alignment-related changes as discussions - Update licenses to the new year - ... and 4 more: https://git.openjdk.java.net/riscv-port/compare/c7944edf...2a6ff151
src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp line 245:
243: // RVC: RISCV's amoswap instructions need an alignment for the memory address it swaps 244: // when we reach here we may get a 2-byte alignment so need to align it 245: __ align(4, nmethod_barrier_guard_offset());
Is it necessary to pass nmethod_barrier_guard_offset() as the "extra_offset" parameter here since this function will return (12 * instruction_size) which is multiple of 4 ?
Yes - it seems a little obscure though. 1. The background is that: about the `nmethod_entry_barrier`: nmethod_entry_barrier (begin) { <- when we reach here, we may get a 2-byte alignment because of RVC's existence. ... amoswap <the swapped address in the code segment> <- though the size of the whole stub is a multiple of 4 currently, here we may also get a 2-byte alignment because of the above issue. } nmethod_entry_barrier (end) and about the trampoline: trampoline (begin) { <- when we reach here, we may get a 2-byte alignment because of RVC's existence. auipc ld jalr <the real jump target in the code segment> <- same as the above explanation. } trampline (end) 2. The reason to pass another argument is that: we might consider just keeping the start address of the code slices (nmethod_entry_barrier and trampoline) aligned to 4-byte to solve this issue, but this assumption is based on 'the size of the code slices (nmethod_entry_barrier and trampoline themselves) are a multiple of 4', and we don't know if they will change in the future by RVC. So I explicitly pass the real target address to the `align` function as a new argument to make sure the code is always right in the future. I am pleased with other suggestions for this part. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 03:32:43 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp line 245:
243: // RVC: RISCV's amoswap instructions need an alignment for the memory address it swaps 244: // when we reach here we may get a 2-byte alignment so need to align it 245: __ align(4, nmethod_barrier_guard_offset());
Is it necessary to pass nmethod_barrier_guard_offset() as the "extra_offset" parameter here since this function will return (12 * instruction_size) which is multiple of 4 ?
Yes - it seems a little obscure though.
1. The background is that:
about the `nmethod_entry_barrier`:
nmethod_entry_barrier (begin) { <- when we reach here, we may get a 2-byte alignment because of RVC's existence. ... amoswap <the swapped address in the code segment> <- though the size of the whole stub is a multiple of 4 currently, here we may also get a 2-byte alignment because of the above issue. } nmethod_entry_barrier (end)
and about the trampoline:
trampoline (begin) { <- when we reach here, we may get a 2-byte alignment because of RVC's existence. auipc ld jalr <the real jump target in the code segment> <- same as the above explanation. } trampline (end)
2. The reason to pass another argument is that:
we might consider just keeping the start address of the code slices (nmethod_entry_barrier and trampoline) aligned to 4-byte to solve this issue, but this assumption is based on 'the size of the code slices (nmethod_entry_barrier and trampoline themselves) are a multiple of 4', and we don't know if they will change in the future by RVC. So I explicitly pass the real target address to the `align` function as a new argument to make sure the code is always right in the future.
I am pleased with other suggestions for this part.
For the nmethod_entry_barrier case, I would omit the extra_offset parameter (and also the nmethod_barrier_guard_offset funtion). I think your newly-added assertion at line 265 is enough. But looks like it's different in the trampoline case where I see we need a wordSize (8bytes) alignment. So we may keep the change for this case. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 06:46:59 GMT, Fei Yang <fyang@openjdk.org> wrote:
Yes - it seems a little obscure though.
1. The background is that:
about the `nmethod_entry_barrier`:
nmethod_entry_barrier (begin) { <- when we reach here, we may get a 2-byte alignment because of RVC's existence. ... amoswap <the swapped address in the code segment> <- though the size of the whole stub is a multiple of 4 currently, here we may also get a 2-byte alignment because of the above issue. } nmethod_entry_barrier (end)
and about the trampoline:
trampoline (begin) { <- when we reach here, we may get a 2-byte alignment because of RVC's existence. auipc ld jalr <the real jump target in the code segment> <- same as the above explanation. } trampline (end)
2. The reason to pass another argument is that:
we might consider just keeping the start address of the code slices (nmethod_entry_barrier and trampoline) aligned to 4-byte to solve this issue, but this assumption is based on 'the size of the code slices (nmethod_entry_barrier and trampoline themselves) are a multiple of 4', and we don't know if they will change in the future by RVC. So I explicitly pass the real target address to the `align` function as a new argument to make sure the code is always right in the future.
I am pleased with other suggestions for this part.
For the nmethod_entry_barrier case, I would omit the extra_offset parameter (and also the nmethod_barrier_guard_offset funtion). I think your newly-added assertion at line 265 is enough.
But looks like it's different in the trampoline case where I see we need a wordSize (8bytes) alignment. So we may keep the change for this case.
Yes, I agree with your point. That trampoline part is special and requires to be scrupulous to make changes. So maybe I'd better remove the `nmethod_barrier_guard_offset` part right? ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 06:57:05 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
For the nmethod_entry_barrier case, I would omit the extra_offset parameter (and also the nmethod_barrier_guard_offset funtion). I think your newly-added assertion at line 265 is enough.
But looks like it's different in the trampoline case where I see we need a wordSize (8bytes) alignment. So we may keep the change for this case.
Yes, I agree with your point. That trampoline part is special and requires to be scrupulous to make changes.
So maybe I'd better remove the `nmethod_barrier_guard_offset` part right?
Yes. Please also update the code comments. It looks strange to find those comments starts with a "RVC:". Also we should make the exact alignment required explicit in comments like: "RISCV's amoswap instructions need an alignment for the memory address it swaps" or "RISCV CAS needs an alignment for memory", etc. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 07:14:52 GMT, Fei Yang <fyang@openjdk.org> wrote:
Yes, I agree with your point. That trampoline part is special and requires to be scrupulous to make changes.
So maybe I'd better remove the `nmethod_barrier_guard_offset` part right?
Yes. Please also update the code comments. It looks strange to find those comments starts with a "RVC:". Also we should make the exact alignment required explicit in comments like: "RISCV's amoswap instructions need an alignment for the memory address it swaps" or "RISCV CAS needs an alignment for memory", etc.
done - polished and it needs some testing works. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Remove useless and polish comments ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/2a6ff151..70ddc0b3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=10 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=09-10 Stats: 46 lines in 4 files changed: 5 ins; 27 del; 14 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Split c_ldsp and c_fldsp ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/70ddc0b3..f9caca8d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=11 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=10-11 Stats: 25 lines in 1 file changed: 14 ins; 4 del; 7 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Remove `nmethod_entry_barrier`-related things ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/f9caca8d..c0c4416b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=12 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=11-12 Stats: 16 lines in 2 files changed: 0 ins; 13 del; 3 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Polish `nmethod_entry_barrier` and RISC-V CAS related comments ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/c0c4416b..aef57f3b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=13 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=12-13 Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 07:30:28 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
Polish `nmethod_entry_barrier` and RISC-V CAS related comments
Thanks for resolving the comments :-) We will need more tests for this patch, both in respects of functionality and performance. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 07:30:28 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
Polish `nmethod_entry_barrier` and RISC-V CAS related comments
I have tested this patch under whole `test/jdk` and `test/hotspot/jtreg` folders without Monday's change through last weekend on a Hifive Unleashed and only revealed [JDK-8279664](https://bugs.openjdk.java.net/browse/JDK-8279664), which has nothing to do with this patch, for the [RISC-V backend initial-load patch](https://github.com/openjdk/riscv-port/commit/7148c39832fe522bad91df77ef4b5c9...) could also reveal this problem on the Unleashed board I'm currently using. This one is a bit weird and I need more time to dig deep into that. About Monday's incremental changesets I tested `test/hotspot/jtreg/compiler`, `test/hotspot/jtreg/gc/z`, and `test/hotspot/jtreg/gc/shenandoah` with or without RVC and revealed no errors. Also SPECjbb2015 is tested and the result seems to have no regression found under RVC - the result on Unleashed remains a `max-jOPs` level of score 370 with minor fluctuations. I think maybe this patch could safely move forward -- except I need to take a look at JDK-8279664 to find whether it is a board-related issue because I cannot reproduce it on C910 and qemu. And thanks for the scrupulous reviews and the delicious suggestions, Felix and Yadong. :-) ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Wed, 12 Jan 2022 02:58:59 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
I have tested this patch under whole `test/jdk` and `test/hotspot/jtreg` folders without Monday's change through last weekend on a Hifive Unleashed and only revealed [JDK-8279664](https://bugs.openjdk.java.net/browse/JDK-8279664), which has nothing to do with this patch, for the [RISC-V backend initial-load patch](https://github.com/openjdk/riscv-port/commit/7148c39832fe522bad91df77ef4b5c9...) could also reveal this problem on the Unleashed board I'm currently using. This one is a bit weird and I need more time to dig deep into that.
About Monday's incremental changesets I tested `test/hotspot/jtreg/compiler`, `test/hotspot/jtreg/gc/z`, and `test/hotspot/jtreg/gc/shenandoah` with or without RVC and revealed no errors. Also SPECjbb2015 is tested and the result seems to have no regression found under RVC - the result on Unleashed remains a `max-jOPs` level of score 370 with minor fluctuations.
I think maybe this patch could safely move forward -- except I need to take a look at JDK-8279664 to find whether it is a board-related issue because I cannot reproduce it on C910 and qemu. And thanks for the scrupulous reviews and the delicious suggestions, Felix and Yadong. :-)
What about codesize improvement of the C2 JIT code? BTW: You need to rebase the patch on the latest repo. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 10 Jan 2022 07:30:28 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has refreshed the contents of this pull request, and previous commits have been removed. Incremental views are not available.
I have tested this patch under whole `test/jdk` and `test/hotspot/jtreg` folders without Monday's change through last weekend on a Hifive Unleashed and only revealed [JDK-8279664](https://bugs.openjdk.java.net/browse/JDK-8279664), which has nothing to do with this patch, for the [RISC-V backend initial-load patch](https://github.com/openjdk/riscv-port/commit/7148c39832fe522bad91df77ef4b5c9...) could also reveal this problem on the Unleashed board I'm currently using. This one is a bit weird and I need more time to dig deep into that. About Monday's incremental changesets I tested `test/hotspot/jtreg/compiler`, `test/hotspot/jtreg/gc/z`, and `test/hotspot/jtreg/gc/shenandoah` with or without RVC and revealed no errors. Also SPECjbb2015 is tested and the result seems to have no regression found under RVC - the result on Unleashed remains a `max-jOPs` level of score 370 with minor fluctuations. I think maybe this patch could safely move forward -- except I need to take a look at JDK-8279664 to find whether it is a board-related issue because I cannot reproduce it on C910 and qemu. And thanks for the scrupulous reviews and the delicious suggestions, Felix and Yadong. :-)
What about codesize improvement of the C2 JIT code? BTW: You need to rebase the patch on the latest repo.
BTW: You need to rebase the patch on the latest repo.
Thanks for the kind reminder - rebased and tested a simple `test/hotspot/jtreg/compiler` with/without RVC on qemu without errors found.
What about codesize improvement of the C2 JIT code? To be short and precise, currently just 'fair' but far from 'good', as the effect of [original patch](https://github.com/openjdk/riscv-port/pull/24).
This patch only enables the basic definitions of RVC and some scattered usages in the dot ad file, so its title is just a very simple 'riscv: RVC support' and I didn't mention its effect. But considering currently register spills and `pusha/popa`s are covered by this patch, which could cover many instructions (C2 and some stub code), so the result is not 'poor' though. With respect to this, I shall consider ways to further enable compressions for C1/C2's instructions to take it back to nearly a level of the original patch afterward by adding further patches. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 18 commits: - Polish `nmethod_entry_barrier` and RISC-V CAS related comments - Remove `nmethod_entry_barrier`-related things - Split c_ldsp and c_fldsp - Remove useless and polish comments - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - Remove assembler_riscv_c.hpp as discussions - ... and 8 more: https://git.openjdk.java.net/riscv-port/compare/67049937...176ae66f ------------- Changes: https://git.openjdk.java.net/riscv-port/pull/34/files Webrev: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=14 Stats: 1227 lines in 11 files changed: 1145 ins; 10 del; 72 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 19 commits: - Polish comments: remove the 'RVC:' prefix - Polish `nmethod_entry_barrier` and RISC-V CAS related comments - Remove `nmethod_entry_barrier`-related things - Split c_ldsp and c_fldsp - Remove useless and polish comments - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - ... and 9 more: https://git.openjdk.java.net/riscv-port/compare/a61d66c5...969a4d4d ------------- Changes: https://git.openjdk.java.net/riscv-port/pull/34/files Webrev: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=15 Stats: 1227 lines in 11 files changed: 1145 ins; 10 del; 72 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Thu, 13 Jan 2022 10:02:27 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 19 commits:
- Polish comments: remove the 'RVC:' prefix - Polish `nmethod_entry_barrier` and RISC-V CAS related comments - Remove `nmethod_entry_barrier`-related things - Split c_ldsp and c_fldsp - Remove useless and polish comments - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - ... and 9 more: https://git.openjdk.java.net/riscv-port/compare/a61d66c5...969a4d4d
lgtm ------------- Marked as reviewed by yadongwang (Author). PR: https://git.openjdk.java.net/riscv-port/pull/34
On Thu, 13 Jan 2022 10:02:27 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 19 commits:
- Polish comments: remove the 'RVC:' prefix - Polish `nmethod_entry_barrier` and RISC-V CAS related comments - Remove `nmethod_entry_barrier`-related things - Split c_ldsp and c_fldsp - Remove useless and polish comments - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - Manually inline all macros into functions as discussions - ... and 9 more: https://git.openjdk.java.net/riscv-port/compare/a61d66c5...969a4d4d
src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1329:
1327: // the address of jal itself (which will be patched later) should not span the cache line. 1328: // See CallStaticJavaDirectNode::compute_padding() for more info. 1329: __ align(4);
It's better to add some alignment asserts before the code that may be patched, for example, in MacroAssembler::emit_static_call_stub. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Sat, 15 Jan 2022 03:31:35 GMT, Yadong Wang <yadongwang@openjdk.org> wrote:
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase.
src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1329:
1327: // the address of jal itself (which will be patched later) should not span the cache line. 1328: // See CallStaticJavaDirectNode::compute_padding() for more info. 1329: __ align(4);
It's better to add some alignment asserts before the code that may be patched, for example, in MacroAssembler::emit_static_call_stub.
Thanks for the reasonable suggestion. Rebased and added the assertion with testing a fastdebug build under RVC for `test/jtreg/hotspot/compiler` without errors found. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits: - Add an assertion for patchable jals' alignment - Polish comments: remove the 'RVC:' prefix - Polish `nmethod_entry_barrier` and RISC-V CAS related comments - Remove `nmethod_entry_barrier`-related things - Split c_ldsp and c_fldsp - Remove useless and polish comments - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - ... and 10 more: https://git.openjdk.java.net/riscv-port/compare/d119eda6...3496b133 ------------- Changes: https://git.openjdk.java.net/riscv-port/pull/34/files Webrev: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=16 Stats: 1228 lines in 11 files changed: 1146 ins; 10 del; 72 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 20 additional commits since the last revision: - Add an assertion for patchable jals' alignment - Polish comments: remove the 'RVC:' prefix - Polish `nmethod_entry_barrier` and RISC-V CAS related comments - Remove `nmethod_entry_barrier`-related things - Split c_ldsp and c_fldsp - Remove useless and polish comments - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - ... and 10 more: https://git.openjdk.java.net/riscv-port/compare/83e32f9f...37899306 ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/3496b133..37899306 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=17 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=16-17 Stats: 1413 lines in 103 files changed: 898 ins; 276 del; 239 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 17 Jan 2022 03:09:25 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits:
- Add an assertion for patchable jals' alignment - Polish comments: remove the 'RVC:' prefix - Polish `nmethod_entry_barrier` and RISC-V CAS related comments - Remove `nmethod_entry_barrier`-related things - Split c_ldsp and c_fldsp - Remove useless and polish comments - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - ... and 10 more: https://git.openjdk.java.net/riscv-port/compare/f26f83ff...37899306
Still some suggestions about the comments, otherwise looks good. src/hotspot/cpu/riscv/assembler_riscv.hpp line 2030:
2028: // 2. RVC instructions in Assembler always begin with 'c_' prefix, as 'c_li', 2029: // but most of time we have no need to explicitly use these instructions. 2030: // 3. We introduce 'CompressibleRegion' to hint instructions in this Region's RTTI range
Suggestion: // 3. 'CompressibleRegion' is introduced to hint instructions in this Region's RTTI range src/hotspot/cpu/riscv/assembler_riscv.hpp line 2031:
2029: // but most of time we have no need to explicitly use these instructions. 2030: // 3. We introduce 'CompressibleRegion' to hint instructions in this Region's RTTI range 2031: // are qualified to change to their 2-byte versions.
// are qualified to be compressed with their 2-byte versions. src/hotspot/cpu/riscv/assembler_riscv.hpp line 2038:
2036: // 2037: // 4. Using -XX:PrintAssemblyOptions=no-aliases could print RVC instructions instead of 2038: // normal ones.
Suggestion: // 4. Using -XX:PrintAssemblyOptions=no-aliases could distinguish RVC instructions from // normal ones. src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1328:
1326: // With RVC a call may get 2-byte aligned. 1327: // the address of jal itself (which will be patched later) should not span the cache line. 1328: // See CallStaticJavaDirectNode::compute_padding() for more info.
Suggestion: // With RVC a call instruction (which will be patched later) may get 2-byte aligned and could // span multiple cache lines. See CallStaticJavaDirectNode::compute_padding() for more info. src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1352:
1350: void LIR_Assembler::emit_static_call_stub() { 1351: address call_pc = __ pc(); 1352: assert((__ offset() % 4) == 0, "call pc (patchable jals) must be aligned to maintain atomicity");
Suggestion: assert((__ offset() % 4) == 0, "call sites must be properly aligned"); src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp line 241:
239: } 240: 241: // RISCV's amoswap instructions need a 4-byte alignment for the 4-byte word it swaps in memory
Suggestion: // RISCV's amoswap instructions require that the memory address must be naturally aligned. src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp line 262:
260: __ bind(guard); 261: 262: assert(__ offset() % 4 == 0, "RISCV CAS needs a 4-byte alignment for the 4-byte word it swaps in memory");
Suggestion: assert(__ offset() % 4 == 0, "bad alignment"); src/hotspot/cpu/riscv/riscv.ad line 1197:
1195: // Patching this unaligned address will make the write operation not atomic. 1196: // Other threads may be running the same piece of code at full speed, causing concurrency issues. 1197: // So we must ensure that it does not span a cache line so that it can be patched.
Suggestion: // With RVC a call site may get 2-byte aligned. // The offset encoding in jal instruction bits [12, 31] could span multiple cache lines. // Patching this jal instruction will not be atomic when its address is not naturally aligned. // Other threads may be running the same piece of code at the same time, thus causing concurrency issues. ------------- Changes requested by fyang (Lead). PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 17 Jan 2022 11:42:28 GMT, Fei Yang <fyang@openjdk.org> wrote:
Xiaolin Zheng has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 20 additional commits since the last revision:
- Add an assertion for patchable jals' alignment - Polish comments: remove the 'RVC:' prefix - Polish `nmethod_entry_barrier` and RISC-V CAS related comments - Remove `nmethod_entry_barrier`-related things - Split c_ldsp and c_fldsp - Remove useless and polish comments - Move RVC code to the proper location after rebasing (#42) - Rename misc functions and change the positions of some comments - Remove remaining macros as discussions - Remain an 'minimum_alignment' unchanged - ... and 10 more: https://git.openjdk.java.net/riscv-port/compare/0e400fce...37899306
src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1328:
1326: // With RVC a call may get 2-byte aligned. 1327: // the address of jal itself (which will be patched later) should not span the cache line. 1328: // See CallStaticJavaDirectNode::compute_padding() for more info.
Suggestion: // With RVC a call instruction (which will be patched later) may get 2-byte aligned and could // span multiple cache lines. See CallStaticJavaDirectNode::compute_padding() for more info.
Thanks for spending time polishing my poor English :-) Revised all as suggestions, and it looks nicer than before. ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision: Polish English comments as suggestions ------------- Changes: - all: https://git.openjdk.java.net/riscv-port/pull/34/files - new: https://git.openjdk.java.net/riscv-port/pull/34/files/37899306..502ff3d8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=18 - incr: https://webrevs.openjdk.java.net/?repo=riscv-port&pr=34&range=17-18 Stats: 14 lines in 4 files changed: 0 ins; 2 del; 12 mod Patch: https://git.openjdk.java.net/riscv-port/pull/34.diff Fetch: git fetch https://git.openjdk.java.net/riscv-port pull/34/head:pull/34 PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 17 Jan 2022 12:30:43 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
Polish English comments as suggestions
Approved. Thanks. ------------- Marked as reviewed by fyang (Lead). PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 17 Jan 2022 12:30:43 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
Polish English comments as suggestions
Thank you for the nice reviews, Felix and Yadong! ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
On Mon, 20 Dec 2021 08:10:10 GMT, Xiaolin Zheng <xlinzheng@openjdk.org> wrote:
Hi team,
This patch includes the basic definition of the RVC instruction set and some cleanups. Tested a simple `test/hotspot/jtreg/compiler/` folder on qemu.
Using `<JAVA_HOME>/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseRVC -XX:+UnlockDiagnosticVMOptions -XX:+PrintAssembly -XX:PrintAssemblyOptions=no-aliases,numeric -XX:+PrintStubCode -XX:-TieredCompilation` could show RVC instructions.
Thanks, Xiaolin
This pull request has now been integrated. Changeset: fe82bcc1 Author: Xiaolin Zheng <xlinzheng@openjdk.org> Committer: Yadong Wang <yadongwang@openjdk.org> URL: https://git.openjdk.java.net/riscv-port/commit/fe82bcc14bbe64d4fc568aa29f01d... Stats: 1226 lines in 11 files changed: 1144 ins; 10 del; 72 mod 8278994: riscv: RVC support Co-authored-by: Wei Kuai <kuaiwei.kw@alibaba-inc.com> Reviewed-by: fyang, yadongwang ------------- PR: https://git.openjdk.java.net/riscv-port/pull/34
participants (3)
-
Fei Yang
-
Xiaolin Zheng
-
Yadong Wang