george.burgess.iv created this revision.
george.burgess.iv added reviewers: michaelplatings, efriedma.
Herald added subscribers: kristof.beyls, javed.absar.
Herald added a project: clang.
I'm not convinced this is an excellent approach, in part because I'm unclear on
where all we expect to funnel the results of `TargetInfo::initFeatureMap`.
Thought I'd throw it out for review anyway, and see what people with actual
context think. :)
The underlying problem I'm trying to solve is that, given the following code
with a suitable ARM target,
___attribute__((target("crc"))) void foo() {}
clang ends up codegening a function with the '+soft-float-abi' target feature,
which we go out of our way to remove in the frontend for the default target
features, since the backend apparently tries to figure out whether the soft
float ABI should be used on its own. This is more or less harmless on its own,
but it causes the backend to emit a diagnostic directly to `errs()`. Rather
than try to find a way to silence that diag, it seems better to not have to
emit it in the first place.
An alternate fix would be to somehow try to filter this in cfe/lib/CodeGen, but
there appear to be many callers of `initFeatureMap`; I get the vague feeling
that we shouldn't be letting `+soft-float-abi` slip through any of them. Again,
could be wrong about that due to my lack of familiarity with `initFeatureMap`'s
uses.
If we agree that this is a good way forward, there also appears to be
`+neonfp`/`-neonfp` additions happening in `handleTargetFeatures` that should
prooooobably be happening in `initFeatureMap` instead? If that's the case, I'm
happy to do that as a follow-up, and make it so that `handleTargetFeatures` no
longer mutates its input (which comments indicate would be desirable, along
with a more general move of all of this initialization stuff to the
construction of our various `TargetInfo` subclasses).
Repository:
rC Clang
https://reviews.llvm.org/D61750
Files:
lib/Basic/Targets/ARM.cpp
test/CodeGen/arm-soft-float-abi-filtering.c
Index: test/CodeGen/arm-soft-float-abi-filtering.c
===================================================================
--- /dev/null
+++ test/CodeGen/arm-soft-float-abi-filtering.c
@@ -0,0 +1,7 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang_cc1 -emit-llvm -o - -triple arm-none-linux-gnueabi
-target-feature "+soft-float-abi" %s | FileCheck %s
+
+// CHECK-NOT: +soft-float-abi
+
+void foo() {}
+__attribute__((target("crc"))) void bar() {}
Index: lib/Basic/Targets/ARM.cpp
===================================================================
--- lib/Basic/Targets/ARM.cpp
+++ lib/Basic/Targets/ARM.cpp
@@ -313,6 +313,8 @@
this->MCountName = Opts.EABIVersion == llvm::EABI::GNU
? "\01__gnu_mcount_nc"
: "\01mcount";
+
+ SoftFloatABI = llvm::is_contained(Opts.FeaturesAsWritten, "+soft-float-abi");
}
StringRef ARMTargetInfo::getABI() const { return ABI; }
@@ -375,12 +377,21 @@
// Convert user-provided arm and thumb GNU target attributes to
// [-|+]thumb-mode target features respectively.
- std::vector<std::string> UpdatedFeaturesVec(FeaturesVec);
- for (auto &Feature : UpdatedFeaturesVec) {
- if (Feature.compare("+arm") == 0)
- Feature = "-thumb-mode";
- else if (Feature.compare("+thumb") == 0)
- Feature = "+thumb-mode";
+ std::vector<std::string> UpdatedFeaturesVec;
+ for (const auto &Feature : FeaturesVec) {
+ // Skip soft-float-abi; it's something we only use to initialize a bit of
+ // class state, and is otherwise unrecognized.
+ if (Feature == "+soft-float-abi")
+ continue;
+
+ StringRef FixedFeature;
+ if (Feature == "+arm")
+ FixedFeature = "-thumb-mode";
+ else if (Feature == "+thumb")
+ FixedFeature = "+thumb-mode";
+ else
+ FixedFeature = Feature;
+ UpdatedFeaturesVec.push_back(FixedFeature.str());
}
return TargetInfo::initFeatureMap(Features, Diags, CPU, UpdatedFeaturesVec);
@@ -394,7 +405,8 @@
Crypto = 0;
DSP = 0;
Unaligned = 1;
- SoftFloat = SoftFloatABI = false;
+ SoftFloat = false;
+ // Note that SoftFloatABI is initialized in our constructor.
HWDiv = 0;
DotProd = 0;
HasFloat16 = true;
@@ -405,8 +417,6 @@
for (const auto &Feature : Features) {
if (Feature == "+soft-float") {
SoftFloat = true;
- } else if (Feature == "+soft-float-abi") {
- SoftFloatABI = true;
} else if (Feature == "+vfp2") {
FPU |= VFP2FPU;
HW_FP |= HW_FP_SP | HW_FP_DP;
@@ -475,11 +485,6 @@
else if (FPMath == FP_VFP)
Features.push_back("-neonfp");
- // Remove front-end specific options which the backend handles differently.
- auto Feature = llvm::find(Features, "+soft-float-abi");
- if (Feature != Features.end())
- Features.erase(Feature);
-
return true;
}
Index: test/CodeGen/arm-soft-float-abi-filtering.c
===================================================================
--- /dev/null
+++ test/CodeGen/arm-soft-float-abi-filtering.c
@@ -0,0 +1,7 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang_cc1 -emit-llvm -o - -triple arm-none-linux-gnueabi -target-feature "+soft-float-abi" %s | FileCheck %s
+
+// CHECK-NOT: +soft-float-abi
+
+void foo() {}
+__attribute__((target("crc"))) void bar() {}
Index: lib/Basic/Targets/ARM.cpp
===================================================================
--- lib/Basic/Targets/ARM.cpp
+++ lib/Basic/Targets/ARM.cpp
@@ -313,6 +313,8 @@
this->MCountName = Opts.EABIVersion == llvm::EABI::GNU
? "\01__gnu_mcount_nc"
: "\01mcount";
+
+ SoftFloatABI = llvm::is_contained(Opts.FeaturesAsWritten, "+soft-float-abi");
}
StringRef ARMTargetInfo::getABI() const { return ABI; }
@@ -375,12 +377,21 @@
// Convert user-provided arm and thumb GNU target attributes to
// [-|+]thumb-mode target features respectively.
- std::vector<std::string> UpdatedFeaturesVec(FeaturesVec);
- for (auto &Feature : UpdatedFeaturesVec) {
- if (Feature.compare("+arm") == 0)
- Feature = "-thumb-mode";
- else if (Feature.compare("+thumb") == 0)
- Feature = "+thumb-mode";
+ std::vector<std::string> UpdatedFeaturesVec;
+ for (const auto &Feature : FeaturesVec) {
+ // Skip soft-float-abi; it's something we only use to initialize a bit of
+ // class state, and is otherwise unrecognized.
+ if (Feature == "+soft-float-abi")
+ continue;
+
+ StringRef FixedFeature;
+ if (Feature == "+arm")
+ FixedFeature = "-thumb-mode";
+ else if (Feature == "+thumb")
+ FixedFeature = "+thumb-mode";
+ else
+ FixedFeature = Feature;
+ UpdatedFeaturesVec.push_back(FixedFeature.str());
}
return TargetInfo::initFeatureMap(Features, Diags, CPU, UpdatedFeaturesVec);
@@ -394,7 +405,8 @@
Crypto = 0;
DSP = 0;
Unaligned = 1;
- SoftFloat = SoftFloatABI = false;
+ SoftFloat = false;
+ // Note that SoftFloatABI is initialized in our constructor.
HWDiv = 0;
DotProd = 0;
HasFloat16 = true;
@@ -405,8 +417,6 @@
for (const auto &Feature : Features) {
if (Feature == "+soft-float") {
SoftFloat = true;
- } else if (Feature == "+soft-float-abi") {
- SoftFloatABI = true;
} else if (Feature == "+vfp2") {
FPU |= VFP2FPU;
HW_FP |= HW_FP_SP | HW_FP_DP;
@@ -475,11 +485,6 @@
else if (FPMath == FP_VFP)
Features.push_back("-neonfp");
- // Remove front-end specific options which the backend handles differently.
- auto Feature = llvm::find(Features, "+soft-float-abi");
- if (Feature != Features.end())
- Features.erase(Feature);
-
return true;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
