From ff3f3a54e2d1b05c36943bf88ae0be7475d622ed Mon Sep 17 00:00:00 2001 From: Sander de Smalen Date: Thu, 24 Feb 2022 08:46:15 +0000 Subject: [PATCH] [AArch64][AsmParser] Arch directives should set implied features. When assembling for example an SVE instruction with the `.arch +sve2` directive, +sve should be implied by setting +sve2, similar to what would happen if one would pass the mattr=+sve2 flag on the command-line. The AsmParser doesn't set the implied features, meaning that the SVE instruction does not assemble. This patch fixes that. Note that the same does not hold when disabling a feature. For example, +nosve2 does not imply +nosve. Reviewed By: c-rhodes Differential Revision: https://reviews.llvm.org/D120259 --- .../AArch64/AsmParser/AArch64AsmParser.cpp | 35 +++++++++---------- llvm/test/MC/AArch64/SVE/directive-arch.s | 6 ++++ .../MC/AArch64/SVE/directive-arch_extension.s | 12 +++++++ .../MC/AArch64/SVE/directive-cpu-negative.s | 6 ++++ llvm/test/MC/AArch64/SVE/directive-cpu.s | 15 ++++++++ 5 files changed, 55 insertions(+), 19 deletions(-) create mode 100644 llvm/test/MC/AArch64/SVE/directive-cpu-negative.s create mode 100644 llvm/test/MC/AArch64/SVE/directive-cpu.s diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp index a764e8c16348..04ab54c8e0e3 100644 --- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp +++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp @@ -6214,12 +6214,11 @@ bool AArch64AsmParser::parseDirectiveArch(SMLoc L) { if (Extension.Features.none()) report_fatal_error("unsupported architectural extension: " + Name); - FeatureBitset ToggleFeatures = EnableFeature - ? (~Features & Extension.Features) - : ( Features & Extension.Features); - FeatureBitset Features = - ComputeAvailableFeatures(STI.ToggleFeature(ToggleFeatures)); - setAvailableFeatures(Features); + FeatureBitset ToggleFeatures = + EnableFeature + ? STI.SetFeatureBitsTransitively(~Features & Extension.Features) + : STI.ToggleFeature(Features & Extension.Features); + setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures)); break; } } @@ -6252,12 +6251,11 @@ bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) { if (Extension.Features.none()) return Error(ExtLoc, "unsupported architectural extension: " + Name); - FeatureBitset ToggleFeatures = EnableFeature - ? (~Features & Extension.Features) - : (Features & Extension.Features); - FeatureBitset Features = - ComputeAvailableFeatures(STI.ToggleFeature(ToggleFeatures)); - setAvailableFeatures(Features); + FeatureBitset ToggleFeatures = + EnableFeature + ? STI.SetFeatureBitsTransitively(~Features & Extension.Features) + : STI.ToggleFeature(Features & Extension.Features); + setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures)); return false; } @@ -6297,7 +6295,6 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) { ExpandCryptoAEK(llvm::AArch64::getCPUArchKind(CPU), RequestedExtensions); - FeatureBitset Features = STI.getFeatureBits(); for (auto Name : RequestedExtensions) { // Advance source location past '+'. CurLoc = incrementLoc(CurLoc, 1); @@ -6317,12 +6314,12 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) { if (Extension.Features.none()) report_fatal_error("unsupported architectural extension: " + Name); - FeatureBitset ToggleFeatures = EnableFeature - ? (~Features & Extension.Features) - : ( Features & Extension.Features); - FeatureBitset Features = - ComputeAvailableFeatures(STI.ToggleFeature(ToggleFeatures)); - setAvailableFeatures(Features); + FeatureBitset Features = STI.getFeatureBits(); + FeatureBitset ToggleFeatures = + EnableFeature + ? STI.SetFeatureBitsTransitively(~Features & Extension.Features) + : STI.ToggleFeature(Features & Extension.Features); + setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures)); FoundExtension = true; break; diff --git a/llvm/test/MC/AArch64/SVE/directive-arch.s b/llvm/test/MC/AArch64/SVE/directive-arch.s index 99ddf4e7bf9f..81473fbec822 100644 --- a/llvm/test/MC/AArch64/SVE/directive-arch.s +++ b/llvm/test/MC/AArch64/SVE/directive-arch.s @@ -4,3 +4,9 @@ ptrue p0.b, pow2 // CHECK: ptrue p0.b, pow2 + +// Test that the implied +sve feature is also set from +sve2. +.arch armv8-a+sve2 + +ptrue p0.b, pow2 +// CHECK: ptrue p0.b, pow2 diff --git a/llvm/test/MC/AArch64/SVE/directive-arch_extension.s b/llvm/test/MC/AArch64/SVE/directive-arch_extension.s index 72351b69f58b..c516c7d8126c 100644 --- a/llvm/test/MC/AArch64/SVE/directive-arch_extension.s +++ b/llvm/test/MC/AArch64/SVE/directive-arch_extension.s @@ -4,3 +4,15 @@ ptrue p0.b, pow2 // CHECK: ptrue p0.b, pow2 + +// Test that the implied +sve feature is also set from +sve2. +.arch_extension nosve +.arch_extension sve2 +ptrue p0.b, pow2 +// CHECK: ptrue p0.b, pow2 + +// Check that setting +nosve2 does not imply +nosve +.arch_extension nosve2 + +ptrue p0.b, pow2 +// CHECK: ptrue p0.b, pow2 diff --git a/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s b/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s new file mode 100644 index 000000000000..82acc1b0b0be --- /dev/null +++ b/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s @@ -0,0 +1,6 @@ +// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s + +.cpu generic+sve+nosve +ptrue p0.b, pow2 +// CHECK: error: instruction requires: sve or sme +// CHECK-NEXT: ptrue p0.b, pow2 diff --git a/llvm/test/MC/AArch64/SVE/directive-cpu.s b/llvm/test/MC/AArch64/SVE/directive-cpu.s new file mode 100644 index 000000000000..37d399afb390 --- /dev/null +++ b/llvm/test/MC/AArch64/SVE/directive-cpu.s @@ -0,0 +1,15 @@ +// RUN: llvm-mc -triple=aarch64 < %s | FileCheck %s + +.cpu generic+sve +ptrue p0.b, pow2 +// CHECK: ptrue p0.b, pow2 + +// Test that the implied +sve feature is also set from +sve2. +.cpu generic+sve2 +ptrue p0.b, pow2 +// CHECK: ptrue p0.b, pow2 + +// Check that setting +nosve2 does not imply +nosve +.cpu generic+sve2+nosve2 +ptrue p0.b, pow2 +// CHECK: ptrue p0.b, pow2