diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -240,6 +240,12 @@ "always_inline function %1 requires target feature '%2', but would " "be inlined into function %0 that is compiled without support for '%2'">; +def warn_avx_calling_convention + : Warning<"AVX vector %select{return|argument}0 of type %1 without '%2' " + "enabled changes the ABI">, + InGroup>; +def err_avx_calling_convention : Error; + def err_alias_to_undefined : Error< "%select{alias|ifunc}0 must point to a defined " "%select{variable or |}1function">; diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -4245,7 +4245,7 @@ llvm::FunctionType *IRFuncTy = getTypes().GetFunctionType(CallInfo); const Decl *TargetDecl = Callee.getAbstractInfo().getCalleeDecl().getDecl(); - if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) { // We can only guarantee that a function is called from the correct // context/function based on the appropriate target attributes, // so only check in the case where we have both always_inline and target @@ -4256,6 +4256,12 @@ TargetDecl->hasAttr()) checkTargetFeatures(Loc, FD); + // Some architectures (such as x86-64) have the ABI changed based on + // attribute-target/features. Give them a chance to diagnose. + CGM.getTargetCodeGenInfo().checkFunctionCallABI( + CGM, Loc, dyn_cast_or_null(CurCodeDecl), FD, CallArgs); + } + #ifndef NDEBUG if (!(CallInfo.isVariadic() && CallInfo.getArgStruct())) { // For an inalloca varargs function, we don't expect CallInfo to match the diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h --- a/clang/lib/CodeGen/TargetInfo.h +++ b/clang/lib/CodeGen/TargetInfo.h @@ -63,6 +63,13 @@ CodeGen::CodeGenModule &CGM, const llvm::MapVector &MangledDeclNames) const {} + /// Any further codegen related checks that need to be done on a function call + /// in a target specific manner. + virtual void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc, + const FunctionDecl *Caller, + const FunctionDecl *Callee, + const CallArgList &Args) const {} + /// Determines the size of struct _Unwind_Exception on this platform, /// in 8-bit units. The Itanium ABI defines this as: /// struct _Unwind_Exception { diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp --- a/clang/lib/CodeGen/TargetInfo.cpp +++ b/clang/lib/CodeGen/TargetInfo.cpp @@ -20,6 +20,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/RecordLayout.h" #include "clang/Basic/CodeGenOptions.h" +#include "clang/Basic/DiagnosticFrontend.h" #include "clang/CodeGen/CGFunctionInfo.h" #include "clang/CodeGen/SwiftCallingConv.h" #include "llvm/ADT/SmallBitVector.h" @@ -2466,8 +2467,110 @@ } } } + + void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc, + const FunctionDecl *Caller, + const FunctionDecl *Callee, + const CallArgList &Args) const override; }; +static void initFeatureMaps(const ASTContext &Ctx, + llvm::StringMap &CallerMap, + const FunctionDecl *Caller, + llvm::StringMap &CalleeMap, + const FunctionDecl *Callee) { + if (CalleeMap.empty() && CallerMap.empty()) { + // The caller is potentially nullptr in the case where the call isn't in a + // function. In this case, the getFunctionFeatureMap ensures we just get + // the TU level setting (since it cannot be modified by 'target'.. + Ctx.getFunctionFeatureMap(CallerMap, Caller); + Ctx.getFunctionFeatureMap(CalleeMap, Callee); + } +} + +static bool checkAVXParamFeature(DiagnosticsEngine &Diag, + SourceLocation CallLoc, + const llvm::StringMap &CallerMap, + const llvm::StringMap &CalleeMap, + QualType Ty, StringRef Feature, + bool IsArgument) { + bool CallerHasFeat = CallerMap.lookup(Feature); + bool CalleeHasFeat = CalleeMap.lookup(Feature); + if (!CallerHasFeat && !CalleeHasFeat) + return Diag.Report(CallLoc, diag::warn_avx_calling_convention) + << IsArgument << Ty << Feature; + + // Mixing calling conventions here is very clearly an error. + if (!CallerHasFeat || !CalleeHasFeat) + return Diag.Report(CallLoc, diag::err_avx_calling_convention) + << IsArgument << Ty << Feature; + + // Else, both caller and callee have the required feature, so there is no need + // to diagnose. + return false; +} + +static bool checkAVXParam(DiagnosticsEngine &Diag, ASTContext &Ctx, + SourceLocation CallLoc, + const llvm::StringMap &CallerMap, + const llvm::StringMap &CalleeMap, QualType Ty, + bool IsArgument) { + uint64_t Size = Ctx.getTypeSize(Ty); + if (Size > 256) + return checkAVXParamFeature(Diag, CallLoc, CallerMap, CalleeMap, Ty, + "avx512f", IsArgument); + + if (Size > 128) + return checkAVXParamFeature(Diag, CallLoc, CallerMap, CalleeMap, Ty, "avx", + IsArgument); + + return false; +} + +void X86_64TargetCodeGenInfo::checkFunctionCallABI( + CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller, + const FunctionDecl *Callee, const CallArgList &Args) const { + llvm::StringMap CallerMap; + llvm::StringMap CalleeMap; + unsigned ArgIndex = 0; + + // We need to loop through the actual call arguments rather than the the + // function's parameters, in case this variadic. + for (const CallArg &Arg : Args) { + // The "avx" feature changes how vectors >128 in size are passed. "avx512f" + // additionally changes how vectors >256 in size are passed. Like GCC, we + // warn when a function is called with an argument where this will change. + // Unlike GCC, we also error when it is an obvious ABI mismatch, that is, + // the caller and callee features are mismatched. + // Unfortunately, we cannot do this diagnostic in SEMA, since the callee can + // change its ABI with attribute-target after this call. + if (Arg.getType()->isVectorType() && + CGM.getContext().getTypeSize(Arg.getType()) > 128) { + initFeatureMaps(CGM.getContext(), CallerMap, Caller, CalleeMap, Callee); + QualType Ty = Arg.getType(); + // The CallArg seems to have desugared the type already, so for clearer + // diagnostics, replace it with the type in the FunctionDecl if possible. + if (ArgIndex < Callee->getNumParams()) + Ty = Callee->getParamDecl(ArgIndex)->getType(); + + if (checkAVXParam(CGM.getDiags(), CGM.getContext(), CallLoc, CallerMap, + CalleeMap, Ty, /*IsArgument*/ true)) + return; + } + ++ArgIndex; + } + + // Check return always, as we don't have a good way of knowing in codegen + // whether this value is used, tail-called, etc. + if (Callee->getReturnType()->isVectorType() && + CGM.getContext().getTypeSize(Callee->getReturnType()) > 128) { + initFeatureMaps(CGM.getContext(), CallerMap, Caller, CalleeMap, Callee); + checkAVXParam(CGM.getDiags(), CGM.getContext(), CallLoc, CallerMap, + CalleeMap, Callee->getReturnType(), + /*IsArgument*/ false); + } +} + static std::string qualifyWindowsLibrary(llvm::StringRef Lib) { // If the argument does not end in .lib, automatically add the suffix. // If the argument contains a space, enclose it in quotes. diff --git a/clang/test/CodeGen/target-avx-abi-diag.c b/clang/test/CodeGen/target-avx-abi-diag.c new file mode 100644 --- /dev/null +++ b/clang/test/CodeGen/target-avx-abi-diag.c @@ -0,0 +1,50 @@ +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -verify=no256,no512 -o - -S +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +avx -verify=no512 -o - -S +// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +avx512f -verify=both -o - -S + +// both-no-diagnostics + +typedef short avx512fType __attribute__((vector_size(64))); +typedef short avx256Type __attribute__((vector_size(32))); + +__attribute__((target("avx"))) void takesAvx256(avx256Type t); +__attribute__((target("avx512f"))) void takesAvx512(avx512fType t); +void takesAvx256_no_target(avx256Type t); +void takesAvx512_no_target(avx512fType t); + +void variadic(int i, ...); +__attribute__((target("avx512f"))) void variadic_err(int i, ...); + +// If neither side has an attribute, warn. +void call_warn(void) { + avx256Type t1; + takesAvx256_no_target(t1); // no256-warning {{AVX vector argument of type 'avx256Type' (vector of 16 'short' values) without 'avx' enabled changes the ABI}} + + avx512fType t2; + takesAvx512_no_target(t2); // no512-warning {{AVX vector argument of type 'avx512fType' (vector of 32 'short' values) without 'avx512f' enabled changes the ABI}} + + variadic(1, t1); // no256-warning {{AVX vector argument of type 'avx256Type' (vector of 16 'short' values) without 'avx' enabled changes the ABI}} + variadic(3, t2); // no512-warning {{AVX vector argument of type 'avx512fType' (vector of 32 'short' values) without 'avx512f' enabled changes the ABI}} +} + +// If only 1 side has an attribute, error. +void call_errors(void) { + avx256Type t1; + takesAvx256(t1); // no256-error {{AVX vector argument of type 'avx256Type' (vector of 16 'short' values) without 'avx' enabled changes the ABI}} + avx512fType t2; + takesAvx512(t2); // no512-error {{AVX vector argument of type 'avx512fType' (vector of 32 'short' values) without 'avx512f' enabled changes the ABI}} + + variadic_err(1, t1); // no256-error {{AVX vector argument of type 'avx256Type' (vector of 16 'short' values) without 'avx' enabled changes the ABI}} + variadic_err(3, t2); // no512-error {{AVX vector argument of type 'avx512fType' (vector of 32 'short' values) without 'avx512f' enabled changes the ABI}} +} + +// These two don't diagnose anything, since these are valid calls. +__attribute__((target("avx"))) void call_avx256_ok(void) { + avx256Type t; + takesAvx256(t); +} + +__attribute__((target("avx512f"))) void call_avx512_ok(void) { + avx512fType t; + takesAvx512(t); +} diff --git a/clang/test/CodeGen/target-builtin-error-3.c b/clang/test/CodeGen/target-builtin-error-3.c --- a/clang/test/CodeGen/target-builtin-error-3.c +++ b/clang/test/CodeGen/target-builtin-error-3.c @@ -18,11 +18,12 @@ return __extension__ ({ __m256 __a = (a); (__m128i)__builtin_ia32_vcvtps2ph256((__v8sf)__a, (0x00)); }); // expected-error {{'__builtin_ia32_vcvtps2ph256' needs target feature f16c}} } static inline half16 __attribute__((__overloadable__)) convert_half( float16 a ) { - half16 r; - r.lo = convert_half( a.lo); + half16 r; + r.lo = convert_half(a.lo); return r; } void avx_test( uint16_t *destData, float16 argbF) { - ((half16U*)destData)[0] = convert_half(argbF); + // expected-warning@+1{{AVX vector argument of type 'float16' (vector of 16 'float' values) without 'avx512f' enabled changes the ABI}} + ((half16U *)destData)[0] = convert_half(argbF); } diff --git a/clang/test/CodeGen/target-builtin-noerror.c b/clang/test/CodeGen/target-builtin-noerror.c --- a/clang/test/CodeGen/target-builtin-noerror.c +++ b/clang/test/CodeGen/target-builtin-noerror.c @@ -6,15 +6,15 @@ // No warnings. extern __m256i a; -int __attribute__((target("avx"))) bar(__m256i a) { +int __attribute__((target("avx"))) bar() { return _mm256_extract_epi32(a, 3); } int baz() { - return bar(a); + return bar(); } -int __attribute__((target("avx"))) qq_avx(__m256i a) { +int __attribute__((target("avx"))) qq_avx() { return _mm256_extract_epi32(a, 3); } @@ -25,7 +25,7 @@ extern __m256i a; int qq() { if (__builtin_cpu_supports("avx")) - return qq_avx(a); + return qq_avx(); else return qq_noavx(); }