Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for RV64E and LP64E #52

Merged
merged 1 commit into from
Nov 27, 2023
Merged

Add support for RV64E and LP64E #52

merged 1 commit into from
Nov 27, 2023

Conversation

a4lg
Copy link
Contributor

@a4lg a4lg commented Oct 22, 2023

__riscv_64e is defined analogously to __riscv_32e.

Changing the meaning of __riscv_32e should not cause any problems since if we are just testing for existence of the E extension, there is __riscv_e preprocessor macro.

wangliu-iscas pushed a commit to plctlab/patchwork-gcc that referenced this pull request Oct 22, 2023
@a4lg @ouuleilei-bot
RISC-V: Initial RV64E and LP64E support
5c98efb 
Along with RV32E, RV64E is ratified.  Though ILP32E and LP64E ABIs are
still draft, it's worth supporting it.

This commit should not be merged until two proposals below are
going to proceed.

LP64E proposal (including suggested changes):
<riscv-non-isa/riscv-elf-psabi-doc#299>

New "__riscv_64e" proposal by the author of this commit:
<riscv-non-isa/riscv-c-api-doc#52>

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc
	(riscv_subset_list::parse_std_ext): Allow RV64E.
	* config.gcc: Parse base ISA RV64E and ABI LP64E.
	* config/riscv/arch-canonicalize: Parse base ISA 'rv64e'.
	* config/riscv/riscv-c.cc (riscv_cpu_cpp_builtins):
	Build different macro per RV32E/RV64E.
	Add handling for ABI_LP64E.
	* config/riscv/riscv-d.cc (riscv_d_handle_target_float_abi):
	Add handling for ABI_LP64E.
	* config/riscv/riscv-opts.h (enum riscv_abi_type): Add ABI_LP64E.
	* config/riscv/riscv.cc (riscv_option_override): Enhance error
	handling to support RV64E and LP64E.
	(riscv_conditional_register_usage): Change "RV32E" in a comment
	to "RV32E/RV64E".
	* config/riscv/riscv.h
	(UNITS_PER_FP_ARG): Add handling for ABI_LP64E.
	(STACK_BOUNDARY): Ditto.
	(ABI_STACK_BOUNDARY): Ditto.
	(MAX_ARGS_IN_REGISTERS): Ditto.
	(ABI_SPEC): Add support for "lp64e".
	* config/riscv/riscv.opt: Parse -mabi=lp64e as ABI_LP64E.
	* doc/invoke.texi: Add documentation of the LP64E ABI.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/predef-1.c: Test for __riscv_64e.
	* gcc.target/riscv/predef-2.c: Ditto.
	* gcc.target/riscv/predef-3.c: Ditto.
	* gcc.target/riscv/predef-4.c: Ditto.
	* gcc.target/riscv/predef-5.c: Ditto.
	* gcc.target/riscv/predef-6.c: Ditto.
	* gcc.target/riscv/predef-7.c: Ditto.
	* gcc.target/riscv/predef-8.c: Ditto.
	* gcc.target/riscv/predef-9.c: New test for RV32E and LP64E,
	based on predef-7.c.
@cmuellner
Copy link
Collaborator

Current status is:

  • GCC exposes __riscv_32e if RVE is enabled (regardless which XLEN is in use). Afaik RVE is limited to rv32 in GCC at the moment, so this change won't break anything.
  • LLVM exposes __riscv_abi_rve if ABIName == "ilp32e" (see clang/lib/Basic/Targets/RISCV.cpp).

The referenced GCC patch looks good.
We probably want to have a equal behavior in GCC and LLVM (we could define __riscv_abi macros in GCC and define __riscv_*e macros in LLVM).

In general, I am fine with the change.

a4lg added a commit to a4lg/gcc that referenced this pull request Oct 23, 2023
@a4lg
RISC-V: Initial RV64E and LP64E support
72be4bc 
Along with RV32E, RV64E is ratified.  Though ILP32E and LP64E ABIs are
still draft, it's worth supporting it.

This commit should not be merged until two proposals below are
going to proceed.

LP64E proposal (including suggested changes):
<riscv-non-isa/riscv-elf-psabi-doc#299>

New "__riscv_64e" proposal by the author of this commit:
<riscv-non-isa/riscv-c-api-doc#52>

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc
	(riscv_subset_list::parse_std_ext): Allow RV64E.
	* config.gcc: Parse base ISA 'rv64e' and ABI 'lp64e'.
	* config/riscv/arch-canonicalize: Parse base ISA 'rv64e'.
	* config/riscv/riscv-c.cc (riscv_cpu_cpp_builtins):
	Define different macro per XLEN.  Add handling for ABI_LP64E.
	* config/riscv/riscv-d.cc (riscv_d_handle_target_float_abi):
	Add handling for ABI_LP64E.
	* config/riscv/riscv-opts.h (enum riscv_abi_type): Add ABI_LP64E.
	* config/riscv/riscv.cc (riscv_option_override): Enhance error
	handling to support RV64E and LP64E.
	(riscv_conditional_register_usage): Change "RV32E" in a comment
	to "RV32E/RV64E".
	* config/riscv/riscv.h
	(UNITS_PER_FP_ARG): Add handling for ABI_LP64E.
	(STACK_BOUNDARY): Ditto.
	(ABI_STACK_BOUNDARY): Ditto.
	(MAX_ARGS_IN_REGISTERS): Ditto.
	(ABI_SPEC): Add support for "lp64e".
	* config/riscv/riscv.opt: Parse -mabi=lp64e as ABI_LP64E.
	* doc/invoke.texi: Add documentation of the LP64E ABI.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/predef-1.c: Test for __riscv_64e.
	* gcc.target/riscv/predef-2.c: Ditto.
	* gcc.target/riscv/predef-3.c: Ditto.
	* gcc.target/riscv/predef-4.c: Ditto.
	* gcc.target/riscv/predef-5.c: Ditto.
	* gcc.target/riscv/predef-6.c: Ditto.
	* gcc.target/riscv/predef-7.c: Ditto.
	* gcc.target/riscv/predef-8.c: Ditto.
	* gcc.target/riscv/predef-9.c: New test for RV64E and LP64E,
	based on predef-7.c.
@a4lg
Add support for RV64E and LP64E
882b242 
`__riscv_64e` is defined analogously to `__riscv_32e`.

Changing the meaning of `__riscv_32e` should not cause any problems
since if we are just testing for existence of the `E` extension,
there is `__riscv_e` preprocessor macro.
@a4lg a4lg changed the title Add new macro for RV64E Add support for RV64E and LP64E Oct 23, 2023
@a4lg
Copy link
Contributor Author

a4lg commented Oct 23, 2023

Additional changes:

Added LP64E ABI references to the definitions of __riscv_abi_rve and __riscv_float_abi_soft.

@a4lg
Copy link
Contributor Author

a4lg commented Oct 23, 2023

@cmuellner
Seems GCC also defines __riscv{,_float}_abi macros (cf. riscv_cpu_cpp_builtins function in gcc/config/riscv/riscv-c.cc).

@pz9115
Copy link

pz9115 commented Oct 24, 2023

LGTM, we also need the LLVM community comments.

@wangpc-pp
Copy link
Contributor

The LLVM implementation (https://reviews.llvm.org/D70401) is the same as the proposal, so I will give a LGTM. Thanks! :-)

@cmuellner
Copy link
Collaborator

Are there patches for GCC available?

@a4lg
Copy link
Contributor Author

a4lg commented Nov 21, 2023

@cmuellner
My RFC PATCH submission:
https://gcc.gnu.org/pipermail/gcc-patches/2023-October/633846.html
and corresponding commit:
a4lg/gcc@72be4bc

I'll rebase against the latest master and make a ping, removing "RFC" status.

@cmuellner
Copy link
Collaborator

@kito-cheng: any objections to merging this?
The change here is straight-forward and we have patches.
The ABI side (riscv-non-isa/riscv-elf-psabi-doc#299) is not finalized, but I think this won't have an influence on this PR.

kito-cheng
kito-cheng approved these changes Nov 21, 2023
View reviewed changes
Copy link
Collaborator

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have __riscv_e to check e is enabled for both rv64 or rv32, but I think it's OK to add macro for symmetric, so LGTM

@a4lg
Copy link
Contributor Author

a4lg commented Nov 27, 2023

Since this proposal and my GCC patch set (with a bit of modification from RFC PATCH) are approved, GCC's RV64E/LP64E support is merged into its master/trunk.

@cmuellner
Copy link
Collaborator

Ok, so let's merge this.

Thanks!

@cmuellner cmuellner merged commit 16c8466 into riscv-non-isa:master Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kito-cheng kito-cheng kito-cheng approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@a4lg @cmuellner @pz9115 @wangpc-pp @kito-cheng