https://github.com/MacDue created https://github.com/llvm/llvm-project/pull/174608
Previously, we would emit a warning when if we encountered a `always_inline` function with incompatible streaming attributes within a `streaming[_compatible]` function. This has two major issues. 1. It's prone to false-positives/non-issue warnings * Existing library code may be marked `always_inline` without the appropriate `__arm_streaming_compatible` attribute, but the body of the function could still be safe to inline * In these cases, the warning can be an annoyance as the issue may not be in the user's code 2. It does not catch transitive errors with non-streaming safe builtins * If a non-streaming safe builtin is wrapped in an `always_inline` function, calling that function from a streaming function is simply a warning in the frontend, but can result in a backend crash This patch improves this by adding a bit to `FunctionDeclBits` to track if a function contains a non-streaming safe expression. This bit is set when a non-streaming builtin is found, and is propagated through `always_inline` calls. With this, we can emit an error if a non-streaming safe builtin occurs within a function directly or transitively via `always_inline` callees. This allows us to downgrade the existing warning to only occur with `-Wall`, as it is unlikely to be an issue if the error was not emitted. >From f0815a818c70b205c06c9d2811e0d3c2f661ff1a Mon Sep 17 00:00:00 2001 From: Benjamin Maxwell <[email protected]> Date: Mon, 5 Jan 2026 16:49:28 +0000 Subject: [PATCH] [clang][SME] Rework streaming mode always_inline errors/warnings Previously, we would emit a warning when if we encountered a `always_inline` function with incompatible streaming attributes within a `streaming[_compatible]` function. This has two major issues. 1. It's prone to false-positives/non-issue warnings * Existing library code may be marked `always_inline` without the appropriate `__arm_streaming_compatible` attribute, but the body of the function could still be safe to inline * In these cases, the warning can be an annoyance as the issue may not be in the user's code 2. It does not catch transitive errors with non-streaming safe builtins * If a non-streaming safe builtin is wrapped in an `always_inline` function, calling that function from a streaming function is simply a warning in the frontend, but can result in a backend crash This patch improves this by adding a bit to `FunctionDeclBits` to track if a function contains a non-streaming safe expression. This bit is set when a non-streaming builtin is found, and is propagated through `always_inline` calls. With this, we can emit an error if a non-streaming safe builtin occurs within a function directly or transitively via `always_inline` callees. This allows us to downgrade the existing warning to only occur with `-Wall`, as it is unlikely to be an issue if the error was not emitted. --- clang/include/clang/AST/Decl.h | 7 +++ clang/include/clang/AST/DeclBase.h | 5 ++ .../clang/Basic/DiagnosticFrontendKinds.td | 4 +- clang/include/clang/Basic/DiagnosticGroups.td | 8 +-- clang/lib/CodeGen/Targets/AArch64.cpp | 16 +++++- clang/lib/Sema/SemaARM.cpp | 8 ++- clang/lib/Sema/SemaExpr.cpp | 7 +++ .../CodeGen/AArch64/sme-always-inline.cpp | 49 +++++++++++++++++++ .../AArch64/sme-inline-streaming-attrs.c | 8 +-- 9 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 clang/test/CodeGen/AArch64/sme-always-inline.cpp diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 2e8ceff453547..1505162a5ec79 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2370,6 +2370,13 @@ class FunctionDecl : public DeclaratorDecl, FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val; } + bool bodyContainsNonStreamingSafeExpression() const { + return FunctionDeclBits.BodyContainsNonStreamingSafeExpression; + } + void setBodyContainsNonStreamingSafeExpression(bool Val = true) { + FunctionDeclBits.BodyContainsNonStreamingSafeExpression = Val; + } + /// Whether this function is "trivial" in some specialized C++ senses. /// Can only be true for default constructors, copy constructors, /// copy assignment operators, and destructors. Not meaningful until diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 5519787d71f88..5364a83c69adb 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -1829,6 +1829,11 @@ class DeclContext { // refers to an enclosing template for hte purposes of [temp.friend]p9. LLVM_PREFERRED_TYPE(bool) uint64_t FriendConstraintRefersToEnclosingTemplate : 1; + + // Indicates this function contains a non-(arm)-streaming safe expression. + // This can be directly, or transitively via always_inline calls. + LLVM_PREFERRED_TYPE(bool) + uint64_t BodyContainsNonStreamingSafeExpression : 1; }; /// Number of inherited and non-inherited bits in FunctionDeclBitfields. diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index 66c7f92622d1e..d9a10ceba61d6 100644 --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -299,7 +299,9 @@ def err_function_always_inline_attribute_mismatch : Error< "always_inline function %1 and its caller %0 have mismatching %2 attributes">; def warn_function_always_inline_attribute_mismatch : Warning< "always_inline function %1 and its caller %0 have mismatching %2 attributes, " - "inlining may change runtime behaviour">, InGroup<AArch64SMEAttributes>; + "inlining may change runtime behaviour">, InGroup<AArch64SMEAttributes>, DefaultIgnore; +def err_function_always_inline_non_streaming_builtins : Error< + "always_inline function %0 cannot be inlined into streaming caller as it contains calls to non-streaming builtins">; def err_function_always_inline_new_za : Error< "always_inline function %0 has new za state">; def err_function_always_inline_new_zt0 diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 3764475fbd3df..f9345041e2977 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1327,12 +1327,15 @@ def PerfConstraintImpliesNoexcept : DiagGroup<"perf-constraint-implies-noexcept" // Uniqueness Analysis warnings def Consumed : DiagGroup<"consumed">; +// A warning group related to AArch64 SME function attribues. +def AArch64SMEAttributes : DiagGroup<"aarch64-sme-attributes">; + // Note that putting warnings in -Wall will not disable them by default. If a // warning should be active _only_ when -Wall is passed in, mark it as // DefaultIgnore in addition to putting it here. def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, MisleadingIndentation, PackedNonPod, - VLACxxExtension]>; + VLACxxExtension, AArch64SMEAttributes]>; // Warnings that should be in clang-cl /w4. def : DiagGroup<"CL4", [All, Extra]>; @@ -1645,9 +1648,6 @@ 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">; diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 963b74927036a..3e86377f74209 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -1231,11 +1231,12 @@ enum class ArmSMEInlinability : uint8_t { ErrorCalleeRequiresNewZT0 = 1 << 1, WarnIncompatibleStreamingModes = 1 << 2, ErrorIncompatibleStreamingModes = 1 << 3, + ErrorContainsNonStreamingBuiltin = 1 << 4, IncompatibleStreamingModes = WarnIncompatibleStreamingModes | ErrorIncompatibleStreamingModes, - LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/ErrorIncompatibleStreamingModes), + LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/ErrorContainsNonStreamingBuiltin), }; /// Determines if there are any Arm SME ABI issues with inlining \p Callee into @@ -1248,9 +1249,15 @@ static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller, IsArmStreamingFunction(Callee, /*IncludeLocallyStreaming=*/true); bool CallerIsStreamingCompatible = isStreamingCompatible(Caller); bool CalleeIsStreamingCompatible = isStreamingCompatible(Callee); + bool CalleeContainsNonStreamingBuiltinCall = + Callee->bodyContainsNonStreamingSafeExpression(); ArmSMEInlinability Inlinability = ArmSMEInlinability::Ok; + if ((CallerIsStreamingCompatible || CallerIsStreaming) && + CalleeContainsNonStreamingBuiltinCall) + Inlinability |= ArmSMEInlinability::ErrorContainsNonStreamingBuiltin; + if (!CalleeIsStreamingCompatible && (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible)) { if (CalleeIsStreaming) @@ -1258,6 +1265,7 @@ static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller, else Inlinability |= ArmSMEInlinability::WarnIncompatibleStreamingModes; } + if (auto *NewAttr = Callee->getAttr<ArmNewAttr>()) { if (NewAttr->isNewZA()) Inlinability |= ArmSMEInlinability::ErrorCalleeRequiresNewZA; @@ -1276,6 +1284,12 @@ void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming( ArmSMEInlinability Inlinability = GetArmSMEInlinability(Caller, Callee); + if ((Inlinability & ArmSMEInlinability::ErrorContainsNonStreamingBuiltin) != + ArmSMEInlinability::Ok) + CGM.getDiags().Report( + CallLoc, diag::err_function_always_inline_non_streaming_builtins) + << Callee->getDeclName(); + if ((Inlinability & ArmSMEInlinability::IncompatibleStreamingModes) != ArmSMEInlinability::Ok) CGM.getDiags().Report( diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp index 81504b74c5e45..7e1e4c45eac72 100644 --- a/clang/lib/Sema/SemaARM.cpp +++ b/clang/lib/Sema/SemaARM.cpp @@ -623,9 +623,13 @@ static bool checkArmStreamingBuiltin(Sema &S, CallExpr *TheCall, BuiltinType == SemaARM::ArmStreaming) S.Diag(TheCall->getBeginLoc(), diag::err_attribute_arm_sm_incompat_builtin) << TheCall->getSourceRange() << "streaming"; - else + else { + if (BuiltinType == SemaARM::ArmNonStreaming) { + const_cast<FunctionDecl *>(FD) + ->setBodyContainsNonStreamingSafeExpression(); + } return false; - + } return true; } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index e12e4b204afad..6f68ededa0669 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -6916,6 +6916,13 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl, FunctionDecl *FDecl = dyn_cast_or_null<FunctionDecl>(NDecl); unsigned BuiltinID = (FDecl ? FDecl->getBuiltinID() : 0); + if (auto *Caller = getCurFunctionDecl()) { + if (FDecl && FDecl->hasAttr<AlwaysInlineAttr>() && + FDecl->bodyContainsNonStreamingSafeExpression()) { + Caller->setBodyContainsNonStreamingSafeExpression(); + } + } + auto IsSJLJ = [&] { switch (BuiltinID) { case Builtin::BI__builtin_longjmp: diff --git a/clang/test/CodeGen/AArch64/sme-always-inline.cpp b/clang/test/CodeGen/AArch64/sme-always-inline.cpp new file mode 100644 index 0000000000000..e45074296f9af --- /dev/null +++ b/clang/test/CodeGen/AArch64/sme-always-inline.cpp @@ -0,0 +1,49 @@ +// RUN: %clang_cc1 -Wall -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +neon -verify=expected-Wall %s -DTEST_STREAMING +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +neon -verify %s -DTEST_STREAMING +// RUN: %clang_cc1 -Wall -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +neon -verify=expected-Wall %s -DTEST_COMPATIBLE +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +neon -verify %s -DTEST_COMPATIBLE + +#if defined(TEST_STREAMING) +#define SM_ATTR __arm_streaming +#elif defined(TEST_COMPATIBLE) +#define SM_ATTR __arm_streaming_compatible +#else +#error "Expected TEST_STREAMING or TEST_COMPATIBLE" +#endif + +__attribute__((always_inline)) void incompatible_neon() { + __attribute((vector_size(16))) char vec = { 0 }; + vec = __builtin_neon_vqaddq_v(vec, vec, 33); +} + +__attribute__((always_inline)) void compatible_missing_attrs() { + // <Empty> +} + +void foo() { + incompatible_neon(); +} + +__attribute__((always_inline)) void bar() { + incompatible_neon(); +} + +__attribute__((always_inline)) void baz() { + compatible_missing_attrs(); +} + +void streaming_error() SM_ATTR { + // expected-error@+3 {{always_inline function 'bar' cannot be inlined into streaming caller as it contains calls to non-streaming builtins}} + // expected-Wall-warning@+2 {{always_inline function 'bar' and its caller 'streaming_error' have mismatching streaming attributes, inlining may change runtime behaviour}} + // expected-Wall-error@+1 {{always_inline function 'bar' cannot be inlined into streaming caller as it contains calls to non-streaming builtins}} + bar(); // -> incompatible_neon -> __builtin_neon_vqaddq_v (error) +} + +void streaming_warning() SM_ATTR { + // expected-Wall-warning@+1 {{always_inline function 'baz' and its caller 'streaming_warning' have mismatching streaming attributes, inlining may change runtime behaviour}} + baz(); // -> compatible_missing_attrs (no error) +} + +void streaming_no_warning() SM_ATTR { + foo(); // `foo` is not always_inline (no error/warning) +} diff --git a/clang/test/CodeGen/AArch64/sme-inline-streaming-attrs.c b/clang/test/CodeGen/AArch64/sme-inline-streaming-attrs.c index 68102c9ded40c..cb7bbcb9e2308 100644 --- a/clang/test/CodeGen/AArch64/sme-inline-streaming-attrs.c +++ b/clang/test/CodeGen/AArch64/sme-inline-streaming-attrs.c @@ -1,7 +1,7 @@ -// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_NONE %s -// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_COMPATIBLE %s -// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_STREAMING %s -// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_LOCALLY %s +// RUN: %clang_cc1 -Wall -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_NONE %s +// RUN: %clang_cc1 -Wall -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_COMPATIBLE %s +// RUN: %clang_cc1 -Wall -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_STREAMING %s +// RUN: %clang_cc1 -Wall -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_LOCALLY %s // REQUIRES: aarch64-registered-target _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
