[lld] [llvm] [RISCV] Support RISCV Atomics ABI attributes (PR #84597)
Paul Kirth via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 2 09:26:30 PDT 2024
================
@@ -75,6 +75,13 @@ void RISCVTargetStreamer::emitTargetAttributes(const MCSubtargetInfo &STI,
auto &ISAInfo = *ParseResult;
emitTextAttribute(RISCVAttrs::ARCH, ISAInfo->toString());
}
+
+ if (STI.hasFeature(RISCV::FeatureStdExtA)) {
+ unsigned AtomicABITag = STI.hasFeature(RISCV::FeatureTrailingSeqCstFence)
+ ? RISCVAttrs::RISCVAtomicAbiTag::AtomicABI::A6S
----------------
ilovepi wrote:
> Is this the right way round? Isn't `STI.hasFeature(RISCV::FeatureTrailingSeqCstFence)` true if `+no-seq-cst-trailing-fence` is present? And if there's no trailing fence, then we have A6C? Or am I misreading?
>
I think this should be correct. The default is now "enabled", so adding `+no-seq-cst-trailing-fence` should make `STI.hasFeature(RISCV::FeatureTrailingSeqCstFence)` return `false`. I think the tests indicate that's how its working.
> I think it would be better to name the feature `FeatureNoTrailingSeqCstFence` as we do for `FeatureNoRVCHints`. Though I think this patch would be best if it aimed to change no defaults at all, and left that as a follow-on patch (which would hopefully be a quick LGTM as I think there's no objection to that, going by this review thread).
Sure I'm happy to split this up into two patches. I'll try to get that out soon.
https://github.com/llvm/llvm-project/pull/84597
More information about the llvm-commits
mailing list