-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Add new Tier-3 target: riscv64a23-unknown-linux-gnu #145076
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
base: master
Are you sure you want to change the base?
Add new Tier-3 target: riscv64a23-unknown-linux-gnu #145076
Conversation
These commits modify compiler targets. Some changes occurred in src/doc/rustc/src/platform-support cc @Noratrieb |
This comment has been minimized.
This comment has been minimized.
f77bc29
to
577b85a
Compare
src/doc/rustc/src/platform-support/riscv64a23-unknown-linux-gnu.md
Outdated
Show resolved
Hide resolved
src/doc/rustc/src/platform-support/riscv64a23-unknown-linux-gnu.md
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tier 3 targets do not get built nor tested in rust-lang/rust CI, and by implication, no prebuilt artifacts will be distributed for Tier 3 targets.
src/doc/rustc/src/platform-support/riscv64a23-unknown-linux-gnu.md
Outdated
Show resolved
Hide resolved
src/doc/rustc/src/platform-support/riscv64a23-unknown-linux-gnu.md
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #145077) made this pull request unmergeable. Please resolve the merge conflicts. |
64b98ad
to
3ec3eea
Compare
This comment has been minimized.
This comment has been minimized.
3ec3eea
to
1be21d3
Compare
Some changes occurred in src/tools/clippy cc @rust-lang/clippy This PR modifies This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. This PR modifies If appropriate, please update |
1be21d3
to
87fd289
Compare
This will run all tests for `riscv64a23-unknown-linux-gnu` in a QEMU instance.
…u.md Co-authored-by: zachs18 <[email protected]>
This reverts commit 289688c.
Ready for review. @zachs18 @Noratrieb @jieyouxu |
I personally recommend starting from RVA23U64 (excluding RVA23S64) and discuss possibilities extending it later. |
Here are my thoughts:
|
r? compiler_leads |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ZhongyaoChen Okay, for the "Supm" extension, that's convincing for me. So, the remaining subjects I strongly oppose are RVA23S64 and its extensions (all new extensions starting with either
|
@a4lg good point, changes applied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I request complete removal of RVA23S64 from this submission, not just from the target spec (including all RVA23S64-specific supervisor mode extensions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. My minimal requirements are satisfied and seems almost ready for submission (we need to ask @Amanieu for the "Supm" extension though).
Hi, This PR has been open for two weeks. All changes requested applied. |
The problem with Supm is that it doesn't actually enable any instructions or features on its own, so it is rather meaningless. I therefore don't think we should expose it as a user-visible target feature. Additionally, I checked and LLVM currently doesn't make use of it at all internally. We may revisit this once LLVM actually makes use of the Supm feature. |
From my understanding, it's meant to be a constraint on the execution environment, right? So, that would mean the user environment needs to provide the pointer masking. Maybe conformance with I'm deferring to your and LLVM's opinion on this, though. |
The problem is that "some form of pointer masking" isn't actually enough information to actually do anything. It doesn't tell you how many bits are masked, how to enable/disable masking, etc. |
As a sidenote:
So my personal questions are:
|
The pointer mask width is variable, depending on the virtual address width (Sv57, Sv48, Sv39), which represents the max, but a smaller width can be used in practice (e.g., Sv48 allows 16 bits, but we can only use 8 bits). Additionally, this kernel patch specifies that userspace can enable/set/ask the number of mask/tag bits needed by the application using a syscall.
Agreed. But there's an extra issue: rva23u64 implies Supm support. In Rust, removing Supm but keeping rva23u64 is odd; passing rva23u64 to LLVM means Supm is needed, whether or not it's visible in Rust target features.
@a4lg Since LLVM doesn’t use Supm yet, sorry, these questions are unclear now. |
This is what I meant about the |
I think this convention is appropriate.
Yeah, supm is meaningless for now, untill someone apply Supm to HWASAN in LLVM. I still suggest keeping Supm here; otherwise, it could conflict with the RVA23 spec and be inconsistent with other compilers. |
I agree about inconsistency but disagree about conflicting claim. As I mentioned earlier, RVA23 profiles only constrain the execution environment so that any program which uses a subset of RVA23(e.g. U64) is guaranteed to run on RVA23(U64) environment. Easier example would be RVA22U64 or RV64GC programs that will run on an RVA23U64-compliant environment (RVA23U64 is a superset of RVA22U64 and almost a superset of RV64GC except the Zifencei extension). Important thing here is: Rust compiler is not an execution environment. |
@a4lg |
I think I have a better understand of Supm now. Essentially what it means is that, if Supm is reported as supported on a Linux system then it directly implies the presence of the syscall support. So it actually does provide enough information for people to work with. In consideration of that I will reverse my previous stance on Supm: I think it's fine to expose it here and in the runtime feature detection as well. |
MCP: Tier 3 target proposal: riscv64a23-unknown-linux-gnu
Changes: