-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[RISCV] Emitting proper atomic ABI tag when Zalasr is enabled #121017
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
Conversation
|
@llvm/pr-subscribers-backend-risc-v Author: Brendan Sweeney (mehnadnerd) ChangesWhen Zalasr is enabled, it will emit the A7 atomic ABI tag. Full diff: https://github.com/llvm/llvm-project/pull/121017.diff 1 Files Affected:
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
index 99f57f47835abd..6035dffaa8ac8e 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
@@ -86,9 +86,11 @@ void RISCVTargetStreamer::emitTargetAttributes(const MCSubtargetInfo &STI,
if (RiscvAbiAttr && STI.hasFeature(RISCV::FeatureStdExtA)) {
unsigned AtomicABITag = static_cast<unsigned>(
- STI.hasFeature(RISCV::FeatureNoTrailingSeqCstFence)
- ? RISCVAttrs::RISCVAtomicAbiTag::A6C
- : RISCVAttrs::RISCVAtomicAbiTag::A6S);
+ STI.hasFeature(RISCV::FeatureStdExtZalasr)
+ ? RISCVAttrs::RISCVAtomicAbiTag::A7
+ : STI.hasFeature(RISCV::FeatureNoTrailingSeqCstFence)
+ ? RISCVAttrs::RISCVAtomicAbiTag::A6C
+ : RISCVAttrs::RISCVAtomicAbiTag::A6S);
emitAttribute(RISCVAttrs::ATOMIC_ABI, AtomicABITag);
}
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Still upgrading tests |
2077859 to
a96abb9
Compare
a96abb9 to
ff3ea14
Compare
|
Everything should be ready now. I don't have write access so please land it once you are done reviewing it. |
When Zalasr is enabled, it will emit the A7 atomic ABI tag.
1c2efc9 to
8517aea
Compare
preames
left a comment
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.
LGTM
|
FYI, I have spoken with @mehnadnerd offline. He does not have commit access, and has asked me to land this on his behalf after approval. I'm going to give it until Monday in case any other reviewers have comments, and then land. |
topperc
left a comment
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.
LGTM
When Zalasr is enabled, it will emit the A7 atomic ABI tag. Zalasr is the load-acquire and store-release extension, and the reason A7 (and A6S) exists is to support it.
The A7 atomic ABI is compatible with A6S (which is what is currently emitted as the tag), but A7 is not compatible with A6C, while A6C and A6S are compatible.
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-atomic.adoc#risc-v-atomics-mappings
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#tag_riscv_atomic_abi-14-uleb128version