Skip to content

Commit

Permalink
[ARM][TargetParser] Improve handling of dependencies between target f…
Browse files Browse the repository at this point in the history
…eatures

The patch at https://reviews.llvm.org/D64048 added "negative"
dependency handling in `ARM::appendArchExtFeatures`: feature "noX"
removes all features, which imply "X".

This patch adds the "positive" handling: feature "X" adds all the
feature strings implied by "X".

(This patch also comes from the suggestion here
https://reviews.llvm.org/D72633#inline-658582)

Differential Revision: https://reviews.llvm.org/D72762
  • Loading branch information
momchil-velikov committed Feb 5, 2020
1 parent 91b3083 commit 3627c91
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 10 deletions.
2 changes: 0 additions & 2 deletions clang/lib/Basic/Targets/ARM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -480,10 +480,8 @@ bool ARMTargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
} else if (Feature == "+dotprod") {
DotProd = true;
} else if (Feature == "+mve") {
DSP = 1;
MVE |= MVE_INT;
} else if (Feature == "+mve.fp") {
DSP = 1;
HasLegalHalfType = true;
FPU |= FPARMV8;
MVE |= MVE_INT | MVE_FP;
Expand Down
4 changes: 3 additions & 1 deletion clang/test/Driver/arm-mfpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -414,12 +414,14 @@
// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-d32"
// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-neon"
// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-crypto"
// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "+mve"
// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "+dsp"
// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-mve.fp"
// CHECK-MVEFP-FPUNONE-NOT: "-target-feature" "-fpregs"


// RUN: %clang -target arm-none-none-eabi %s -march=armv8.1-m.main+mve -mfpu=none -### -c 2>&1 \
// RUN: | FileCheck --check-prefix=CHECK-MVEI-FPUNONE %s
// CHECK-MVEI-FPUNONE-DAG: "-target-feature" "-mve.fp"
// CHECK-MVEI-FPUNONE-DAG: "-target-feature" "+mve"
// CHECK-MVEI-FPUNONE-DAG: "-target-feature" "+dsp"
// CHECK-MVEI-FPUNONE-NOT: "-target-feature" "-fpregs"
12 changes: 9 additions & 3 deletions clang/test/Preprocessor/arm-target-features.c
Original file line number Diff line number Diff line change
Expand Up @@ -761,9 +761,10 @@
// CHECK-V81M-MVEFP: #define __ARM_FEATURE_SIMD32 1
// CHECK-V81M-MVEFP: #define __ARM_FPV5__ 1

// nofp discards mve.fp
// nofp discards mve.fp, but not mve/dsp
// 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
// CHECK-V81M-MVEFP-NOFP: #define __ARM_FEATURE_DSP 1
// CHECK-V81M-MVEFP-NOFP: #define __ARM_FEATURE_MVE 1

// 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
Expand All @@ -773,11 +774,16 @@
// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 1

// nodsp discards both dsp and mve
// nodsp discards both dsp and mve ...
// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+nodsp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-NODSP %s
// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP

// ... and also mve.fp
// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nodsp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-NODSP %s
// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP

// 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
Expand Down
11 changes: 7 additions & 4 deletions llvm/lib/Support/ARMTargetParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,13 @@ bool ARM::appendArchExtFeatures(
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 (Negated) {
if ((AE.ID & ID) == ID && AE.NegFeature)
Features.push_back(AE.NegFeature);
} else {
if ((AE.ID & ID) == AE.ID && AE.Feature)
Features.push_back(AE.Feature);
}
}

if (CPU == "")
Expand Down
21 changes: 21 additions & 0 deletions llvm/unittests/Support/TargetParserTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,27 @@ TEST(TargetParserTest, ARMArchExtFeature) {
}
}

static bool
testArchExtDependency(const char *ArchExt,
const std::initializer_list<const char *> &Expected) {
std::vector<StringRef> Features;

if (!ARM::appendArchExtFeatures("", ARM::ArchKind::ARMV8_1MMainline, ArchExt,
Features))
return false;

return llvm::all_of(Expected, [&](StringRef Ext) {
return llvm::is_contained(Features, Ext);
});
}

TEST(TargetParserTest, ARMArchExtDependencies) {
EXPECT_TRUE(testArchExtDependency("mve", {"+mve", "+dsp"}));
EXPECT_TRUE(testArchExtDependency("mve.fp", {"+mve.fp", "+mve", "+dsp"}));
EXPECT_TRUE(testArchExtDependency("nodsp", {"-dsp", "-mve", "-mve.fp"}));
EXPECT_TRUE(testArchExtDependency("nomve", {"-mve", "-mve.fp"}));
}

TEST(TargetParserTest, ARMparseHWDiv) {
const char *hwdiv[] = {"thumb", "arm", "arm,thumb", "thumb,arm"};

Expand Down

0 comments on commit 3627c91

Please sign in to comment.