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

Allow RISC-V ABI to be configured by mabi #4322

Merged
merged 8 commits into from
Feb 14, 2023

Conversation

zyedidia
Copy link
Contributor

@zyedidia zyedidia commented Feb 12, 2023

This PR allows the RISC-V ABI to be configured by the user. RISC-V has the following ABIs: lp64, lp64f, lp64d (64-bit only), and ilp32, ilp32f, ilp32d (32-bit only). See here for a slightly more detailed description, and here for the GCC documentation on this.

The latest LDC release changed the default 64-bit ABI from lp64 to lp64d. I think the default should continue to be lp64 because it is the most basic, and additional features/extensions should be opt-in (this PR changes back to lp64 default). The latest release also enabled the D extension by default. This PR undoes that and leaves the D extension as opt-in (with -mattr=+d), just like every other extension.

The ABI is now configurable using -mabi=... instead of hard-coding to lp64d. The mabi option is also no longer hidden. The default ABI on riscv64 is now lp64 and on riscv32 is ilp32. Also, the backend linker now gets passed the appropriate mabi flags and march flags. Instead of hard-coding to mabi=lp64d and march=rv64gc (only for 64-bit though), now the value passed via mabi is used (or the default ABI), and march is properly constructed from the enabled features (passed via mattr). For example, to compile for rv64gc (shorthand for rv64imacfdc_zicsr_zifencei) with lp64d one would now pass:

-mtriple=riscv64-unknown-elf -mattr=+m,+a,+c,+f,+d,+c -mabi=lp64d

The backend linker/gcc driver would be passed -march=rv64gc -mabi=lp64d. Here are some other examples:

-mtriple=riscv32-unknown-elf -mattr=+m,+a,+c -mabi=ilp32
gcc/linker flags: -march=rv32imac_zicsr_zifencei -mabi=ilp32
-mtriple=riscv64-unknown-elf -mattr=+m,+a,+c,+f -mabi=lp64f
gcc/linker flags: -march=rv64imafc_zicsr_zifencei -mabi=lp64f
-mtriple=riscv64-unknown-elf
gcc/linker flags: -march=rv64i_zicsr_zifencei -mabi=lp64

Since Phobos probably needs to be built with rv64gc and lp64d the flags used to invoke the build should be

-mtriple=riscv64-unknown-elf -mattr=+m,+a,+c,+f,+d,+c -mabi=lp64d

I'm not sure if code in other repos needs to be changed as a result of this.

Fixes #4321.

driver/tool.cpp Outdated Show resolved Hide resolved
driver/tool.cpp Outdated Show resolved Hide resolved
driver/tool.cpp Outdated Show resolved Hide resolved
@JohanEngelen
Copy link
Member

LGTM

zyedidia and others added 2 commits February 14, 2023 11:35
Co-authored-by: Johan Engelen <jbc.engelen@gmail.com>
@JohanEngelen
Copy link
Member

It's OK like this, but you could merge all lit tests into one file, and then use --check-prefix to have different CHECKs per FileCheck invocation.

@JohanEngelen JohanEngelen enabled auto-merge (squash) February 14, 2023 19:49
@JohanEngelen JohanEngelen merged commit 7f2498b into ldc-developers:master Feb 14, 2023
@andreas-schwab
Copy link
Contributor

This probably breaks building druntime.

core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs0, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs1, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs2, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs3, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs4, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs5, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs6, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs7, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs8, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs9, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs10, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs11, 0(a2)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs0, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs1, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs2, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs3, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs4, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs5, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs6, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs7, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs8, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs9, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs10, 0(a0)
^
core/thread/osthread.d(1594):1:2: error: instruction requires the following: 'D' (Double-Precision Floating-Point)
fsd fs11, 0(a0)
^
make[2]: *** [runtime/CMakeFiles/druntime-ldc-shared.dir/build.make:346: runtime/objects-shared/core/atomic.o] Error 1

@zyedidia
Copy link
Contributor Author

I think the builder should just be updated to compile with -mattr=+m,+a,+c,+f,+d,+c -mabi=lp64d.

@andreas-schwab
Copy link
Contributor

When configured for riscv64 the default should always be rv64gc.

@zyedidia
Copy link
Contributor Author

I think that would be reasonable if there were a way to specify the exact RISC-V extensions to use using a flag like -march. This is how Clang operates: the default (if no flags are passed) is rv64imac for bare-metal targets (riscv64-unknown-elf), and rv64gc for hosted targets (riscv64-linux-elf), but -march can be used to override the default and specify rv64i, rv64gc, or something else.

Currently with LDC, if the default were changed to be rv64gc, then to use rv64im, the user would have to pass -mattr=-a,-c,-f,-d,-c, which is un-ergonomic and strange because it relies on the default and specifies the extensions to remove from the default instead of the ones to include from the base. If we make -march work like Clang, then I would be in favor of changing the default to rv64gc or rv64imac.

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.

No options to control the RISC-V ABI
4 participants