This revision was automatically updated to reflect the committed changes.
erichkeane marked an inline comment as done.
Closed by commit rG2831a317b689: Implement AVX ABI Warning/error (authored by
erichkeane).
Herald added a project: clang.
Changed prior to commit:
https://reviews.llvm.org/D82562?vs=274450&id=274800#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82562/new/
https://reviews.llvm.org/D82562
Files:
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/CodeGen/TargetInfo.h
clang/test/CodeGen/target-avx-abi-diag.c
clang/test/CodeGen/target-builtin-error-3.c
clang/test/CodeGen/target-builtin-noerror.c
Index: clang/test/CodeGen/target-builtin-noerror.c
===================================================================
--- clang/test/CodeGen/target-builtin-noerror.c
+++ 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();
}
Index: clang/test/CodeGen/target-builtin-error-3.c
===================================================================
--- clang/test/CodeGen/target-builtin-error-3.c
+++ 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);
}
Index: clang/test/CodeGen/target-avx-abi-diag.c
===================================================================
--- /dev/null
+++ 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);
+}
Index: clang/lib/CodeGen/TargetInfo.h
===================================================================
--- clang/lib/CodeGen/TargetInfo.h
+++ clang/lib/CodeGen/TargetInfo.h
@@ -63,6 +63,13 @@
CodeGen::CodeGenModule &CGM,
const llvm::MapVector<GlobalDecl, StringRef> &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 {
Index: clang/lib/CodeGen/TargetInfo.cpp
===================================================================
--- clang/lib/CodeGen/TargetInfo.cpp
+++ 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<bool> &CallerMap,
+ const FunctionDecl *Caller,
+ llvm::StringMap<bool> &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<bool> &CallerMap,
+ const llvm::StringMap<bool> &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<bool> &CallerMap,
+ const llvm::StringMap<bool> &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<bool> CallerMap;
+ llvm::StringMap<bool> 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.
Index: clang/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ 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<FunctionDecl>(TargetDecl))
+ if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(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<TargetAttr>())
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<FunctionDecl>(CurCodeDecl), FD, CallArgs);
+ }
+
#ifndef NDEBUG
if (!(CallInfo.isVariadic() && CallInfo.getArgStruct())) {
// For an inalloca varargs function, we don't expect CallInfo to match the
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ 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<DiagGroup<"psabi">>;
+def err_avx_calling_convention : Error<warn_avx_calling_convention.Text>;
+
def err_alias_to_undefined : Error<
"%select{alias|ifunc}0 must point to a defined "
"%select{variable or |}1function">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
