Eric Christopher <echri...@gmail.com> writes:
> Can't be. We don't know we're going to make the call until we code gen. :)
Okay... but then why don't any tests fail with the attached? It looks
like it does the right thing in a very cursory test, as well.
commit 63d58fd0b4b77c9486901c103ec70c974d0c2553
Author: Justin Bogner <m...@justinbogner.com>
Date: Fri Oct 16 19:43:42 2015 -0700
wip: Move this to sema?
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index cce5405..4aa3e2a 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -8870,8 +8870,9 @@ private:
bool CheckObjCString(Expr *Arg);
- ExprResult CheckBuiltinFunctionCall(FunctionDecl *FDecl,
- unsigned BuiltinID, CallExpr *TheCall);
+ bool checkBuiltinTargetFeatures(const FunctionDecl *TargetDecl);
+ ExprResult CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
+ CallExpr *TheCall);
bool CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall,
unsigned MaxWidth);
diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp
index 6dff363..2e83782 100644
--- a/lib/CodeGen/CGBuiltin.cpp
+++ b/lib/CodeGen/CGBuiltin.cpp
@@ -289,62 +289,6 @@ Value *CodeGenFunction::EmitVAStartEnd(Value *ArgValue, bool IsStart) {
return Builder.CreateCall(CGM.getIntrinsic(inst), ArgValue);
}
-// Returns true if we have a valid set of target features.
-bool CodeGenFunction::checkBuiltinTargetFeatures(
- const FunctionDecl *TargetDecl) {
- // Early exit if this is an indirect call.
- if (!TargetDecl)
- return true;
-
- // Get the current enclosing function if it exists.
- if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {
- unsigned BuiltinID = TargetDecl->getBuiltinID();
- const char *FeatureList =
- CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
- if (FeatureList && StringRef(FeatureList) != "") {
- StringRef TargetCPU = Target.getTargetOpts().CPU;
- llvm::StringMap<bool> FeatureMap;
-
- 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();
-
- // Make a copy of the features as passed on the command line into the
- // beginning of the additional features from the function to override.
- ParsedAttr.first.insert(
- ParsedAttr.first.begin(),
- Target.getTargetOpts().FeaturesAsWritten.begin(),
- Target.getTargetOpts().FeaturesAsWritten.end());
-
- if (ParsedAttr.second != "")
- TargetCPU = ParsedAttr.second;
-
- // Now populate the feature map, first with the TargetCPU which is
- // either
- // the default or a new one from the target attribute string. Then we'll
- // use the passed in features (FeaturesAsWritten) along with the new
- // ones
- // from the attribute.
- Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
- ParsedAttr.first);
- } else {
- Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
- Target.getTargetOpts().Features);
- }
-
- // If we have at least one of the features in the feature list return
- // true, otherwise return false.
- SmallVector<StringRef, 1> AttrFeatures;
- StringRef(FeatureList).split(AttrFeatures, ",");
- for (const auto &Feature : AttrFeatures)
- if (FeatureMap[Feature])
- return true;
- return false;
- }
- }
- return true;
-}
-
RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
unsigned BuiltinID, const CallExpr *E,
ReturnValueSlot ReturnValue) {
@@ -1846,16 +1790,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
if (getContext().BuiltinInfo.isPredefinedLibFunction(BuiltinID))
return emitLibraryCall(*this, FD, E, EmitScalarExpr(E->getCallee()));
- // Check that a call to a target specific builtin has the correct target
- // features.
- // This is down here to avoid non-target specific builtins, however, if
- // generic builtins start to require generic target features then we
- // can move this up to the beginning of the function.
- if (!checkBuiltinTargetFeatures(FD))
- CGM.getDiags().Report(E->getLocStart(), diag::err_builtin_needs_feature)
- << FD->getDeclName()
- << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
-
// See if we have a target specific intrinsic.
const char *Name = getContext().BuiltinInfo.getName(BuiltinID);
Intrinsic::ID IntrinsicID = Intrinsic::not_intrinsic;
diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index d67e6f6..390e346 100644
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -2633,8 +2633,6 @@ public:
RValue EmitCallExpr(const CallExpr *E,
ReturnValueSlot ReturnValue = ReturnValueSlot());
- bool checkBuiltinTargetFeatures(const FunctionDecl *TargetDecl);
-
llvm::CallInst *EmitRuntimeCall(llvm::Value *callee,
const Twine &name = "");
llvm::CallInst *EmitRuntimeCall(llvm::Value *callee,
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index a8d882a..0376a9e 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -225,11 +225,76 @@ static bool SemaBuiltinSEHScopeCheck(Sema &SemaRef, CallExpr *TheCall,
return false;
}
+// Returns true if we have a valid set of target features.
+bool Sema::checkBuiltinTargetFeatures(const FunctionDecl *TargetDecl) {
+ // Early exit if this is an indirect call.
+ if (!TargetDecl)
+ return true;
+
+ auto &Target = Context.getTargetInfo();
+
+ // Get the current enclosing function if it exists.
+ if (const FunctionDecl *FD =
+ dyn_cast_or_null<FunctionDecl>(getCurFunctionDecl())) {
+ unsigned BuiltinID = TargetDecl->getBuiltinID();
+ const char *FeatureList =
+ Context.BuiltinInfo.getRequiredFeatures(BuiltinID);
+ if (FeatureList && StringRef(FeatureList) != "") {
+ StringRef TargetCPU = Target.getTargetOpts().CPU;
+ llvm::StringMap<bool> FeatureMap;
+
+ 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();
+
+ // Make a copy of the features as passed on the command line into the
+ // beginning of the additional features from the function to override.
+ ParsedAttr.first.insert(
+ ParsedAttr.first.begin(),
+ Target.getTargetOpts().FeaturesAsWritten.begin(),
+ Target.getTargetOpts().FeaturesAsWritten.end());
+
+ if (ParsedAttr.second != "")
+ TargetCPU = ParsedAttr.second;
+
+ // Now populate the feature map, first with the TargetCPU which is
+ // either
+ // the default or a new one from the target attribute string. Then we'll
+ // use the passed in features (FeaturesAsWritten) along with the new
+ // ones
+ // from the attribute.
+ Target.initFeatureMap(FeatureMap, Diags, TargetCPU,
+ ParsedAttr.first);
+ } else {
+ Target.initFeatureMap(FeatureMap, Diags, TargetCPU,
+ Target.getTargetOpts().Features);
+ }
+
+ // If we have at least one of the features in the feature list return
+ // true, otherwise return false.
+ SmallVector<StringRef, 1> AttrFeatures;
+ StringRef(FeatureList).split(AttrFeatures, ",");
+ for (const auto &Feature : AttrFeatures)
+ if (!FeatureMap[Feature])
+ return true;
+ return false;
+ }
+ }
+ return true;
+}
+
ExprResult
Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
CallExpr *TheCall) {
ExprResult TheCallResult(TheCall);
+ // Check that a call to a target specific builtin has the correct target
+ // features.
+ if (!checkBuiltinTargetFeatures(FDecl))
+ Diag(TheCall->getExprLoc(), diag::err_builtin_needs_feature)
+ << FDecl->getDeclName()
+ << Context.BuiltinInfo.getRequiredFeatures(BuiltinID);
+
// Find out if any arguments are required to be integer constant expressions.
unsigned ICEArguments = 0;
ASTContext::GetBuiltinTypeError Error;
> On Fri, Oct 16, 2015, 7:50 PM Justin Bogner <m...@justinbogner.com> wrote:
>
>> Eric Christopher via cfe-commits <cfe-commits@lists.llvm.org> writes:
>> > Author: echristo
>> > Date: Thu Oct 15 18:47:11 2015
>> > New Revision: 250473
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=250473&view=rev
>> > Log:
>> > Add an error when calling a builtin that requires features that don't
>> > match the feature set of the function that they're being called from.
>> >
>> > This ensures that we can effectively diagnose some[1] code that would
>> > instead ICE in the backend with a failure to select message.
>> >
>> > Example:
>> >
>> > __m128d foo(__m128d a, __m128d b) {
>> > return __builtin_ia32_addsubps(b, a);
>> > }
>> >
>> > compiled for normal x86_64 via:
>> >
>> > clang -target x86_64-linux-gnu -c
>> >
>> > would fail to compile in the back end because the normal subtarget
>> > features for x86_64 only include sse2 and the builtin requires sse3.
>> >
>> > [1] We're still not erroring on:
>> >
>> > __m128i bar(__m128i const *p) { return _mm_lddqu_si128(p); }
>> >
>> > where we should fail and error on an always_inline function being
>> > inlined into a function that doesn't support the subtarget features
>> > required.
>> >
>> > Added:
>> > cfe/trunk/test/CodeGen/target-builtin-error-2.c
>> > cfe/trunk/test/CodeGen/target-builtin-error.c
>> > cfe/trunk/test/CodeGen/target-builtin-noerror.c
>> > Modified:
>> > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> > cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> > cfe/trunk/lib/CodeGen/CGCall.cpp
>> > cfe/trunk/lib/CodeGen/CodeGenFunction.h
>> >
>> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=250473&r1=250472&r2=250473&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct 15
>> 18:47:11 2015
>> > @@ -430,6 +430,7 @@ def warn_redecl_library_builtin : Warnin
>> > def err_builtin_definition : Error<"definition of builtin function %0">;
>> > def err_arm_invalid_specialreg : Error<"invalid special register for
>> builtin">;
>> > def err_invalid_cpu_supports : Error<"invalid cpu feature string for
>> builtin">;
>> > +def err_builtin_needs_feature : Error<"%0 needs target feature %1">;
>> > def warn_builtin_unknown : Warning<"use of unknown builtin %0">,
>> > InGroup<ImplicitFunctionDeclare>, DefaultError;
>> > def warn_dyn_class_memaccess : Warning<
>> >
>> > Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=250473&r1=250472&r2=250473&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
>> > +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Oct 15 18:47:11 2015
>> > @@ -21,6 +21,7 @@
>> > #include "clang/Basic/TargetBuiltins.h"
>> > #include "clang/Basic/TargetInfo.h"
>> > #include "clang/CodeGen/CGFunctionInfo.h"
>> > +#include "clang/Sema/SemaDiagnostic.h"
>> > #include "llvm/ADT/StringExtras.h"
>> > #include "llvm/IR/CallSite.h"
>> > #include "llvm/IR/DataLayout.h"
>> > @@ -288,6 +289,62 @@ Value *CodeGenFunction::EmitVAStartEnd(V
>> > return Builder.CreateCall(CGM.getIntrinsic(inst), ArgValue);
>> > }
>> >
>> > +// Returns true if we have a valid set of target features.
>> > +bool CodeGenFunction::checkBuiltinTargetFeatures(
>> > + const FunctionDecl *TargetDecl) {
>> > + // Early exit if this is an indirect call.
>> > + if (!TargetDecl)
>> > + return true;
>> > +
>> > + // Get the current enclosing function if it exists.
>> > + if (const FunctionDecl *FD =
>> dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {
>> > + unsigned BuiltinID = TargetDecl->getBuiltinID();
>> > + const char *FeatureList =
>> > + CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
>> > + if (FeatureList && StringRef(FeatureList) != "") {
>> > + StringRef TargetCPU = Target.getTargetOpts().CPU;
>> > + llvm::StringMap<bool> FeatureMap;
>> > +
>> > + 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();
>> > +
>> > + // Make a copy of the features as passed on the command line
>> into the
>> > + // beginning of the additional features from the function to
>> override.
>> > + ParsedAttr.first.insert(
>> > + ParsedAttr.first.begin(),
>> > + Target.getTargetOpts().FeaturesAsWritten.begin(),
>> > + Target.getTargetOpts().FeaturesAsWritten.end());
>> > +
>> > + if (ParsedAttr.second != "")
>> > + TargetCPU = ParsedAttr.second;
>> > +
>> > + // Now populate the feature map, first with the TargetCPU which
>> is
>> > + // either
>> > + // the default or a new one from the target attribute string.
>> Then we'll
>> > + // use the passed in features (FeaturesAsWritten) along with
>> the new
>> > + // ones
>> > + // from the attribute.
>> > + Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
>> > + ParsedAttr.first);
>> > + } else {
>> > + Target.initFeatureMap(FeatureMap, CGM.getDiags(), TargetCPU,
>> > + Target.getTargetOpts().Features);
>> > + }
>> > +
>> > + // If we have at least one of the features in the feature list
>> return
>> > + // true, otherwise return false.
>> > + SmallVector<StringRef, 1> AttrFeatures;
>> > + StringRef(FeatureList).split(AttrFeatures, ",");
>> > + for (const auto &Feature : AttrFeatures)
>> > + if (FeatureMap[Feature])
>> > + return true;
>> > + return false;
>> > + }
>> > + }
>> > + return true;
>> > +}
>> > +
>> > RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
>> > unsigned BuiltinID, const
>> CallExpr *E,
>> > ReturnValueSlot ReturnValue) {
>> > @@ -1789,6 +1846,16 @@ RValue CodeGenFunction::EmitBuiltinExpr(
>> > if (getContext().BuiltinInfo.isPredefinedLibFunction(BuiltinID))
>> > return emitLibraryCall(*this, FD, E,
>> EmitScalarExpr(E->getCallee()));
>> >
>> > + // Check that a call to a target specific builtin has the correct
>> target
>> > + // features.
>> > + // This is down here to avoid non-target specific builtins, however,
>> if
>> > + // generic builtins start to require generic target features then we
>> > + // can move this up to the beginning of the function.
>> > + if (!checkBuiltinTargetFeatures(FD))
>> > + CGM.getDiags().Report(E->getLocStart(),
>> diag::err_builtin_needs_feature)
>> > + << FD->getDeclName()
>> > + << CGM.getContext().BuiltinInfo.getRequiredFeatures(BuiltinID);
>>
>> This check really belongs in Sema, not CodeGen. I'd expect it to be part
>> of CheckBuiltinFunctionCall in SemaChecking.cpp.
>>
>> > +
>> > // See if we have a target specific intrinsic.
>> > const char *Name = getContext().BuiltinInfo.getName(BuiltinID);
>> > Intrinsic::ID IntrinsicID = Intrinsic::not_intrinsic;
>> >
>> > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=250473&r1=250472&r2=250473&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
>> > +++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Oct 15 18:47:11 2015
>> > @@ -21,6 +21,7 @@
>> > #include "clang/AST/Decl.h"
>> > #include "clang/AST/DeclCXX.h"
>> > #include "clang/AST/DeclObjC.h"
>> > +#include "clang/Basic/TargetBuiltins.h"
>> > #include "clang/Basic/TargetInfo.h"
>> > #include "clang/CodeGen/CGFunctionInfo.h"
>> > #include "clang/Frontend/CodeGenOptions.h"
>> >
>> > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=250473&r1=250472&r2=250473&view=diff
>> >
>> ==============================================================================
>> > --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
>> > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Thu Oct 15 18:47:11 2015
>> > @@ -2633,6 +2633,8 @@ public:
>> > RValue EmitCallExpr(const CallExpr *E,
>> > ReturnValueSlot ReturnValue = ReturnValueSlot());
>> >
>> > + bool checkBuiltinTargetFeatures(const FunctionDecl *TargetDecl);
>> > +
>> > llvm::CallInst *EmitRuntimeCall(llvm::Value *callee,
>> > const Twine &name = "");
>> > llvm::CallInst *EmitRuntimeCall(llvm::Value *callee,
>> >
>> > Added: cfe/trunk/test/CodeGen/target-builtin-error-2.c
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-error-2.c?rev=250473&view=auto
>> >
>> ==============================================================================
>> > --- cfe/trunk/test/CodeGen/target-builtin-error-2.c (added)
>> > +++ cfe/trunk/test/CodeGen/target-builtin-error-2.c Thu Oct 15 18:47:11
>> 2015
>> > @@ -0,0 +1,13 @@
>> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -
>> > +#define __MM_MALLOC_H
>> > +
>> > +#include <x86intrin.h>
>> > +
>> > +// Since we do code generation on a function level this needs to error
>> out since
>> > +// the subtarget feature won't be available.
>> > +__m256d wombat(__m128i a) {
>> > + if (__builtin_cpu_supports("avx"))
>> > + return __builtin_ia32_cvtdq2pd256((__v4si)a); // expected-error
>> {{'__builtin_ia32_cvtdq2pd256' needs target feature avx}}
>> > + else
>> > + return (__m256d){0, 0, 0, 0};
>> > +}
>> >
>> > Added: cfe/trunk/test/CodeGen/target-builtin-error.c
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-error.c?rev=250473&view=auto
>> >
>> ==============================================================================
>> > --- cfe/trunk/test/CodeGen/target-builtin-error.c (added)
>> > +++ cfe/trunk/test/CodeGen/target-builtin-error.c Thu Oct 15 18:47:11
>> 2015
>> > @@ -0,0 +1,8 @@
>> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o -
>> > +#define __MM_MALLOC_H
>> > +
>> > +#include <x86intrin.h>
>> > +
>> > +__m128d foo(__m128d a, __m128d b) {
>> > + return __builtin_ia32_addsubps(b, a); // expected-error
>> {{'__builtin_ia32_addsubps' needs target feature sse3}}
>> > +}
>> >
>> > Added: cfe/trunk/test/CodeGen/target-builtin-noerror.c
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-builtin-noerror.c?rev=250473&view=auto
>> >
>> ==============================================================================
>> > --- cfe/trunk/test/CodeGen/target-builtin-noerror.c (added)
>> > +++ cfe/trunk/test/CodeGen/target-builtin-noerror.c Thu Oct 15 18:47:11
>> 2015
>> > @@ -0,0 +1,30 @@
>> > +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -o -
>> > +#define __MM_MALLOC_H
>> > +
>> > +#include <x86intrin.h>
>> > +
>> > +// No warnings.
>> > +extern __m256i a;
>> > +int __attribute__((target("avx"))) bar(__m256i a) {
>> > + return _mm256_extract_epi32(a, 3);
>> > +}
>> > +
>> > +int baz() {
>> > + return bar(a);
>> > +}
>> > +
>> > +int __attribute__((target("avx"))) qq_avx(__m256i a) {
>> > + return _mm256_extract_epi32(a, 3);
>> > +}
>> > +
>> > +int qq_noavx() {
>> > + return 0;
>> > +}
>> > +
>> > +extern __m256i a;
>> > +int qq() {
>> > + if (__builtin_cpu_supports("avx"))
>> > + return qq_avx(a);
>> > + else
>> > + return qq_noavx();
>> > +}
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
