Skip to content

[Clang][AArch64] Warn when calling streaming/non-streaming about vect… #79842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Apr 10, 2024
Merged
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -1412,6 +1412,9 @@ def MultiGPU: DiagGroup<"multi-gpu">;
// libc and the CRT to be skipped.
def AVRRtlibLinkingQuirks : DiagGroup<"avr-rtlib-linking-quirks">;

// A warning group related to AArch64 SME function attribues.
def AArch64SMEAttributes : DiagGroup<"aarch64-sme-attributes">;

// A warning group for things that will change semantics in the future.
def FutureCompat : DiagGroup<"future-compat">;

Expand Down
10 changes: 10 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -3755,6 +3755,16 @@ def err_sme_definition_using_za_in_non_sme_target : Error<
"function using ZA state requires 'sme'">;
def err_sme_definition_using_zt0_in_non_sme2_target : Error<
"function using ZT0 state requires 'sme2'">;
def warn_sme_streaming_pass_return_vl_to_non_streaming : Warning<
"passing a VL-dependent argument to/from a function that has a different"
" streaming-mode. The streaming and non-streaming vector lengths may be"
" different">,
InGroup<AArch64SMEAttributes>, DefaultIgnore;
def warn_sme_locally_streaming_has_vl_args_returns : Warning<
"passing/returning a VL-dependent argument to/from a __arm_locally_streaming"
" function. The streaming and non-streaming vector"
" lengths may be different">,
InGroup<AArch64SMEAttributes>, DefaultIgnore;
Comment on lines +3758 to +3767
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would prefer it if you could give these warnings a 'bool' operand, such that it would give a message about either the return value or the operand, for example:

def warn_sme_streaming_pass_return_vl_to_non_streaming : Warning<
  "%select{passing|returning}0 a VL-dependent argument %select{to|from}0 a function with a different"
  " streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime">,
  InGroup<AArch64SMEAttributes>, DefaultIgnore;

And then emit the diagnostic like this:

if (ScalableArgs)
  Diag(Loc, diag::warn_sme_streaming_pass_return_vl_to_non_streaming) << /*IsArg=*/true;
if (ScalableRet)
  Diag(Loc, diag::warn_sme_streaming_pass_return_vl_to_non_streaming) << /*IsArg=*/false;

This results in:

warning: returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime

and

warning: passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime

I won't hold up the patch for it, but I would appreciate it if you could do this either as part of this PR or as a follow-up patch.

def err_conflicting_attributes_arm_state : Error<
"conflicting attributes for state '%0'">;
def err_sme_streaming_cannot_be_multiversioned : Error<
Expand Down
22 changes: 21 additions & 1 deletion clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7938,6 +7938,7 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
// For variadic functions, we may have more args than parameters.
// For some K&R functions, we may have less args than parameters.
const auto N = std::min<unsigned>(Proto->getNumParams(), Args.size());
bool AnyScalableArgsOrRet = Proto->getReturnType()->isSizelessVectorType();
for (unsigned ArgIdx = 0; ArgIdx < N; ++ArgIdx) {
// Args[ArgIdx] can be null in malformed code.
if (const Expr *Arg = Args[ArgIdx]) {
Expand All @@ -7951,6 +7952,8 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
checkAIXMemberAlignment((Arg->getExprLoc()), Arg);

QualType ParamTy = Proto->getParamType(ArgIdx);
if (ParamTy->isSizelessVectorType())
AnyScalableArgsOrRet = true;
QualType ArgTy = Arg->getType();
CheckArgAlignment(Arg->getExprLoc(), FDecl, std::to_string(ArgIdx + 1),
ArgTy, ParamTy);
Expand All @@ -7971,6 +7974,23 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
}
}

// If the call requires a streaming-mode change and has scalable vector
// arguments or return values, then warn the user that the streaming and
// non-streaming vector lengths may be different.
const auto *CallerFD = dyn_cast<FunctionDecl>(CurContext);
if (CallerFD && (!FD || !FD->getBuiltinID()) && AnyScalableArgsOrRet) {
bool IsCalleeStreaming =
ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateSMEnabledMask;
bool IsCalleeStreamingCompatible =
ExtInfo.AArch64SMEAttributes &
FunctionType::SME_PStateSMCompatibleMask;
ArmStreamingType CallerFnType = getArmStreamingFnType(CallerFD);
if (!IsCalleeStreamingCompatible &&
(CallerFnType == ArmStreamingCompatible ||
((CallerFnType == ArmStreaming) ^ IsCalleeStreaming)))
Diag(Loc, diag::warn_sme_streaming_pass_return_vl_to_non_streaming);
}

FunctionType::ArmStateValue CalleeArmZAState =
FunctionType::getArmZAState(ExtInfo.AArch64SMEAttributes);
FunctionType::ArmStateValue CalleeArmZT0State =
Expand All @@ -7979,7 +7999,7 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
CalleeArmZT0State != FunctionType::ARM_None) {
bool CallerHasZAState = false;
bool CallerHasZT0State = false;
if (const auto *CallerFD = dyn_cast<FunctionDecl>(CurContext)) {
if (CallerFD) {
auto *Attr = CallerFD->getAttr<ArmNewAttr>();
if (Attr && Attr->isNewZA())
CallerHasZAState = true;
Expand Down
12 changes: 11 additions & 1 deletion clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12395,12 +12395,22 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
}

// Check if the function definition uses any AArch64 SME features without
// having the '+sme' feature enabled.
// having the '+sme' feature enabled and warn user if sme locally streaming
// function returns or uses arguments with VL-based types.
if (DeclIsDefn) {
const auto *Attr = NewFD->getAttr<ArmNewAttr>();
bool UsesSM = NewFD->hasAttr<ArmLocallyStreamingAttr>();
bool UsesZA = Attr && Attr->isNewZA();
bool UsesZT0 = Attr && Attr->isNewZT0();

if (NewFD->hasAttr<ArmLocallyStreamingAttr>()) {
if (NewFD->getReturnType()->isSizelessVectorType() ||
llvm::any_of(NewFD->parameters(), [](ParmVarDecl *P) {
return P->getOriginalType()->isSizelessVectorType();
}))
Diag(NewFD->getLocation(),
diag::warn_sme_locally_streaming_has_vl_args_returns);
}
if (const auto *FPT = NewFD->getType()->getAs<FunctionProtoType>()) {
FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
UsesSM |=
Expand Down
6 changes: 5 additions & 1 deletion clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
// RUN: -target-feature +sme2 -target-feature +sve2 -target-feature +neon -fsyntax-only -verify %s
// RUN: -target-feature +sme2 -target-feature +sve2 -target-feature +neon -Waarch64-sme-attributes -fsyntax-only -verify %s

// REQUIRES: aarch64-registered-target

Expand Down Expand Up @@ -33,6 +33,7 @@ svuint32_t incompat_sve_sm(svbool_t pg, svuint32_t a, int16_t b) __arm_streaming
return __builtin_sve_svld1_gather_u32base_index_u32(pg, a, b);
}

// expected-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
__arm_locally_streaming svuint32_t incompat_sve_ls(svbool_t pg, svuint32_t a, int64_t b) {
// expected-warning@+1 {{builtin call has undefined behaviour when called from a streaming function}}
return __builtin_sve_svld1_gather_u32base_index_u32(pg, a, b);
Expand All @@ -48,6 +49,7 @@ svuint32_t incompat_sve2_sm(svbool_t pg, svuint32_t a, int64_t b) __arm_streamin
return __builtin_sve_svldnt1_gather_u32base_index_u32(pg, a, b);
}

// expected-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
__arm_locally_streaming svuint32_t incompat_sve2_ls(svbool_t pg, svuint32_t a, int64_t b) {
// expected-warning@+1 {{builtin call has undefined behaviour when called from a streaming function}}
return __builtin_sve_svldnt1_gather_u32base_index_u32(pg, a, b);
Expand All @@ -68,6 +70,7 @@ svfloat64_t streaming_caller_sve(svbool_t pg, svfloat64_t a, float64_t b) __arm_
return svadd_n_f64_m(pg, a, b);
}

// expected-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
__arm_locally_streaming svfloat64_t locally_streaming_caller_sve(svbool_t pg, svfloat64_t a, float64_t b) {
// expected-no-warning
return svadd_n_f64_m(pg, a, b);
Expand All @@ -83,6 +86,7 @@ svint16_t streaming_caller_sve2(svint16_t op1, svint16_t op2) __arm_streaming {
return svmul_lane_s16(op1, op2, 0);
}

// expected-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
__arm_locally_streaming svint16_t locally_streaming_caller_sve2(svint16_t op1, svint16_t op2) {
// expected-no-warning
return svmul_lane_s16(op1, op2, 0);
Expand Down
136 changes: 134 additions & 2 deletions clang/test/Sema/aarch64-sme-func-attrs.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme2 -fsyntax-only -verify %s
// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme2 -fsyntax-only -verify=expected-cpp -x c++ %s
// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme2 -target-feature +sve -Waarch64-sme-attributes -fsyntax-only -verify %s
// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme2 -target-feature +sve -Waarch64-sme-attributes -fsyntax-only -verify=expected-cpp -x c++ %s

// Valid attributes

Expand Down Expand Up @@ -496,3 +496,135 @@ void fmv_caller() {
just_fine();
incompatible_locally_streaming();
}

void sme_streaming_with_vl_arg(__SVInt8_t a) __arm_streaming { }

__SVInt8_t sme_streaming_returns_vl(void) __arm_streaming { __SVInt8_t r; return r; }

void sme_streaming_compatible_with_vl_arg(__SVInt8_t a) __arm_streaming_compatible { }

__SVInt8_t sme_streaming_compatible_returns_vl(void) __arm_streaming_compatible { __SVInt8_t r; return r; }

void sme_no_streaming_with_vl_arg(__SVInt8_t a) { }

__SVInt8_t sme_no_streaming_returns_vl(void) { __SVInt8_t r; return r; }

// expected-warning@+2 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
__arm_locally_streaming void sme_locally_streaming_with_vl_arg(__SVInt8_t a) { }

// expected-warning@+2 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
__arm_locally_streaming __SVInt8_t sme_locally_streaming_returns_vl(void) { __SVInt8_t r; return r; }

void sme_no_streaming_calling_streaming_with_vl_args() {
__SVInt8_t a;
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
sme_streaming_with_vl_arg(a);
}

void sme_no_streaming_calling_streaming_with_return_vl() {
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
__SVInt8_t r = sme_streaming_returns_vl();
}

void sme_streaming_calling_non_streaming_with_vl_args(void) __arm_streaming {
__SVInt8_t a;
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
sme_no_streaming_with_vl_arg(a);
}

void sme_streaming_calling_non_streaming_with_return_vl(void) __arm_streaming {
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
__SVInt8_t r = sme_no_streaming_returns_vl();
}

void sme_no_streaming_calling_streaming_with_vl_args_param(__SVInt8_t arg, void (*sc)( __SVInt8_t arg) __arm_streaming) {
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
sc(arg);
}

__SVInt8_t sme_no_streaming_calling_streaming_return_vl_param(__SVInt8_t (*s)(void) __arm_streaming) {
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
return s();
}

void sme_streaming_compatible_calling_streaming_with_vl_args(__SVInt8_t arg) __arm_streaming_compatible {
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
sme_streaming_with_vl_arg(arg);
}

void sme_streaming_compatible_calling_sme_streaming_return_vl(void) __arm_streaming_compatible {
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
__SVInt8_t r = sme_streaming_returns_vl();
}

void sme_streaming_compatible_calling_no_streaming_with_vl_args(__SVInt8_t arg) __arm_streaming_compatible {
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
sme_no_streaming_with_vl_arg(arg);
}

void sme_streaming_compatible_calling_no_sme_streaming_return_vl(void) __arm_streaming_compatible {
// expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
// expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
__SVInt8_t r = sme_no_streaming_returns_vl();
}

void sme_streaming_calling_streaming(__SVInt8_t arg, void (*s)( __SVInt8_t arg) __arm_streaming) __arm_streaming {
s(arg);
}

__SVInt8_t sme_streaming_calling_streaming_return_vl(__SVInt8_t (*s)(void) __arm_streaming) __arm_streaming {
return s();
}

void sme_streaming_calling_streaming_with_vl_args(__SVInt8_t a) __arm_streaming {
sme_streaming_with_vl_arg(a);
}

void sme_streaming_calling_streaming_with_return_vl(void) __arm_streaming {
__SVInt8_t r = sme_streaming_returns_vl();
}

void sme_streaming_calling_streaming_compatible_with_vl_args(__SVInt8_t a) __arm_streaming {
sme_streaming_compatible_with_vl_arg(a);
}

void sme_streaming_calling_streaming_compatible_with_return_vl(void) __arm_streaming {
__SVInt8_t r = sme_streaming_compatible_returns_vl();
}

void sme_no_streaming_calling_streaming_compatible_with_vl_args() {
__SVInt8_t a;
sme_streaming_compatible_with_vl_arg(a);
}

void sme_no_streaming_calling_streaming_compatible_with_return_vl() {
__SVInt8_t r = sme_streaming_compatible_returns_vl();
}

void sme_no_streaming_calling_non_streaming_compatible_with_vl_args() {
__SVInt8_t a;
sme_no_streaming_with_vl_arg(a);
}

void sme_no_streaming_calling_non_streaming_compatible_with_return_vl() {
__SVInt8_t r = sme_no_streaming_returns_vl();
}

void sme_streaming_compatible_calling_streaming_compatible_with_vl_args(__SVInt8_t arg) __arm_streaming_compatible {
sme_streaming_compatible_with_vl_arg(arg);
}

void sme_streaming_compatible_calling_streaming_compatible_with_return_vl(void) __arm_streaming_compatible {
__SVInt8_t r = sme_streaming_compatible_returns_vl();
}