craig.topper created this revision.
craig.topper added reviewers: echristo, erichkeane.
If an always inline function requests a different CPU than its caller we should
probably error.
If the callee CPU has features that the caller CPU doesn't we would already
error for the feature mismatch, but it prints a misleading error about the
first feature that mismatches.
If the callee CPU feature list a subset of the caller features we wouldn't
error at all.
We also only error right now if the callee as a target attribute, but don't
check anything if only the caller has a target attribute. This is consistent
with our previous checking behavior, but we might want to fix that. I've left a
TODO.
I've simplified some of GetCPUAndFeaturesAttributes since I needed to return
the CPU string from getFunctionFeatureMap for the other callers anyway. This
also saves us a second parse of the target attribute when adding attributes.
https://reviews.llvm.org/D46410
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
test/CodeGen/target-cpu-error-2.c
test/CodeGen/target-cpu-error-3.c
test/CodeGen/target-cpu-error.c
Index: test/CodeGen/target-cpu-error.c
===================================================================
--- /dev/null
+++ test/CodeGen/target-cpu-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -
+int __attribute__((target("arch=silvermont"), always_inline)) foo(int a) {
+ return a + 4;
+}
+int __attribute__((target("arch=nehalem"))) bar() {
+ return foo(4); // expected-error {{always_inline function 'foo' requires target CPU 'silvermont', but would be inlined into function 'bar' that is compiled for 'nehalem'}}
+}
+
Index: test/CodeGen/target-cpu-error-3.c
===================================================================
--- /dev/null
+++ test/CodeGen/target-cpu-error-3.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -target-cpu silvermont
+int __attribute__((target("sse4.2"), always_inline)) foo(int a) {
+ return a + 4;
+}
+int __attribute__((target("arch=nehalem"))) bar() {
+ return foo(4); // expected-error {{always_inline function 'foo' requires target CPU 'silvermont', but would be inlined into function 'bar' that is compiled for 'nehalem'}}
+}
+
Index: test/CodeGen/target-cpu-error-2.c
===================================================================
--- /dev/null
+++ test/CodeGen/target-cpu-error-2.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -target-cpu nehalem
+int __attribute__((target("arch=silvermont"), always_inline)) foo(int a) {
+ return a + 4;
+}
+int bar() {
+ return foo(4); // expected-error {{always_inline function 'foo' requires target CPU 'silvermont', but would be inlined into function 'bar' that is compiled for 'nehalem'}}
+}
+
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1083,9 +1083,10 @@
void AddDefaultFnAttrs(llvm::Function &F);
// Fills in the supplied string map with the set of target features for the
- // passed in function.
- void getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
- const FunctionDecl *FD);
+ // passed in function. Also fills in the target CPU.
+ void getFunctionCPUAndFeatureMap(llvm::StringMap<bool> &FeatureMap,
+ StringRef &TargetCPU,
+ const FunctionDecl *FD);
StringRef getMangledName(GlobalDecl GD);
StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1282,23 +1282,15 @@
bool AddedAttr = false;
if (TD) {
llvm::StringMap<bool> FeatureMap;
- getFunctionFeatureMap(FeatureMap, FD);
+ getFunctionCPUAndFeatureMap(FeatureMap, TargetCPU, FD);
// Produce the canonical string for this set of features.
for (const llvm::StringMap<bool>::value_type &Entry : FeatureMap)
Features.push_back((Entry.getValue() ? "+" : "-") + Entry.getKey().str());
-
- // Now add the target-cpu and target-features to the function.
- // While we populated the feature map above, we still need to
- // get and parse the target attribute so we can get the cpu for
- // the function.
- TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
- if (ParsedAttr.Architecture != "" &&
- getTarget().isValidCPUName(ParsedAttr.Architecture))
- TargetCPU = ParsedAttr.Architecture;
} else {
// Otherwise just add the existing target cpu and target features to the
// function.
+ TargetCPU = getTarget().getTargetOpts().CPU;
Features = getTarget().getTargetOpts().Features;
}
@@ -4988,9 +4980,11 @@
// Fills in the supplied string map with the set of target features for the
// passed in function.
-void CodeGenModule::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
- const FunctionDecl *FD) {
- StringRef TargetCPU = Target.getTargetOpts().CPU;
+void
+CodeGenModule::getFunctionCPUAndFeatureMap(llvm::StringMap<bool> &FeatureMap,
+ StringRef &TargetCPU,
+ const FunctionDecl *FD) {
+ TargetCPU = Target.getTargetOpts().CPU;
if (const auto *TD = FD->getAttr<TargetAttr>()) {
// If we have a TargetAttr build up the feature map based on that.
TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -2267,35 +2267,6 @@
CGF->InsertHelper(I, Name, BB, InsertPt);
}
-static bool hasRequiredFeatures(const SmallVectorImpl<StringRef> &ReqFeatures,
- CodeGenModule &CGM, const FunctionDecl *FD,
- std::string &FirstMissing) {
- // If there aren't any required features listed then go ahead and return.
- if (ReqFeatures.empty())
- return false;
-
- // Now build up the set of caller features and verify that all the required
- // features are there.
- llvm::StringMap<bool> CallerFeatureMap;
- CGM.getFunctionFeatureMap(CallerFeatureMap, FD);
-
- // If we have at least one of the features in the feature list return
- // true, otherwise return false.
- return std::all_of(
- ReqFeatures.begin(), ReqFeatures.end(), [&](StringRef Feature) {
- SmallVector<StringRef, 1> OrFeatures;
- Feature.split(OrFeatures, '|');
- return std::any_of(OrFeatures.begin(), OrFeatures.end(),
- [&](StringRef Feature) {
- if (!CallerFeatureMap.lookup(Feature)) {
- FirstMissing = Feature.str();
- return false;
- }
- return true;
- });
- });
-}
-
// Emits an error if we don't have a valid set of target features for the
// called function.
void CodeGenFunction::checkTargetFeatures(const CallExpr *E,
@@ -2313,34 +2284,68 @@
// Grab the required features for the call. For a builtin this is listed in
// the td file with the default cpu, for an always_inline function this is any
// listed cpu and any listed features.
+ SmallVector<StringRef, 1> ReqFeatures;
+ // CalleeFeatureMap needs to be at this scope to prevent ReqFeatures from
+ // having dangling StringRefs.
+ llvm::StringMap<bool> CalleeFeatureMap;
+ StringRef RequiredCPU;
unsigned BuiltinID = TargetDecl->getBuiltinID();
- std::string MissingFeature;
if (BuiltinID) {
- SmallVector<StringRef, 1> ReqFeatures;
const char *FeatureList =
CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
// Return if the builtin doesn't have any required features.
if (!FeatureList || StringRef(FeatureList) == "")
return;
StringRef(FeatureList).split(ReqFeatures, ',');
- if (!hasRequiredFeatures(ReqFeatures, CGM, FD, MissingFeature))
- CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature)
- << TargetDecl->getDeclName()
- << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
-
} else if (TargetDecl->hasAttr<TargetAttr>()) {
- // Get the required features for the callee.
- SmallVector<StringRef, 1> ReqFeatures;
- llvm::StringMap<bool> CalleeFeatureMap;
- CGM.getFunctionFeatureMap(CalleeFeatureMap, TargetDecl);
+ CGM.getFunctionCPUAndFeatureMap(CalleeFeatureMap, RequiredCPU, TargetDecl);
for (const auto &F : CalleeFeatureMap) {
// Only positive features are "required".
if (F.getValue())
ReqFeatures.push_back(F.getKey());
}
- if (!hasRequiredFeatures(ReqFeatures, CGM, FD, MissingFeature))
- CGM.getDiags().Report(E->getLocStart(), diag::err_function_needs_feature)
- << FD->getDeclName() << TargetDecl->getDeclName() << MissingFeature;
+ }
+ // TODO: What if the caller has a TargetAttr, but the callee doesn't.
+
+ // Now build up the set of caller features and verify that all the required
+ // features are there.
+ llvm::StringMap<bool> CallerFeatureMap;
+ StringRef CallerCPU;
+ CGM.getFunctionCPUAndFeatureMap(CallerFeatureMap, CallerCPU, FD);
+
+ // Make sure the CPUs match unless this is a builtin.
+ if (!BuiltinID && CallerCPU != RequiredCPU) {
+ CGM.getDiags().Report(E->getLocStart(), diag::err_function_needs_cpu)
+ << FD->getDeclName() << TargetDecl->getDeclName() << RequiredCPU
+ << CallerCPU;
+ return;
+ }
+
+ // If we have all of the required features, we're done.
+ std::string FirstMissing;
+ if (llvm::all_of(ReqFeatures,
+ [&](StringRef Feature) {
+ SmallVector<StringRef, 1> OrFeatures;
+ Feature.split(OrFeatures, '|');
+ return llvm::any_of(OrFeatures,
+ [&](StringRef Feature) {
+ if (!CallerFeatureMap.lookup(Feature)) {
+ FirstMissing = Feature.str();
+ return false;
+ }
+ return true;
+ });
+ }))
+ return;
+
+ // We have a missing feature, diagnose it.
+ if (BuiltinID) {
+ CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature)
+ << TargetDecl->getDeclName()
+ << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
+ } else {
+ CGM.getDiags().Report(E->getLocStart(), diag::err_function_needs_feature)
+ << FD->getDeclName() << TargetDecl->getDeclName() << FirstMissing;
}
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -611,6 +611,9 @@
: Error<"always_inline function %1 requires target feature '%2', but would "
"be inlined into function %0 that is compiled without support for "
"'%2'">;
+def err_function_needs_cpu
+ : Error<"always_inline function %1 requires target CPU '%2', but would "
+ "be inlined into function %0 that is compiled for '%3'">;
def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
InGroup<ImplicitFunctionDeclare>, DefaultError;
def warn_cstruct_memaccess : Warning<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
