Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions llvm/lib/Support/AArch64BuildAttributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ StringRef getOptionalStr(unsigned Optional) {
case OPTIONAL:
return "optional";
case OPTIONAL_NOT_FOUND:
LLVM_FALLTHROUGH;
default:
return "";
}
Expand All @@ -62,17 +61,14 @@ StringRef getTypeStr(unsigned Type) {
case NTBS:
return "ntbs";
case TYPE_NOT_FOUND:
LLVM_FALLTHROUGH;
default:
return "";
}
}
SubsectionType getTypeID(StringRef Type) {
return StringSwitch<SubsectionType>(Type)
.Case("uleb128", ULEB128)
.Case("ULEB128", ULEB128)
.Case("NTBS", NTBS)
.Case("ntbs", NTBS)
.Cases("uleb128", "ULEB128", ULEB128)
.Cases("ntbs", "NTBS", NTBS)
.Default(TYPE_NOT_FOUND);
}
StringRef getSubsectionTypeUnknownError() {
Expand All @@ -86,7 +82,6 @@ StringRef getPauthABITagsStr(unsigned PauthABITag) {
case TAG_PAUTH_SCHEMA:
return "Tag_PAuth_Schema";
case PAUTHABI_TAG_NOT_FOUND:
LLVM_FALLTHROUGH;
default:
return "";
}
Expand All @@ -107,7 +102,6 @@ StringRef getFeatureAndBitsTagsStr(unsigned FeatureAndBitsTag) {
case TAG_FEATURE_GCS:
return "Tag_Feature_GCS";
case FEATURE_AND_BITS_TAG_NOT_FOUND:
LLVM_FALLTHROUGH;
default:
return "";
}
Expand Down
30 changes: 14 additions & 16 deletions llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7821,13 +7821,6 @@ bool AArch64AsmParser::parseDirectiveAeabiSubSectionHeader(SMLoc L) {
// (3)uleb128 separated by 2 commas.
MCAsmParser &Parser = getParser();

bool HasActiveSubsection = true;
std::unique_ptr<MCELFStreamer::AttributeSubSection> ActiveSubsection =
getTargetStreamer().getActiveAtributesSubsection();
if (nullptr == ActiveSubsection) {
HasActiveSubsection = false;
}

// Consume the name (subsection name)
StringRef SubsectionName;
AArch64BuildAttributes::VendorID SubsectionNameID;
Expand All @@ -7846,6 +7839,13 @@ bool AArch64AsmParser::parseDirectiveAeabiSubSectionHeader(SMLoc L) {
return true;
}

bool SubsectionExists = true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this extra bool, you can just check if (ExistingSubsection) below.

std::unique_ptr<MCELFStreamer::AttributeSubSection> ExistingSubsection =
getTargetStreamer().getActiveSubsectionByName(SubsectionName);
if (nullptr == ExistingSubsection) {
SubsectionExists = false;
}

// Consume the first parameter (optionality parameter)
AArch64BuildAttributes::SubsectionOptional IsOptional;
// options: optional/required
Expand All @@ -7858,20 +7858,19 @@ bool AArch64AsmParser::parseDirectiveAeabiSubSectionHeader(SMLoc L) {
Optionality);
return true;
}
if (HasActiveSubsection &&
(SubsectionName == ActiveSubsection->VendorName)) {
if (IsOptional != ActiveSubsection->IsOptional) {
if (SubsectionExists) {
if (IsOptional != ExistingSubsection->IsOptional) {
Error(Parser.getTok().getLoc(),
"optionality mismatch! subsection '" + SubsectionName +
"' already exists with optionality defined as '" +
Twine(ActiveSubsection->IsOptional) + "' and not '" +
Twine(ExistingSubsection->IsOptional) + "' and not '" +
Twine(IsOptional) + "' (0: required, 1: optional)");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the numbers here are only used in the ELF encoding, so they aren't relevant to the assembly programmer. The diagnostic should refer to the identifiers used in the assembly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, also for type.

return true;
}
}
} else {
Error(Parser.getTok().getLoc(),
"optionality parameter not found, expected required|optinal");
"optionality parameter not found, expected required|optional");
return true;
}
// Check for possible IsOptional unaccepted values for known subsections
Expand Down Expand Up @@ -7906,13 +7905,12 @@ bool AArch64AsmParser::parseDirectiveAeabiSubSectionHeader(SMLoc L) {
Name);
return true;
}
if (HasActiveSubsection &&
(SubsectionName == ActiveSubsection->VendorName)) {
if (Type != ActiveSubsection->ParameterType) {
if (SubsectionExists) {
if (Type != ExistingSubsection->ParameterType) {
Error(Parser.getTok().getLoc(),
"type mismatch! subsection '" + SubsectionName +
"' already exists with type defined as '" +
Twine(ActiveSubsection->ParameterType) + "' and not '" +
Twine(ExistingSubsection->ParameterType) + "' and not '" +
Twine(Type) + "' (0: uleb128, 1: ntbs)");
return true;
}
Expand Down
28 changes: 7 additions & 21 deletions llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,7 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
Override);
break;
case AArch64BuildAttributes::TAG_FEATURE_BTI:
LLVM_FALLTHROUGH;
case AArch64BuildAttributes::TAG_FEATURE_GCS:
LLVM_FALLTHROUGH;
case AArch64BuildAttributes::TAG_FEATURE_PAC:
OS << "\t.aeabi_attribute" << "\t"
<< AArch64BuildAttributes::getFeatureAndBitsTagsStr(Tag) << ", "
Expand All @@ -212,7 +210,6 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
Override);
break;
case AArch64BuildAttributes::TAG_PAUTH_PLATFORM:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, can these be folded together?

LLVM_FALLTHROUGH;
case AArch64BuildAttributes::TAG_PAUTH_SCHEMA:
OS << "\t.aeabi_attribute" << "\t"
<< AArch64BuildAttributes::getPauthABITagsStr(Tag) << ", " << Value;
Expand Down Expand Up @@ -246,16 +243,7 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {

switch (SubsectionID) {
default: {
assert(0 && "Subsection error");
break;
}
case AArch64BuildAttributes::VENDOR_UNKNOWN: {
// Keep the data structure consistent with the case of ELF emission
// (important for llvm-mc asm parsing)
OS << "\t" << SubsectionTag << "\t" << SubsectionName << ", "
<< OptionalStr << ", " << ParameterStr;
AArch64TargetStreamer::emitAtributesSubsection(SubsectionName, Optional,
ParameterType);
// Treated as a private subsection
break;
}
case AArch64BuildAttributes::AEABI_PAUTHABI: {
Expand All @@ -265,10 +253,6 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
assert(AArch64BuildAttributes::ULEB128 == ParameterType &&
"subsection .aeabi-pauthabi should be "
"marked as uleb128 and not as ntbs");
OS << "\t" << SubsectionTag << "\t" << SubsectionName << ", "
<< OptionalStr << ", " << ParameterStr;
AArch64TargetStreamer::emitAtributesSubsection(SubsectionName, Optional,
ParameterType);
break;
}
case AArch64BuildAttributes::AEABI_FEATURE_AND_BITS: {
Expand All @@ -278,13 +262,15 @@ class AArch64TargetAsmStreamer : public AArch64TargetStreamer {
assert(AArch64BuildAttributes::ULEB128 == ParameterType &&
"subsection .aeabi_feature_and_bits should "
"be marked as uleb128 and not as ntbs");
OS << "\t" << SubsectionTag << "\t" << SubsectionName << ", "
<< OptionalStr << ", " << ParameterStr;
AArch64TargetStreamer::emitAtributesSubsection(SubsectionName, Optional,
ParameterType);
break;
}
}
OS << "\t" << SubsectionTag << "\t" << SubsectionName << ", " << OptionalStr
<< ", " << ParameterStr;
// Keep the data structure consistent with the case of ELF emission
// (important for llvm-mc asm parsing)
AArch64TargetStreamer::emitAtributesSubsection(SubsectionName, Optional,
ParameterType);
OS << "\n";
}

Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Target/AArch64/MCTargetDesc/AArch64TargetStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@ AArch64TargetStreamer::getActiveAtributesSubsection() {
return nullptr;
}

std::unique_ptr<MCELFStreamer::AttributeSubSection>
AArch64TargetStreamer::getActiveSubsectionByName(StringRef name) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is misleading, because this function doesn't check if the subsection is active. This should also have "Attribute" in the name, because "subsection" has other meanings.

Copy link
Contributor Author

@sivan-shani sivan-shani Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

between getAtributeSubsectionByName and getAtributesSubsectionByName, opt for the later. (attribute in singular/plural)

for (MCELFStreamer::AttributeSubSection &SubSection : AttributeSubSections) {
if (name == SubSection.VendorName) {
return std::make_unique<MCELFStreamer::AttributeSubSection>(SubSection);
}
}
return nullptr;
}

void AArch64TargetStreamer::emitAttribute(StringRef VendorName, unsigned Tag,
unsigned Value, std::string String,
bool Override) {
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Target/AArch64/MCTargetDesc/AArch64TargetStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ class AArch64TargetStreamer : public MCTargetStreamer {
void activateAtributesSubsection(StringRef VendorName);
std::unique_ptr<MCELFStreamer::AttributeSubSection>
getActiveAtributesSubsection();
std::unique_ptr<MCELFStreamer::AttributeSubSection>
getActiveSubsectionByName(StringRef name);
void
insertAttributeInPlace(const MCELFStreamer::AttributeItem &Attr,
MCELFStreamer::AttributeSubSection &AttSubSection);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@
// ERR-NEXT: .aeabi_subsection aeabi_pauthabi, a, uleb128

.aeabi_subsection aeabi_pauthabi, 1, uleb128
// ERR: error: optionality parameter not found, expected required|optinal
// ERR: error: optionality parameter not found, expected required|optional
// ERR-NEXT: .aeabi_subsection aeabi_pauthabi, 1, uleb128

.aeabi_subsection aeabi_pauthabi, ,uleb128
// ERR: error: optionality parameter not found, expected required|optinal
// ERR: error: optionality parameter not found, expected required|optional
// ERR-NEXT: .aeabi_subsection aeabi_pauthabi, ,uleb128

.aeabi_subsection aeabi_pauthabi,uleb128
Expand All @@ -59,4 +59,3 @@
.aeabi_subsection aeabi_pauthabi, required, a
// ERR: error: unknown AArch64 build attributes type, expected uleb128|ntbs: a
// ERR-NEXT: .aeabi_subsection aeabi_pauthabi, required, a

Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,16 @@
.aeabi_subsection private_subsection_1, optional, ntbs
.aeabi_attribute 324, 1
// ERR: error: active subsection type is NTBS (string), found ULEB128 (unsigned)
// ERR-NEXT: .aeabi_attribute 324, 1
// ERR-NEXT: .aeabi_attribute 324, 1

.aeabi_subsection foo, optional, uleb128
.aeabi_subsection bar, optional, uleb128
.aeabi_subsection foo, required, uleb128
// ERR: error: optionality mismatch! subsection 'foo' already exists with optionality defined as '1' and not '0' (0: required, 1: optional)
// ERR-NEXT: .aeabi_subsection foo, required, uleb128

.aeabi_subsection goo, optional, ntbs
.aeabi_subsection zar, optional, ntbs
.aeabi_subsection goo, optional, uleb128
// ERR: error: type mismatch! subsection 'goo' already exists with type defined as '1' and not '0' (0: uleb128, 1: ntbs)
// ERR-NEXT: .aeabi_subsection goo, optional, uleb128
Loading