Skip to content

Revert "[AArch64][AsmParser] Directives should clear transitively implied features (#106625)" #106813

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

Merged
Show file tree
Hide file tree
Changes from all commits
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
102 changes: 55 additions & 47 deletions llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6947,22 +6947,18 @@ static void ExpandCryptoAEK(const AArch64::ArchInfo &ArchInfo,
}
}

static SMLoc incrementLoc(SMLoc L, int Offset) {
return SMLoc::getFromPointer(L.getPointer() + Offset);
}

/// parseDirectiveArch
/// ::= .arch token
bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
SMLoc CurLoc = getLoc();
SMLoc ArchLoc = getLoc();

StringRef Arch, ExtensionString;
std::tie(Arch, ExtensionString) =
getParser().parseStringToEndOfStatement().trim().split('+');

const AArch64::ArchInfo *ArchInfo = AArch64::parseArch(Arch);
if (!ArchInfo)
return Error(CurLoc, "unknown arch name");
return Error(ArchLoc, "unknown arch name");

if (parseToken(AsmToken::EndOfStatement))
return true;
Expand All @@ -6982,30 +6978,27 @@ bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
ExtensionString.split(RequestedExtensions, '+');

ExpandCryptoAEK(*ArchInfo, RequestedExtensions);
CurLoc = incrementLoc(CurLoc, Arch.size());

FeatureBitset Features = STI.getFeatureBits();
setAvailableFeatures(ComputeAvailableFeatures(Features));
for (auto Name : RequestedExtensions) {
// Advance source location past '+'.
CurLoc = incrementLoc(CurLoc, 1);

bool EnableFeature = !Name.consume_front_insensitive("no");

auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
return Extension.Name == Name;
});

if (It == std::end(ExtensionMap))
return Error(CurLoc, "unsupported architectural extension: " + Name);
for (const auto &Extension : ExtensionMap) {
if (Extension.Name != Name)
continue;

if (EnableFeature)
STI.SetFeatureBitsTransitively(It->Features);
else
STI.ClearFeatureBitsTransitively(It->Features);
if (Extension.Features.none())
report_fatal_error("unsupported architectural extension: " + Name);

CurLoc = incrementLoc(CurLoc, Name.size());
FeatureBitset ToggleFeatures =
EnableFeature
? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
: STI.ToggleFeature(Features & Extension.Features);
setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
break;
}
}
FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
setAvailableFeatures(Features);
return false;
}

Expand All @@ -7025,21 +7018,28 @@ bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) {
Name = Name.substr(2);
}

auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
return Extension.Name == Name;
});
MCSubtargetInfo &STI = copySTI();
FeatureBitset Features = STI.getFeatureBits();
for (const auto &Extension : ExtensionMap) {
if (Extension.Name != Name)
continue;

if (Extension.Features.none())
return Error(ExtLoc, "unsupported architectural extension: " + Name);

FeatureBitset ToggleFeatures =
EnableFeature
? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
: STI.ToggleFeature(Features & Extension.Features);
setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
return false;
}

if (It == std::end(ExtensionMap))
return Error(ExtLoc, "unsupported architectural extension: " + Name);
return Error(ExtLoc, "unknown architectural extension: " + Name);
}

MCSubtargetInfo &STI = copySTI();
if (EnableFeature)
STI.SetFeatureBitsTransitively(It->Features);
else
STI.ClearFeatureBitsTransitively(It->Features);
FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
setAvailableFeatures(Features);
return false;
static SMLoc incrementLoc(SMLoc L, int Offset) {
return SMLoc::getFromPointer(L.getPointer() + Offset);
}

/// parseDirectiveCPU
Expand Down Expand Up @@ -7075,22 +7075,30 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) {

bool EnableFeature = !Name.consume_front_insensitive("no");

auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
return Extension.Name == Name;
});
bool FoundExtension = false;
for (const auto &Extension : ExtensionMap) {
if (Extension.Name != Name)
continue;

if (It == std::end(ExtensionMap))
Error(CurLoc, "unsupported architectural extension: " + Name);
if (Extension.Features.none())
report_fatal_error("unsupported architectural extension: " + Name);

if (EnableFeature)
STI.SetFeatureBitsTransitively(It->Features);
else
STI.ClearFeatureBitsTransitively(It->Features);
FeatureBitset Features = STI.getFeatureBits();
FeatureBitset ToggleFeatures =
EnableFeature
? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
: STI.ToggleFeature(Features & Extension.Features);
setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
FoundExtension = true;

break;
}

if (!FoundExtension)
Error(CurLoc, "unsupported architectural extension");

CurLoc = incrementLoc(CurLoc, Name.size());
}
FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
setAvailableFeatures(Features);
return false;
}

Expand Down
8 changes: 0 additions & 8 deletions llvm/test/MC/AArch64/SVE/directive-arch-negative.s

This file was deleted.

7 changes: 1 addition & 6 deletions llvm/test/MC/AArch64/SVE/directive-arch_extension-negative.s
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s

.arch_extension sve2+nosve
.arch_extension nosve

ptrue p0.b, pow2
// CHECK: error: instruction requires: sve or sme
// CHECK-NEXT: ptrue p0.b, pow2

// Check that setting +nosve implies +nosve2
adclb z0.s, z1.s, z31.s
// CHECK: error: instruction requires: sve2
// CHECK-NEXT: adclb z0.s, z1.s, z31.s
7 changes: 1 addition & 6 deletions llvm/test/MC/AArch64/SVE/directive-cpu-negative.s
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s

.cpu generic+sve2+nosve
.cpu generic+sve+nosve
ptrue p0.b, pow2
// CHECK: error: instruction requires: sve or sme
// CHECK-NEXT: ptrue p0.b, pow2

// Check that setting +nosve implies +nosve2
adclb z0.s, z1.s, z31.s
// CHECK: error: instruction requires: sve2
// CHECK-NEXT: adclb z0.s, z1.s, z31.s
5 changes: 1 addition & 4 deletions llvm/test/MC/AArch64/directive-arch-negative.s
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@
# CHECK-NEXT: aese v0.8h, v1.8h
# CHECK-NEXT: ^

// We silently ignore invalid features.
.arch armv8+foo
aese v0.8h, v1.8h

# CHECK: error: unsupported architectural extension: foo
# CHECK-NEXT: .arch armv8+foo
# CHECK-NEXT: ^

# CHECK: error: invalid operand for instruction
# CHECK-NEXT: aese v0.8h, v1.8h
# CHECK-NEXT: ^
Expand Down
14 changes: 1 addition & 13 deletions llvm/test/MC/AArch64/directive-arch_extension-negative.s
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// RUN: -filetype asm -o - %s 2>&1 | FileCheck %s

.arch_extension axp64
// CHECK: error: unsupported architectural extension: axp64
// CHECK: error: unknown architectural extension: axp64
// CHECK-NEXT: .arch_extension axp64

crc32cx w0, w1, x3
Expand Down Expand Up @@ -49,8 +49,6 @@ fminnm d0, d0, d1
// CHECK: [[@LINE-1]]:1: error: instruction requires: fp
// CHECK-NEXT: fminnm d0, d0, d1

// nofp implied nosimd, so reinstate it
.arch_extension simd
addp v0.4s, v0.4s, v0.4s
// CHECK-NOT: [[@LINE-1]]:1: error: instruction requires: neon
.arch_extension nosimd
Expand All @@ -72,8 +70,6 @@ casa w5, w7, [x20]
// CHECK: [[@LINE-1]]:1: error: instruction requires: lse
// CHECK-NEXT: casa w5, w7, [x20]

// nolse implied nolse128, so reinstate it
.arch_extension lse128
swpp x0, x2, [x3]
// CHECK-NOT: [[@LINE-1]]:1: error: instruction requires: lse128
.arch_extension nolse128
Expand All @@ -88,8 +84,6 @@ cfp rctx, x0
// CHECK: [[@LINE-1]]:5: error: CFPRCTX requires: predres
// CHECK-NEXT: cfp rctx, x0

// nopredres implied nopredres2, so reinstate it
.arch_extension predres2
cosp rctx, x0
// CHECK-NOT: [[@LINE-1]]:6: error: COSP requires: predres2
.arch_extension nopredres2
Expand Down Expand Up @@ -139,8 +133,6 @@ ldapr x0, [x1]
// CHECK: [[@LINE-1]]:1: error: instruction requires: rcpc
// CHECK-NEXT: ldapr x0, [x1]

// norcpc implied norcpc3, so reinstate it
.arch_extension rcpc3
stilp w24, w0, [x16, #-8]!
// CHECK-NOT: [[@LINE-1]]:1: error: instruction requires: rcpc3
.arch_extension norcpc3
Expand Down Expand Up @@ -177,8 +169,6 @@ cpyfp [x0]!, [x1]!, x2!
// CHECK: [[@LINE-1]]:1: error: instruction requires: mops
// CHECK-NEXT: cpyfp [x0]!, [x1]!, x2!

// nolse128 implied nod128, so reinstate it
.arch_extension d128
// This needs to come before `.arch_extension nothe` as it uses an instruction
// that requires both the and d128
sysp #0, c2, c0, #0, x0, x1
Expand Down Expand Up @@ -214,8 +204,6 @@ umax x0, x1, x2
// CHECK: [[@LINE-1]]:1: error: instruction requires: cssc
// CHECK-NEXT: umax x0, x1, x2

// noras implied norasv2, so reinstate it
.arch_extension rasv2
mrs x0, ERXGSR_EL1
// CHECK-NOT: [[@LINE-1]]:9: error: expected readable system register
.arch_extension norasv2
Expand Down
Loading