labrinea created this revision.
labrinea added reviewers: simon_tatham, SjoerdMeijer, ostannard.
Herald added subscribers: dmgreen, hiraditya, kristof.beyls, javed.absar.
Herald added projects: clang, LLVM.
When processing the command line options `march`, `mcpu` and `mfpu`, we store
the implied target features on a vector. The change D62998
<https://reviews.llvm.org/D62998> introduced a temporary vector, where the
processed features get accumulated. When calling `DecodeARMFeaturesFromCPU`,
which sets the default features for the specified CPU, we certainly don't want
to override the features that have been explicitly specified on the command
line. Therefore, the default features should appear first in the final vector.
This problem became evident once I added the missing (unhandled) target
features in `ARM::getExtensionFeatures` and I am fixing it with this patch.
The second change this patch makes is that it teaches
`ARM::appendArchExtFeatures` to account dependencies when processing target
features: i.e. when you say `-march=armv8.1-m.main+mve.fp+nofp` it means
`mve.fp` should get discarded too.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D63936
Files:
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/test/CodeGen/arm-target-features.c
clang/test/Preprocessor/arm-target-features.c
llvm/include/llvm/Support/ARMTargetParser.def
llvm/lib/Support/ARMTargetParser.cpp
llvm/unittests/Support/TargetParserTest.cpp
Index: llvm/unittests/Support/TargetParserTest.cpp
===================================================================
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -571,17 +571,18 @@
TEST(TargetParserTest, ARMExtensionFeatures) {
std::map<unsigned, std::vector<StringRef>> Extensions;
- Extensions[ARM::AEK_CRC] = { "+crc", "-crc" };
- Extensions[ARM::AEK_DSP] = { "+dsp", "-dsp" };
+ for (auto &Ext : ARM::ARCHExtNames) {
+ if (Ext.Feature && Ext.NegFeature)
+ Extensions[Ext.ID] = { StringRef(Ext.Feature),
+ StringRef(Ext.NegFeature) };
+ }
+
Extensions[ARM::AEK_HWDIVARM] = { "+hwdiv-arm", "-hwdiv-arm" };
Extensions[ARM::AEK_HWDIVTHUMB] = { "+hwdiv", "-hwdiv" };
- Extensions[ARM::AEK_RAS] = { "+ras", "-ras" };
- Extensions[ARM::AEK_FP16FML] = { "+fp16fml", "-fp16fml" };
- Extensions[ARM::AEK_DOTPROD] = { "+dotprod", "-dotprod" };
std::vector<StringRef> Features;
- EXPECT_FALSE(AArch64::getExtensionFeatures(ARM::AEK_INVALID, Features));
+ EXPECT_FALSE(ARM::getExtensionFeatures(ARM::AEK_INVALID, Features));
for (auto &E : Extensions) {
// test +extension
@@ -598,7 +599,7 @@
Found = std::find(std::begin(Features), std::end(Features), E.second.at(1));
EXPECT_TRUE(Found != std::end(Features));
EXPECT_TRUE(Extensions.size() == Features.size());
- }
+ }
}
TEST(TargetParserTest, ARMFPUFeatures) {
Index: llvm/lib/Support/ARMTargetParser.cpp
===================================================================
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -409,30 +409,12 @@
if (Extensions == AEK_INVALID)
return false;
- if (Extensions & AEK_CRC)
- Features.push_back("+crc");
- else
- Features.push_back("-crc");
-
- if (Extensions & AEK_DSP)
- Features.push_back("+dsp");
- else
- Features.push_back("-dsp");
-
- if (Extensions & AEK_FP16FML)
- Features.push_back("+fp16fml");
- else
- Features.push_back("-fp16fml");
-
- if (Extensions & AEK_RAS)
- Features.push_back("+ras");
- else
- Features.push_back("-ras");
-
- if (Extensions & AEK_DOTPROD)
- Features.push_back("+dotprod");
- else
- Features.push_back("-dotprod");
+ for (const auto AE : ARCHExtNames) {
+ if ((Extensions & AE.ID) == AE.ID && AE.Feature)
+ Features.push_back(AE.Feature);
+ else if (AE.NegFeature)
+ Features.push_back(AE.NegFeature);
+ }
return getHWDivFeatures(Extensions, Features);
}
@@ -508,16 +490,30 @@
return ARM::FK_INVALID;
}
+static unsigned getAEKID(StringRef ArchExtName) {
+ for (const auto AE : ARM::ARCHExtNames)
+ if (AE.getName() == ArchExtName)
+ return AE.ID;
+ return ARM::AEK_INVALID;
+}
+
bool ARM::appendArchExtFeatures(
StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
std::vector<StringRef> &Features) {
- StringRef StandardFeature = getArchExtFeature(ArchExt);
- if (!StandardFeature.empty()) {
- Features.push_back(StandardFeature);
- return true;
- }
+ size_t StartingNumFeatures = Features.size();
const bool Negated = stripNegationPrefix(ArchExt);
+ unsigned ID = getAEKID(ArchExt);
+
+ if (ID == AEK_INVALID)
+ return false;
+
+ for (const auto AE : ARCHExtNames) {
+ if (Negated && (AE.ID & ID) == ID && AE.NegFeature)
+ Features.push_back(AE.NegFeature);
+ else if (AE.ID == ID && AE.Feature)
+ Features.push_back(AE.Feature);
+ }
if (CPU == "")
CPU = "generic";
@@ -537,7 +533,7 @@
}
return ARM::getFPUFeatures(FPUKind, Features);
}
- return false;
+ return StartingNumFeatures != Features.size();
}
StringRef ARM::getHWDivName(unsigned HWDivKind) {
Index: llvm/include/llvm/Support/ARMTargetParser.def
===================================================================
--- llvm/include/llvm/Support/ARMTargetParser.def
+++ llvm/include/llvm/Support/ARMTargetParser.def
@@ -148,6 +148,7 @@
ARM_ARCH_EXT_NAME("dotprod", ARM::AEK_DOTPROD, "+dotprod","-dotprod")
ARM_ARCH_EXT_NAME("dsp", ARM::AEK_DSP, "+dsp", "-dsp")
ARM_ARCH_EXT_NAME("fp", ARM::AEK_FP, nullptr, nullptr)
+ARM_ARCH_EXT_NAME("fp.dp", ARM::AEK_FP_DP, nullptr, nullptr)
ARM_ARCH_EXT_NAME("mve", ARM::AEK_SIMD, "+mve", "-mve")
ARM_ARCH_EXT_NAME("mve.fp", (ARM::AEK_SIMD | ARM::AEK_FP), "+mve.fp", "-mve.fp")
ARM_ARCH_EXT_NAME("idiv", (ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB), nullptr, nullptr)
Index: clang/test/Preprocessor/arm-target-features.c
===================================================================
--- clang/test/Preprocessor/arm-target-features.c
+++ clang/test/Preprocessor/arm-target-features.c
@@ -769,6 +769,14 @@
// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_SIMD32 1
// CHECK-V81M-MVE-FP: #define __ARM_FPV5__ 1
+// nofp discards mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nofp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP-NOFP %s
+// CHECK-V81M-MVEFP-NOFP-NOT: #define __ARM_FEATURE_MVE
+
+// nomve discards mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nomve -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP-NOMVE %s
+// CHECK-V81M-MVEFP-NOMVE-NOT: #define __ARM_FEATURE_MVE
+
// RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
// CHECK-V81A: #define __ARM_ARCH 8
// CHECK-V81A: #define __ARM_ARCH_8_1A__ 1
Index: clang/test/CodeGen/arm-target-features.c
===================================================================
--- clang/test/CodeGen/arm-target-features.c
+++ clang/test/CodeGen/arm-target-features.c
@@ -32,7 +32,7 @@
// RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m4 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V82
// RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m5 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V82
-// CHECK-BASIC-V82: "target-features"="+armv8.2-a,+crc,+crypto,+d32,+dotprod,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+hwdiv,+hwdiv-arm,+neon,+ras,+thumb-mode,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp"
+// CHECK-BASIC-V82: "target-features"="+armv8.2-a,+crc,+crypto,+d32,+dotprod,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+fullfp16,+hwdiv,+hwdiv-arm,+neon,+ras,+thumb-mode,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp"
// RUN: %clang_cc1 -triple armv8-linux-gnueabi -target-cpu cortex-a53 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V8-ARM
// CHECK-BASIC-V8-ARM: "target-features"="+armv8-a,+crc,+crypto,+d32,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+hwdiv,+hwdiv-arm,+neon,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp,-thumb-mode"
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -378,7 +378,11 @@
Features.push_back(
Args.MakeArgString((F.second ? "+" : "-") + F.first()));
} else if (!CPUName.empty()) {
- DecodeARMFeaturesFromCPU(D, CPUName, ExtensionFeatures);
+ // This sets the default features for the specified CPU. We certainly don't
+ // want to override the features that have been explicitly specified on the
+ // command line. Therefore, process them directly instead of appending them
+ // at the end later.
+ DecodeARMFeaturesFromCPU(D, CPUName, Features);
}
if (CPUArg)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
