Skip to content

Commit

Permalink
Revert "[RISCV] Support RISCV Atomics ABI attributes (#84597)"
Browse files Browse the repository at this point in the history
This reverts commit 9221f3a.

As reported
<#84597 (comment)>
and confirmed by me locally, adding these attributes causes current GNU
ld to segfault when processing the input. Reverted so we can discuss
the best next step.
  • Loading branch information
asb committed Apr 26, 2024
1 parent 357530f commit 431be86
Show file tree
Hide file tree
Showing 10 changed files with 2 additions and 314 deletions.
63 changes: 0 additions & 63 deletions lld/ELF/Arch/RISCV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1084,62 +1084,10 @@ static void mergeArch(RISCVISAInfo::OrderedExtensionMap &mergedExts,
}
}

static void mergeAtomic(DenseMap<unsigned, unsigned>::iterator it,
const InputSectionBase *oldSection,
const InputSectionBase *newSection, unsigned int oldTag,
unsigned int newTag) {
using RISCVAttrs::RISCVAtomicAbiTag::AtomicABI;
// Same tags stay the same, and UNKNOWN is compatible with anything
if (oldTag == newTag || newTag == AtomicABI::UNKNOWN)
return;

switch (oldTag) {
case AtomicABI::UNKNOWN:
it->getSecond() = newTag;
return;
case AtomicABI::A6C:
switch (newTag) {
case AtomicABI::A6S:
it->getSecond() = AtomicABI::A6C;
return;
case AtomicABI::A7:
error(toString(oldSection) + " has atomic_abi=" + Twine(oldTag) +
" but " + toString(newSection) +
" has atomic_abi=" + Twine(newTag));
return;
};

case AtomicABI::A6S:
switch (newTag) {
case AtomicABI::A6C:
it->getSecond() = AtomicABI::A6C;
return;
case AtomicABI::A7:
it->getSecond() = AtomicABI::A7;
return;
};

case AtomicABI::A7:
switch (newTag) {
case AtomicABI::A6S:
it->getSecond() = AtomicABI::A7;
return;
case AtomicABI::A6C:
error(toString(oldSection) + " has atomic_abi=" + Twine(oldTag) +
" but " + toString(newSection) +
" has atomic_abi=" + Twine(newTag));
return;
};
default:
llvm_unreachable("unknown AtomicABI");
};
}

static RISCVAttributesSection *
mergeAttributesSection(const SmallVector<InputSectionBase *, 0> &sections) {
RISCVISAInfo::OrderedExtensionMap exts;
const InputSectionBase *firstStackAlign = nullptr;
const InputSectionBase *firstAtomicAbi = nullptr;
unsigned firstStackAlignValue = 0, xlen = 0;
bool hasArch = false;

Expand Down Expand Up @@ -1186,17 +1134,6 @@ mergeAttributesSection(const SmallVector<InputSectionBase *, 0> &sections) {
case RISCVAttrs::PRIV_SPEC_MINOR:
case RISCVAttrs::PRIV_SPEC_REVISION:
break;

case llvm::RISCVAttrs::AttrType::ATOMIC_ABI:
if (auto i = parser.getAttributeValue(tag.attr)) {
auto r = merged.intAttr.try_emplace(tag.attr, *i);
if (r.second) {
firstAtomicAbi = sec;
} else {
mergeAtomic(r.first, firstAtomicAbi, sec, r.first->getSecond(), *i);
}
}
continue;
}

// Fallback for deprecated priv_spec* and other unknown attributes: retain
Expand Down
202 changes: 0 additions & 202 deletions lld/test/ELF/riscv-attributes.s
Original file line number Diff line number Diff line change
Expand Up @@ -44,39 +44,6 @@
# RUN: not ld.lld a.o b.o c.o diff_stack_align.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=STACK_ALIGN --implicit-check-not=error:
# STACK_ALIGN: error: diff_stack_align.o:(.riscv.attributes) has stack_align=32 but a.o:(.riscv.attributes) has stack_align=16

## merging atomic_abi values for A6C and A7 lead to an error.
# RUN: llvm-mc -filetype=obj -triple=riscv64 atomic_abi_A6C.s -o atomic_abi_A6C.o
# RUN: llvm-mc -filetype=obj -triple=riscv64 atomic_abi_A7.s -o atomic_abi_A7.o
# RUN: not ld.lld atomic_abi_A6C.o atomic_abi_A7.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=ATOMIC_ABI_ERROR --implicit-check-not=error:
# ATOMIC_ABI_ERROR: error: atomic_abi_A6C.o:(.riscv.attributes) has atomic_abi=1 but atomic_abi_A7.o:(.riscv.attributes) has atomic_abi=3


# RUN: llvm-mc -filetype=obj -triple=riscv64 atomic_abi_A6S.s -o atomic_abi_A6S.o
# RUN: ld.lld atomic_abi_A6S.o atomic_abi_A6C.o -o atomic_abi_A6C_A6S
# RUN: llvm-readobj -A atomic_abi_A6C_A6S | FileCheck %s --check-prefix=A6C_A6S

# RUN: ld.lld atomic_abi_A6S.o atomic_abi_A7.o -o atomic_abi_A6S_A7
# RUN: llvm-readobj -A atomic_abi_A6S_A7 | FileCheck %s --check-prefix=A6S_A7

# RUN: llvm-mc -filetype=obj -triple=riscv64 atomic_abi_unknown.s -o atomic_abi_unknown.o
# RUN: ld.lld atomic_abi_unknown.o atomic_abi_A6C.o -o atomic_abi_A6C_unknown
# RUN: llvm-readobj -A atomic_abi_A6C_unknown | FileCheck %s --check-prefixes=UNKNOWN_A6C

# RUN: ld.lld atomic_abi_unknown.o diff_stack_align.o -o atomic_abi_none_unknown
# RUN: llvm-readobj -A atomic_abi_none_unknown | FileCheck %s --check-prefixes=UNKNOWN_NONE

# RUN: ld.lld diff_stack_align.o atomic_abi_A6C.o -o atomic_abi_A6C_none
# RUN: llvm-readobj -A atomic_abi_A6C_none | FileCheck %s --check-prefixes=NONE_A6C

# RUN: ld.lld atomic_abi_unknown.o atomic_abi_A6S.o -o atomic_abi_A6S_unknown
# RUN: llvm-readobj -A atomic_abi_A6S_unknown | FileCheck %s --check-prefix=UNKNOWN_A6S

# RUN: ld.lld atomic_abi_unknown.o atomic_abi_A7.o -o atomic_abi_A7_unknown
# RUN: llvm-readobj -A atomic_abi_A7_unknown | FileCheck %s --check-prefix=UNKNOWN_A7

# RUN: ld.lld diff_stack_align.o atomic_abi_A7.o -o atomic_abi_A7_none
# RUN: llvm-readobj -A atomic_abi_A7_none | FileCheck %s --check-prefix=NONE_A7

## The deprecated priv_spec is not handled as GNU ld does.
## Differing priv_spec attributes lead to an absent attribute.
# RUN: llvm-mc -filetype=obj -triple=riscv64 diff_priv_spec.s -o diff_priv_spec.o
Expand Down Expand Up @@ -319,175 +286,6 @@
.attribute priv_spec, 3
.attribute priv_spec_minor, 3

#--- atomic_abi_unknown.s
.attribute atomic_abi, 0

#--- atomic_abi_A6C.s
.attribute atomic_abi, 1

#--- atomic_abi_A6S.s
.attribute atomic_abi, 2

#--- atomic_abi_A7.s
.attribute atomic_abi, 3

# UNKNOWN_NONE: BuildAttributes {
# UNKNOWN_NONE-NEXT: FormatVersion: 0x41
# UNKNOWN_NONE-NEXT: Section 1 {
# UNKNOWN_NONE-NEXT: SectionLength: 17
# UNKNOWN_NONE-NEXT: Vendor: riscv
# UNKNOWN_NONE-NEXT: Tag: Tag_File (0x1)
# UNKNOWN_NONE-NEXT: Size: 7
# UNKNOWN_NONE-NEXT: FileAttributes {
# UNKNOWN_NONE-NEXT: Attribute {
# UNKNOWN_NONE-NEXT: Tag: 4
# UNKNOWN_NONE-NEXT: Value: 32
# UNKNOWN_NONE-NEXT: TagName: stack_align
# UNKNOWN_NONE-NEXT: Description: Stack alignment is 32-bytes
# UNKNOWN_NONE-NEXT: }
# UNKNOWN_NONE-NEXT: }
# UNKNOWN_NONE-NEXT: }
# UNKNOWN_NONE-NEXT: }

# NONE_A6C: BuildAttributes {
# NONE_A6C-NEXT: FormatVersion: 0x41
# NONE_A6C-NEXT: Section 1 {
# NONE_A6C-NEXT: SectionLength: 19
# NONE_A6C-NEXT: Vendor: riscv
# NONE_A6C-NEXT: Tag: Tag_File (0x1)
# NONE_A6C-NEXT: Size: 9
# NONE_A6C-NEXT: FileAttributes {
# NONE_A6C-NEXT: Attribute {
# NONE_A6C-NEXT: Tag: 14
# NONE_A6C-NEXT: Value: 1
# NONE_A6C-NEXT: TagName: atomic_abi
# NONE_A6C-NEXT: Description: Atomic ABI is 1
# NONE_A6C-NEXT: }
# NONE_A6C-NEXT: Attribute {
# NONE_A6C-NEXT: Tag: 4
# NONE_A6C-NEXT: Value: 32
# NONE_A6C-NEXT: TagName: stack_align
# NONE_A6C-NEXT: Description: Stack alignment is 32-bytes
# NONE_A6C-NEXT: }
# NONE_A6C-NEXT: }
# NONE_A6C-NEXT: }
# NONE_A6C-NEXT: }

# UNKNOWN_A6C: BuildAttributes {
# UNKNOWN_A6C-NEXT: FormatVersion: 0x41
# UNKNOWN_A6C-NEXT: Section 1 {
# UNKNOWN_A6C-NEXT: SectionLength: 17
# UNKNOWN_A6C-NEXT: Vendor: riscv
# UNKNOWN_A6C-NEXT: Tag: Tag_File (0x1)
# UNKNOWN_A6C-NEXT: Size: 7
# UNKNOWN_A6C-NEXT: FileAttributes {
# UNKNOWN_A6C-NEXT: Attribute {
# UNKNOWN_A6C-NEXT: Tag: 14
# UNKNOWN_A6C-NEXT: Value: 1
# UNKNOWN_A6C-NEXT: TagName: atomic_abi
# UNKNOWN_A6C-NEXT: Description: Atomic ABI is 1
# UNKNOWN_A6C-NEXT: }
# UNKNOWN_A6C-NEXT: }
# UNKNOWN_A6C-NEXT: }
# UNKNOWN_A6C-NEXT: }

# UNKNOWN_A6S: BuildAttributes {
# UNKNOWN_A6S-NEXT: FormatVersion: 0x41
# UNKNOWN_A6S-NEXT: Section 1 {
# UNKNOWN_A6S-NEXT: SectionLength:
# UNKNOWN_A6S-NEXT: Vendor: riscv
# UNKNOWN_A6S-NEXT: Tag: Tag_File (0x1)
# UNKNOWN_A6S-NEXT: Size: 7
# UNKNOWN_A6S-NEXT: FileAttributes {
# UNKNOWN_A6S-NEXT: Attribute {
# UNKNOWN_A6S-NEXT: Tag: 14
# UNKNOWN_A6S-NEXT: Value: 2
# UNKNOWN_A6S-NEXT: TagName: atomic_abi
# UNKNOWN_A6S-NEXT: Description: Atomic ABI is 2
# UNKNOWN_A6S-NEXT: }
# UNKNOWN_A6S-NEXT: }
# UNKNOWN_A6S-NEXT: }
# UNKNOWN_A6S-NEXT: }

# NONE_A7: BuildAttributes {
# NONE_A7-NEXT: FormatVersion: 0x41
# NONE_A7-NEXT: Section 1 {
# NONE_A7-NEXT: SectionLength: 19
# NONE_A7-NEXT: Vendor: riscv
# NONE_A7-NEXT: Tag: Tag_File (0x1)
# NONE_A7-NEXT: Size: 9
# NONE_A7-NEXT: FileAttributes {
# NONE_A7-NEXT: Attribute {
# NONE_A7-NEXT: Tag: 14
# NONE_A7-NEXT: Value: 3
# NONE_A7-NEXT: TagName: atomic_abi
# NONE_A7-NEXT: Description: Atomic ABI is 3
# NONE_A7-NEXT: }
# NONE_A7-NEXT: Attribute {
# NONE_A7-NEXT: Tag: 4
# NONE_A7-NEXT: Value: 32
# NONE_A7-NEXT: TagName: stack_align
# NONE_A7-NEXT: Description: Stack alignment is 32-bytes
# NONE_A7-NEXT: }
# NONE_A7-NEXT: }
# NONE_A7-NEXT: }
# NONE_A7-NEXT: }


# UNKNOWN_A7: BuildAttributes {
# UNKNOWN_A7-NEXT: FormatVersion: 0x41
# UNKNOWN_A7-NEXT: Section 1 {
# UNKNOWN_A7-NEXT: SectionLength: 17
# UNKNOWN_A7-NEXT: Vendor: riscv
# UNKNOWN_A7-NEXT: Tag: Tag_File (0x1)
# UNKNOWN_A7-NEXT: Size: 7
# UNKNOWN_A7-NEXT: FileAttributes {
# UNKNOWN_A7-NEXT: Attribute {
# UNKNOWN_A7-NEXT: Tag: 14
# UNKNOWN_A7-NEXT: Value: 3
# UNKNOWN_A7-NEXT: TagName: atomic_abi
# UNKNOWN_A7-NEXT: Description: Atomic ABI is 3
# UNKNOWN_A7-NEXT: }
# UNKNOWN_A7-NEXT: }
# UNKNOWN_A7-NEXT: }
# UNKNOWN_A7-NEXT: }

# A6C_A6S: BuildAttributes {
# A6C_A6S-NEXT: FormatVersion: 0x41
# A6C_A6S-NEXT: Section 1 {
# A6C_A6S-NEXT: SectionLength: 17
# A6C_A6S-NEXT: Vendor: riscv
# A6C_A6S-NEXT: Tag: Tag_File (0x1)
# A6C_A6S-NEXT: Size: 7
# A6C_A6S-NEXT: FileAttributes {
# A6C_A6S-NEXT: Attribute {
# A6C_A6S-NEXT: Tag: 14
# A6C_A6S-NEXT: Value: 1
# A6C_A6S-NEXT: TagName: atomic_abi
# A6C_A6S-NEXT: Description: Atomic ABI is 1
# A6C_A6S-NEXT: }
# A6C_A6S-NEXT: }
# A6C_A6S-NEXT: }
# A6C_A6S-NEXT: }

# A6S_A7: BuildAttributes {
# A6S_A7-NEXT: FormatVersion: 0x41
# A6S_A7-NEXT: Section 1 {
# A6S_A7-NEXT: SectionLength: 17
# A6S_A7-NEXT: Vendor: riscv
# A6S_A7-NEXT: Tag: Tag_File (0x1)
# A6S_A7-NEXT: Size: 7
# A6S_A7-NEXT: FileAttributes {
# A6S_A7-NEXT: Attribute {
# A6S_A7-NEXT: Tag: 14
# A6S_A7-NEXT: Value: 3
# A6S_A7-NEXT: TagName: atomic_abi
# A6S_A7-NEXT: Description: Atomic ABI is 3
# A6S_A7-NEXT: }
# A6S_A7-NEXT: }
# A6S_A7-NEXT: }
# A6S_A7-NEXT: }

#--- unknown13.s
.attribute 13, "0"
#--- unknown13a.s
Expand Down
1 change: 0 additions & 1 deletion llvm/include/llvm/Support/RISCVAttributeParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class RISCVAttributeParser : public ELFAttributeParser {

Error unalignedAccess(unsigned tag);
Error stackAlign(unsigned tag);
Error atomicAbi(unsigned tag);

public:
RISCVAttributeParser(ScopedPrinter *sw)
Expand Down
13 changes: 0 additions & 13 deletions llvm/include/llvm/Support/RISCVAttributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,8 @@ enum AttrType : unsigned {
PRIV_SPEC = 8,
PRIV_SPEC_MINOR = 10,
PRIV_SPEC_REVISION = 12,
ATOMIC_ABI = 14,
};

namespace RISCVAtomicAbiTag {
enum AtomicABI : unsigned {
// Values for Tag_RISCV_atomic_abi
// Defined at
// https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#tag_riscv_atomic_abi-14-uleb128version
UNKNOWN = 0,
A6C = 1,
A6S = 2,
A7 = 3,
};
} // namespace RISCVAtomicAbiTag

enum { NOT_ALLOWED = 0, ALLOWED = 1 };

} // namespace RISCVAttrs
Expand Down
13 changes: 1 addition & 12 deletions llvm/lib/Support/RISCVAttributeParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,7 @@ const RISCVAttributeParser::DisplayHandler
{
RISCVAttrs::UNALIGNED_ACCESS,
&RISCVAttributeParser::unalignedAccess,
},
{
RISCVAttrs::ATOMIC_ABI,
&RISCVAttributeParser::atomicAbi,
},
};

Error RISCVAttributeParser::atomicAbi(unsigned Tag) {
uint64_t Value = de.getULEB128(cursor);
printAttribute(Tag, Value, "Atomic ABI is " + utostr(Value));
return Error::success();
}
}};

Error RISCVAttributeParser::unalignedAccess(unsigned tag) {
static const char *strings[] = {"No unaligned access", "Unaligned access"};
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Support/RISCVAttributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ static constexpr TagNameItem tagData[] = {
{PRIV_SPEC, "Tag_priv_spec"},
{PRIV_SPEC_MINOR, "Tag_priv_spec_minor"},
{PRIV_SPEC_REVISION, "Tag_priv_spec_revision"},
{ATOMIC_ABI, "Tag_atomic_abi"},
};

constexpr TagNameMap RISCVAttributeTags{tagData};
Expand Down
7 changes: 0 additions & 7 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,6 @@ 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
: RISCVAttrs::RISCVAtomicAbiTag::AtomicABI::A6C;
emitAttribute(RISCVAttrs::ATOMIC_ABI, AtomicABITag);
}
}

// This part is for ascii assembly output
Expand Down
10 changes: 1 addition & 9 deletions llvm/test/CodeGen/RISCV/attributes.ll
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,7 @@
; RUN: llc -mtriple=riscv64 -mattr=+m %s -o - | FileCheck --check-prefixes=CHECK,RV64M %s
; RUN: llc -mtriple=riscv64 -mattr=+zmmul %s -o - | FileCheck --check-prefixes=CHECK,RV64ZMMUL %s
; RUN: llc -mtriple=riscv64 -mattr=+m,+zmmul %s -o - | FileCheck --check-prefixes=CHECK,RV64MZMMUL %s
; RUN: llc -mtriple=riscv64 -mattr=+a %s -o - | FileCheck --check-prefixes=CHECK,RV64A,A6C %s
; RUN: llc -mtriple=riscv64 -mattr=+a,+seq-cst-trailing-fence %s -o - | FileCheck --check-prefixes=CHECK,RV64A,A6S %s
; RUN: llc -mtriple=riscv64 -mattr=+a %s -o - | FileCheck --check-prefixes=CHECK,RV64A %s
; RUN: llc -mtriple=riscv64 -mattr=+f %s -o - | FileCheck --check-prefixes=CHECK,RV64F %s
; RUN: llc -mtriple=riscv64 -mattr=+d %s -o - | FileCheck --check-prefixes=CHECK,RV64D %s
; RUN: llc -mtriple=riscv64 -mattr=+c %s -o - | FileCheck --check-prefixes=CHECK,RV64C %s
Expand Down Expand Up @@ -517,10 +516,3 @@ define i32 @addi(i32 %a) {
%1 = add i32 %a, 1
ret i32 %1
}

define i8 @atomic_load_i8_seq_cst(ptr %a) nounwind {
%1 = load atomic i8, ptr %a seq_cst, align 1
ret i8 %1
; A6S: .attribute 14, 2
; A6C: .attribute 14, 1
}
3 changes: 0 additions & 3 deletions llvm/test/MC/RISCV/attribute.s
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,3 @@

.attribute priv_spec_revision, 0
# CHECK: attribute 12, 0

.attribute atomic_abi, 0
# CHECK: attribute 14, 0
Loading

0 comments on commit 431be86

Please sign in to comment.