rustc_target: Add RISC-V atomic-related features #130877
Conversation
|
Some changes occurred in tests/ui/check-cfg cc @Urgau |
rustbot
added
S-waiting-on-review
According to d9715c698c4940e65e9bd036e22fb5cab3aa2c6f,
I think that we still need a check like this? |
Thanks for pointing this out. If we go in the same direction as LLVM, we certainly need to do that. That said, I'm beginning to think that it is not actually necessary to separate what the ISA states ".. depends upon ..". |
taiki-e
force-pushed
the
riscv-atomic
branch
from
afeb3d4
to
86a8893
Compare
|
Updated to make |
|
I got some more context on why LLVM is the way it is. The RISC-V ISA spec is a hardware spec not a toolchain spec, so we must understand this context. The A extension was later split into Zaamo and Zalrsc. When hardware implements Zaamo and Zalrsc, it implemented A. End of story on the hardware side. But on the toolchain side, things are a little messy, especially when we are dealing with mixed versions of software in the toolchain. If This is the reason why LLVM does not have the implication. GCC + binutils usually ship with the same version, so this incompatibility is less of a worry. This is why the GCC folks were willing to add the implication here. LLVM decided that it would enforce that if you're going to use zacas or zabha, then you better enable a or zaamo, which ensures that we have what is needed without breaking what I talked about above. I don't want to be the one to say how it should be in rust, but I figured that this context could be valuable for those who are making the decision. |
|
☔ The latest upstream changes (presumably #130934) made this pull request unmergeable. Please resolve the merge conflicts. |
taiki-e
force-pushed
the
riscv-atomic
branch
from
86a8893
to
62612af
Compare
|
My understanding is that this may cause issues with an old binutils, but I believe newer binutils only warn instead of error when encountering unknown features. In any case, we can revert this and re-evaluate if it causes problems in nightly. @bors r+ |
bors
added
S-waiting-on-bors
Rollup of 5 pull requests Successful merges: - rust-lang#129638 (Hook up std::net to wasi-libc on wasm32-wasip2 target) - rust-lang#130877 (rustc_target: Add RISC-V atomic-related features) - rust-lang#130914 (Mark some more types as having insignificant dtor) - rust-lang#130961 (Enable `f16` tests on x86 Apple platforms) - rust-lang#130966 (make ptr metadata functions callable from stable const fn) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130877 - taiki-e:riscv-atomic, r=Amanieu rustc_target: Add RISC-V atomic-related features This adds the following three target features to unstable riscv_target_feature. - `zaamo` (Zaamo Extension 1.0.0): Atomic Memory Operations (`amo*.{w,d}{,.aq,.rl,.aqrl}`) ([definition in LLVM](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L229-L231), [available since LLVM 19](llvm/llvm-project@8be079c)) - `zabha` (Zabha Extension 1.0.0): Byte and Halfword Atomic Memory Operations (`amo*.{b,h}{,.aq,.rl,.aqrl}`) ([definition in LLVM](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L238-L240), [available since LLVM 19](llvm/llvm-project@6b74449)) - `zalrsc` (Zalrsc Extension 1.0.0): Load-Reserved/Store-Conditional Instructions (`lr.{w,d}{,.aq,.rl,.aqrl}` and `sc.{w,d}{,.aq,.rl,.aqrl}`) ([definition in LLVM](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L261-L263), [available since LLVM 19](llvm/llvm-project@8be079c)) (Zacas Extension is not included here because it is still marked as experimental in LLVM 19 llvm/llvm-project@70e7d26 and will become non-experimental in LLVM 20 llvm/llvm-project@614aeda) `a` implies `zaamo` and `zalrsc`, and `zabha` implies `zaamo`: - After Zaamo and Zalrsc Extensions are frozen, riscv-isa-manual says "The A extension comprises instructions provided by the Zaamo and Zalrsc extensions" (riscv/riscv-isa-manual@e87412e), and [`a` implies `zaamo` and `zalrsc` in GCC](https://github.com/gcc-mirror/gcc/blob/08693e29ec186fd7941d0b73d4d466388971fe2f/gcc/config/riscv/arch-canonicalize#L44). However, in LLVM, [`a` does not define them as implying `zaamo` and `zalrsc`](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L206). - Zabha and Zaamo are in a similar situation, [riscv-isa-manual](https://github.com/riscv/riscv-isa-manual/blob/main/src/zabha.adoc) says "The Zabha extension depends upon the Zaamo standard extension", and [`zabha` implies `zaamo` in GCC](https://github.com/gcc-mirror/gcc/blob/08693e29ec186fd7941d0b73d4d466388971fe2f/gcc/config/riscv/arch-canonicalize#L45-L46), but [does not in LLVM (but enabling `zabha` without `zaamo` or `a` is not allowed)](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/TargetParser/RISCVISAInfo.cpp#L776-L778). r? `@Amanieu` `@rustbot` label +O-riscv +A-target-feature
This adds the following three target features to unstable riscv_target_feature.
zaamo(Zaamo Extension 1.0.0): Atomic Memory Operations (amo*.{w,d}{,.aq,.rl,.aqrl})(definition in LLVM, available since LLVM 19)
zabha(Zabha Extension 1.0.0): Byte and Halfword Atomic Memory Operations (amo*.{b,h}{,.aq,.rl,.aqrl})(definition in LLVM, available since LLVM 19)
zalrsc(Zalrsc Extension 1.0.0): Load-Reserved/Store-Conditional Instructions (lr.{w,d}{,.aq,.rl,.aqrl}andsc.{w,d}{,.aq,.rl,.aqrl})(definition in LLVM, available since LLVM 19)
(Zacas Extension is not included here because it is still marked as experimental in LLVM 19 llvm/llvm-project@70e7d26 and will become non-experimental in LLVM 20 llvm/llvm-project@614aeda)
aimplieszaamoandzalrsc, andzabhaimplieszaamo:aimplieszaamoandzalrscin GCC. However, in LLVM,adoes not define them as implyingzaamoandzalrsc.zabhaimplieszaamoin GCC, but does not in LLVM (but enablingzabhawithoutzaamoorais not allowed).r? @Amanieu
@rustbot label +O-riscv +A-target-feature