https://github.com/MacDue updated https://github.com/llvm/llvm-project/pull/174608
>From 2538bd01f629e252573e5c470eb83dfcb140570a Mon Sep 17 00:00:00 2001 From: Benjamin Maxwell <[email protected]> Date: Tue, 13 Jan 2026 14:48: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 set to `ASTContext` to track if a function contains an expression that is not safe in streaming mode. A function is inserted into this set when a non-streaming builtin is found, or an always_inline call to a function in the set is found. 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 `-Waarch64-sme-attributes`, as it is unlikely to be an issue if the error was not emitted. --- clang/include/clang/AST/ASTContext.h | 5 ++ .../clang/Basic/DiagnosticFrontendKinds.td | 4 +- clang/include/clang/Sema/SemaARM.h | 4 ++ clang/lib/CodeGen/CGCall.cpp | 6 +- clang/lib/CodeGen/TargetInfo.h | 12 ++-- clang/lib/CodeGen/Targets/AArch64.cpp | 66 +++++++++++++++---- clang/lib/CodeGen/Targets/X86.cpp | 5 +- clang/lib/Sema/SemaARM.cpp | 6 +- .../CodeGen/AArch64/sme-always-inline.cpp | 56 ++++++++++++++++ .../AArch64/sme-inline-streaming-attrs.c | 8 +-- 10 files changed, 141 insertions(+), 31 deletions(-) create mode 100644 clang/test/CodeGen/AArch64/sme-always-inline.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 68205dd1c1fd9..421b3715dc9a4 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -1370,6 +1370,11 @@ class ASTContext : public RefCountedBase<ASTContext> { /// are stored here. llvm::DenseMap<const CXXMethodDecl *, CXXCastPath> LambdaCastPaths; + /// Keep track of functions that contain expressions that are not valid in + /// streaming mode on AArch64. This is used to check inlining validity. + llvm::DenseSet<const FunctionDecl *> + AArch64ContansExprNotSafeForStreamingFunctions; + ASTContext(LangOptions &LOpts, SourceManager &SM, IdentifierTable &idents, SelectorTable &sels, Builtin::Context &builtins, TranslationUnitKind TUKind); diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index e2b257ceae80d..880a50461b8ab 100644 --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -296,7 +296,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/Sema/SemaARM.h b/clang/include/clang/Sema/SemaARM.h index af8e0e9047171..a98a5036b592e 100644 --- a/clang/include/clang/Sema/SemaARM.h +++ b/clang/include/clang/Sema/SemaARM.h @@ -83,6 +83,10 @@ class SemaARM : public SemaBase { void CheckSMEFunctionDefAttributes(const FunctionDecl *FD); + void setFunctionContainsExprNotSafeForStreamingMode(const FunctionDecl *FD) { + getASTContext().AArch64ContansExprNotSafeForStreamingFunctions.insert(FD); + } + /// Return true if the given types are an SVE builtin and a VectorType that /// is a fixed-length representation of the SVE builtin for a specific /// vector-length. diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index d7bdeb3981cf8..06e5f0bd8261c 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -5284,7 +5284,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, // attribute-target/features. Give them a chance to diagnose. const FunctionDecl *CallerDecl = dyn_cast_or_null<FunctionDecl>(CurCodeDecl); const FunctionDecl *CalleeDecl = dyn_cast_or_null<FunctionDecl>(TargetDecl); - CGM.getTargetCodeGenInfo().checkFunctionCallABI(CGM, Loc, CallerDecl, + CGM.getTargetCodeGenInfo().checkFunctionCallABI(*this, Loc, CallerDecl, CalleeDecl, CallArgs, RetTy); // 1. Set up the arguments. @@ -5860,7 +5860,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, // Note: This corresponds to the [[clang::always_inline]] statement attribute. if (InAlwaysInlineAttributedStmt && !CGM.getTargetCodeGenInfo().wouldInliningViolateFunctionCallABI( - CallerDecl, CalleeDecl)) + *this, CallerDecl, CalleeDecl)) Attrs = Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline); @@ -5878,7 +5878,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, !InNoInlineAttributedStmt && !(TargetDecl && TargetDecl->hasAttr<NoInlineAttr>()) && !CGM.getTargetCodeGenInfo().wouldInliningViolateFunctionCallABI( - CallerDecl, CalleeDecl)) { + *this, CallerDecl, CalleeDecl)) { Attrs = Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline); } diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h index db06584d766bf..496eaeb0439d6 100644 --- a/clang/lib/CodeGen/TargetInfo.h +++ b/clang/lib/CodeGen/TargetInfo.h @@ -98,11 +98,10 @@ class TargetCodeGenInfo { /// 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, - QualType ReturnType) const {} + virtual void + checkFunctionCallABI(CodeGenFunction &CGF, SourceLocation CallLoc, + const FunctionDecl *Caller, const FunctionDecl *Callee, + const CallArgList &Args, QualType ReturnType) const {} /// Returns true if inlining the function call would produce incorrect code /// for the current target and should be ignored (even with the always_inline @@ -117,7 +116,8 @@ class TargetCodeGenInfo { /// See previous discussion here: /// https://discourse.llvm.org/t/rfc-avoid-inlining-alwaysinline-functions-when-they-cannot-be-inlined/79528 virtual bool - wouldInliningViolateFunctionCallABI(const FunctionDecl *Caller, + wouldInliningViolateFunctionCallABI(CodeGenFunction &CGF, + const FunctionDecl *Caller, const FunctionDecl *Callee) const { return false; } diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 963b74927036a..5522bdaa0694e 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -182,18 +182,20 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo { void checkFunctionABI(CodeGenModule &CGM, const FunctionDecl *Decl) const override; - void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc, + void checkFunctionCallABI(CodeGenFunction &CGF, SourceLocation CallLoc, const FunctionDecl *Caller, const FunctionDecl *Callee, const CallArgList &Args, QualType ReturnType) const override; bool wouldInliningViolateFunctionCallABI( - const FunctionDecl *Caller, const FunctionDecl *Callee) const override; + CodeGenFunction &CGF, const FunctionDecl *Caller, + const FunctionDecl *Callee) const override; private: // Diagnose calls between functions with incompatible Streaming SVE // attributes. - void checkFunctionCallABIStreaming(CodeGenModule &CGM, SourceLocation CallLoc, + void checkFunctionCallABIStreaming(CodeGenFunction &CGF, + SourceLocation CallLoc, const FunctionDecl *Caller, const FunctionDecl *Callee) const; // Diagnose calls which must pass arguments in floating-point registers when @@ -1231,16 +1233,31 @@ 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), }; +static bool +functionContainsExprNotSafeForStreamingMode(CodeGenModule &CGM, + const FunctionDecl *FD) { + return CGM.getContext() + .AArch64ContansExprNotSafeForStreamingFunctions.contains(FD); +} + +static void +setFunctionContainsExprNotSafeForStreamingMode(CodeGenModule &CGM, + const FunctionDecl *FD) { + CGM.getContext().AArch64ContansExprNotSafeForStreamingFunctions.insert(FD); +} + /// Determines if there are any Arm SME ABI issues with inlining \p Callee into /// \p Caller. Returns the issue (if any) in the ArmSMEInlinability bit enum. -static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller, +static ArmSMEInlinability GetArmSMEInlinability(CodeGenModule &CGM, + const FunctionDecl *Caller, const FunctionDecl *Callee) { bool CallerIsStreaming = IsArmStreamingFunction(Caller, /*IncludeLocallyStreaming=*/true); @@ -1248,9 +1265,15 @@ static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller, IsArmStreamingFunction(Callee, /*IncludeLocallyStreaming=*/true); bool CallerIsStreamingCompatible = isStreamingCompatible(Caller); bool CalleeIsStreamingCompatible = isStreamingCompatible(Callee); + bool CalleeContainsNonStreamingBuiltinCall = + functionContainsExprNotSafeForStreamingMode(CGM, Callee); ArmSMEInlinability Inlinability = ArmSMEInlinability::Ok; + if ((CallerIsStreamingCompatible || CallerIsStreaming) && + CalleeContainsNonStreamingBuiltinCall) + Inlinability |= ArmSMEInlinability::ErrorContainsNonStreamingBuiltin; + if (!CalleeIsStreamingCompatible && (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible)) { if (CalleeIsStreaming) @@ -1258,6 +1281,7 @@ static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller, else Inlinability |= ArmSMEInlinability::WarnIncompatibleStreamingModes; } + if (auto *NewAttr = Callee->getAttr<ArmNewAttr>()) { if (NewAttr->isNewZA()) Inlinability |= ArmSMEInlinability::ErrorCalleeRequiresNewZA; @@ -1269,12 +1293,20 @@ static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller, } void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming( - CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller, + CodeGenFunction &CGF, SourceLocation CallLoc, const FunctionDecl *Caller, const FunctionDecl *Callee) const { - if (!Caller || !Callee || !Callee->hasAttr<AlwaysInlineAttr>()) + if (!Caller || !Callee || CGF.InNoInlineAttributedStmt || + !Callee->hasAttr<AlwaysInlineAttr>()) return; - ArmSMEInlinability Inlinability = GetArmSMEInlinability(Caller, Callee); + CodeGenModule &CGM = CGF.CGM; + ArmSMEInlinability Inlinability = GetArmSMEInlinability(CGM, 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) @@ -1318,20 +1350,28 @@ void AArch64TargetCodeGenInfo::checkFunctionCallABISoftFloat( Callee ? Callee : Caller, CallLoc); } -void AArch64TargetCodeGenInfo::checkFunctionCallABI(CodeGenModule &CGM, +void AArch64TargetCodeGenInfo::checkFunctionCallABI(CodeGenFunction &CGF, SourceLocation CallLoc, const FunctionDecl *Caller, const FunctionDecl *Callee, const CallArgList &Args, QualType ReturnType) const { - checkFunctionCallABIStreaming(CGM, CallLoc, Caller, Callee); - checkFunctionCallABISoftFloat(CGM, CallLoc, Caller, Callee, Args, ReturnType); + if (!CGF.InNoInlineAttributedStmt && Caller && Callee && + Callee->hasAttr<AlwaysInlineAttr>() && + functionContainsExprNotSafeForStreamingMode(CGF.CGM, Callee)) + setFunctionContainsExprNotSafeForStreamingMode(CGF.CGM, Caller); + + checkFunctionCallABIStreaming(CGF, CallLoc, Caller, Callee); + checkFunctionCallABISoftFloat(CGF.CGM, CallLoc, Caller, Callee, Args, + ReturnType); } bool AArch64TargetCodeGenInfo::wouldInliningViolateFunctionCallABI( - const FunctionDecl *Caller, const FunctionDecl *Callee) const { + CodeGenFunction &CGF, const FunctionDecl *Caller, + const FunctionDecl *Callee) const { return Caller && Callee && - GetArmSMEInlinability(Caller, Callee) != ArmSMEInlinability::Ok; + GetArmSMEInlinability(CGF.CGM, Caller, Callee) != + ArmSMEInlinability::Ok; } void AArch64ABIInfo::appendAttributeMangling(TargetClonesAttr *Attr, diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index e6203db8bc245..a36523a7c4a1a 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -1504,7 +1504,7 @@ class X86_64TargetCodeGenInfo : public TargetCodeGenInfo { } } - void checkFunctionCallABI(CodeGenModule &CGM, SourceLocation CallLoc, + void checkFunctionCallABI(CodeGenFunction &CGF, SourceLocation CallLoc, const FunctionDecl *Caller, const FunctionDecl *Callee, const CallArgList &Args, QualType ReturnType) const override; @@ -1570,7 +1570,7 @@ static bool checkAVXParam(DiagnosticsEngine &Diag, ASTContext &Ctx, return false; } -void X86_64TargetCodeGenInfo::checkFunctionCallABI(CodeGenModule &CGM, +void X86_64TargetCodeGenInfo::checkFunctionCallABI(CodeGenFunction &CGF, SourceLocation CallLoc, const FunctionDecl *Caller, const FunctionDecl *Callee, @@ -1579,6 +1579,7 @@ void X86_64TargetCodeGenInfo::checkFunctionCallABI(CodeGenModule &CGM, if (!Callee) return; + CodeGenModule &CGM = CGF.CGM; llvm::StringMap<bool> CallerMap; llvm::StringMap<bool> CalleeMap; unsigned ArgIndex = 0; diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp index 53e8c002a1962..330e78593e3a9 100644 --- a/clang/lib/Sema/SemaARM.cpp +++ b/clang/lib/Sema/SemaARM.cpp @@ -623,9 +623,11 @@ 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) + S.ARM().setFunctionContainsExprNotSafeForStreamingMode(FD); return false; - + } return true; } 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..f678d9d77edfe --- /dev/null +++ b/clang/test/CodeGen/AArch64/sme-always-inline.cpp @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -Waarch64-sme-attributes -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +neon -verify=expected-attr %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 -Waarch64-sme-attributes -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +neon -verify=expected-attr %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(); +} + +// expected-note@+2 {{conflicting attribute is here}} +// expected-attr-note@+1 {{conflicting attribute is here}} +__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-attr-warning@+2 {{always_inline function 'bar' and its caller 'streaming_error' have mismatching streaming attributes, inlining may change runtime behaviour}} + // expected-attr-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-attr-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) + + /// `noinline` has higher precedence than always_inline (so this is not an error) + // expected-warning@+2 {{statement attribute 'clang::noinline' has higher precedence than function attribute 'always_inline'}} + // expected-attr-warning@+1 {{statement attribute 'clang::noinline' has higher precedence than function attribute 'always_inline'}} + [[clang::noinline]] bar(); +} + +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..374e29ab13036 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 -Waarch64-sme-attributes -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_NONE %s +// RUN: %clang_cc1 -Waarch64-sme-attributes -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_COMPATIBLE %s +// RUN: %clang_cc1 -Waarch64-sme-attributes -triple aarch64-none-linux-gnu -S -o /dev/null -target-feature +sme -target-feature +sme2 -verify -DTEST_STREAMING %s +// RUN: %clang_cc1 -Waarch64-sme-attributes -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
