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.

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

Add the LP64E ABI, to support RV64E #299

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

palmer-dabbelt
Copy link
Contributor

This is just a quick copy of ILP32E, not sure if I missed something.

Signed-off-by: Palmer Dabbelt palmer@rivosinc.com

This is just a quick copy of ILP32E, not sure if I missed something.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
Comment on lines +249 to +252
IMPORTANT: RV32E and RV64E are not ratified base ISAs and so we cannot
guarantee the stability of ILP32E or LP64E, in contrast with the rest of this
document. This documents the current ILP32E implementation in GCC as of the
time of writing, but may be subject to change.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
IMPORTANT: RV32E and RV64E are not ratified base ISAs and so we cannot
guarantee the stability of ILP32E or LP64E, in contrast with the rest of this
document. This documents the current ILP32E implementation in GCC as of the
time of writing, but may be subject to change.
IMPORTANT: RV32E and RV64E are not ratified base ISAs and so we cannot
guarantee their stability, in contrast with the rest of this
document. This documents the current ILP32E implementation in GCC as of the
time of writing, but may be subject to change.

the following differences. The stack pointer need only be aligned to a 32-bit
boundary.

The LP64E calling convention is designed to be usable with the RV32E ISA. This
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The LP64E calling convention is designed to be usable with the RV32E ISA. This
The LP64E calling convention is designed to be usable with the RV64E ISA. This

@kito-cheng kito-cheng added this to the Post 1.0 milestone Jul 21, 2022
@kito-cheng
Copy link
Collaborator

Generally LGTM, but this will pending until 1.0 release :)

@@ -246,17 +246,24 @@ provided they hold values no more than FLEN bits wide.

=== ILP32E Calling Convention
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
=== ILP32E Calling Convention
=== ILP32E and LP64E Calling Conventions

@asb
Copy link
Collaborator

asb commented Feb 8, 2023

Quick drive-by comment: The sentence "Tag_RISCV_stack_align records the N-byte stack alignment for this object. The default value is 16 for RV32I or RV64I, and 4 for RV32E." in riscv-elf.adoc should be expanded to cover RV64E.

@palmer-dabbelt
Copy link
Contributor Author

Quick drive-by comment: The sentence "Tag_RISCV_stack_align records the N-byte stack alignment for this object. The default value is 16 for RV32I or RV64I, and 4 for RV32E." in riscv-elf.adoc should be expanded to cover RV64E.

Thanks. I guess it'd be 8-byte aligned for RV64E, as that's the same tradeoff we made for RV32E (ie, penalize Q in favor of everything else).

Did the RV64E stuff actually happen? The original announcement was kind of weird, I'm not sure if it was on purpose.

@asb
Copy link
Collaborator

asb commented Feb 8, 2023

Did the RV64E stuff actually happen? The original announcement was kind of weird, I'm not sure if it was on purpose.

Per the recently ratified extension list it was ratified in January, so it looks like it's official, opening the question of what's next for the RV32E and RV64E ABIs!

@palmer-dabbelt
Copy link
Contributor Author

Did the RV64E stuff actually happen? The original announcement was kind of weird, I'm not sure if it was on purpose.

Per the recently ratified extension list it was ratified in January, so it looks like it's official, opening the question of what's next for the RV32E and RV64E ABIs!

I guess we should probably get this finished, then. Have you guys had any interest in RV64E on the LLVM side of things?

@asb
Copy link
Collaborator

asb commented Feb 8, 2023

Have you guys had any interest in RV64E on the LLVM side of things?

I've been giving some guidance to a colleague who saw it was a gap vs the ratified extensions and thought it might be interesting to sort it (initially just RV64E at the MC/assembler layer). But no external demand.

@a4lg
Copy link

a4lg commented Jul 25, 2023

Since RV32E and RV64E are ratified and psABI 1.0 is already released, I think it's a good time to merge this.

Also, we should move forward to ratify ILP32E and LP64E ABIs.

Comment on lines +249 to +252
IMPORTANT: RV32E and RV64E are not ratified base ISAs and so we cannot
guarantee the stability of ILP32E or LP64E, in contrast with the rest of this
document. This documents the current ILP32E implementation in GCC as of the
time of writing, but may be subject to change.
Copy link

Choose a reason for hiding this comment

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

Suggested change
IMPORTANT: RV32E and RV64E are not ratified base ISAs and so we cannot
guarantee the stability of ILP32E or LP64E, in contrast with the rest of this
document. This documents the current ILP32E implementation in GCC as of the
time of writing, but may be subject to change.

Since RV32E and RV64E are ratified, this section is no longer necessary.

Copy link
Collaborator

Choose a reason for hiding this comment

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

RV32E and RV64E are ratified, but we shouldn't instantly mark ILP32E and LP64E as being finalised. For instance, it looks like we don't properly document the alignment on ILP32E for 2xlen types - where I believe only on the stack these types are 4-byte aligned in GCC? (The note about the stack being 4-byte aligned and the D extension not being compatible is relevant - but you could have a 4 byte stack and require it to be realigned for stack objects that need greater alignment).

wangliu-iscas pushed a commit to plctlab/patchwork-gcc that referenced this pull request Oct 22, 2023
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.
a4lg added a commit to a4lg/gcc that referenced this pull request Oct 23, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants