[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
@@ -814,6 +821,93 @@ Address AArch64ABIInfo::EmitMSVAArg(CodeGenFunction ,
Address VAListAddr,
/*allowHigherAlign*/ false);
}
+class SMEAttributes {
+public:
+ bool IsStreaming = false;
+ bool IsStreamingBody = false;
+ bool IsStreamingCompatible = false;
+ bool HasNewZA = false;
+
+ SMEAttributes(const FunctionDecl *F) {
+if (F->hasAttr())
+ IsStreamingBody = true;
+if (auto *NewAttr = F->getAttr()) {
+ if (NewAttr->isNewZA())
+HasNewZA = true;
+}
+if (const auto *T = F->getType()->getAs()) {
+ if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMEnabledMask)
+IsStreaming = true;
+ if (T->getAArch64SMEAttributes() &
+ FunctionType::SME_PStateSMCompatibleMask)
+IsStreamingCompatible = true;
+}
+ }
+
+ bool hasStreamingBody() const { return IsStreamingBody; }
+ bool hasStreamingInterface() const { return IsStreaming; }
+ bool hasStreamingCompatibleInterface() const { return IsStreamingCompatible;
}
+ bool hasStreamingInterfaceOrBody() const {
+return hasStreamingBody() || hasStreamingInterface();
+ }
+ bool hasNonStreamingInterface() const {
+return !hasStreamingInterface() && !hasStreamingCompatibleInterface();
+ }
+ bool hasNonStreamingInterfaceAndBody() const {
+return hasNonStreamingInterface() && !hasStreamingBody();
+ }
+
+ bool requiresSMChange(const SMEAttributes Callee,
+bool BodyOverridesInterface = false) {
+// If the transition is not through a call (e.g. when considering inlining)
+// and Callee has a streaming body, then we can ignore the interface of
+// Callee.
+if (BodyOverridesInterface && Callee.hasStreamingBody()) {
+ return !hasStreamingInterfaceOrBody();
+}
+
+if (Callee.hasStreamingCompatibleInterface())
+ return false;
+
+if (hasStreamingCompatibleInterface())
+ return true;
+
+// Both non-streaming
+if (hasNonStreamingInterfaceAndBody() && Callee.hasNonStreamingInterface())
+ return false;
+
+// Both streaming
+if (hasStreamingInterfaceOrBody() && Callee.hasStreamingInterface())
+ return false;
+
+return Callee.hasStreamingInterface();
+ }
+
+ bool hasNewZABody() { return HasNewZA; }
+ bool requiresLazySave() const { return HasNewZA; }
SamTebbs33 wrote:
Thanks, I've now removed the lazy save checking to loosen that restriction.
https://github.com/llvm/llvm-project/pull/77936
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
https://github.com/sdesmalen-arm edited https://github.com/llvm/llvm-project/pull/77936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
@@ -814,6 +821,93 @@ Address AArch64ABIInfo::EmitMSVAArg(CodeGenFunction ,
Address VAListAddr,
/*allowHigherAlign*/ false);
}
+class SMEAttributes {
+public:
+ bool IsStreaming = false;
+ bool IsStreamingBody = false;
+ bool IsStreamingCompatible = false;
+ bool HasNewZA = false;
+
+ SMEAttributes(const FunctionDecl *F) {
+if (F->hasAttr())
+ IsStreamingBody = true;
+if (auto *NewAttr = F->getAttr()) {
+ if (NewAttr->isNewZA())
+HasNewZA = true;
+}
+if (const auto *T = F->getType()->getAs()) {
+ if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMEnabledMask)
+IsStreaming = true;
+ if (T->getAArch64SMEAttributes() &
+ FunctionType::SME_PStateSMCompatibleMask)
+IsStreamingCompatible = true;
+}
+ }
+
+ bool hasStreamingBody() const { return IsStreamingBody; }
+ bool hasStreamingInterface() const { return IsStreaming; }
+ bool hasStreamingCompatibleInterface() const { return IsStreamingCompatible;
}
+ bool hasStreamingInterfaceOrBody() const {
+return hasStreamingBody() || hasStreamingInterface();
+ }
+ bool hasNonStreamingInterface() const {
+return !hasStreamingInterface() && !hasStreamingCompatibleInterface();
+ }
+ bool hasNonStreamingInterfaceAndBody() const {
+return hasNonStreamingInterface() && !hasStreamingBody();
+ }
+
+ bool requiresSMChange(const SMEAttributes Callee,
+bool BodyOverridesInterface = false) {
+// If the transition is not through a call (e.g. when considering inlining)
+// and Callee has a streaming body, then we can ignore the interface of
+// Callee.
+if (BodyOverridesInterface && Callee.hasStreamingBody()) {
+ return !hasStreamingInterfaceOrBody();
+}
+
+if (Callee.hasStreamingCompatibleInterface())
+ return false;
+
+if (hasStreamingCompatibleInterface())
+ return true;
+
+// Both non-streaming
+if (hasNonStreamingInterfaceAndBody() && Callee.hasNonStreamingInterface())
+ return false;
+
+// Both streaming
+if (hasStreamingInterfaceOrBody() && Callee.hasStreamingInterface())
+ return false;
+
+return Callee.hasStreamingInterface();
+ }
+
+ bool hasNewZABody() { return HasNewZA; }
+ bool requiresLazySave() const { return HasNewZA; }
sdesmalen-arm wrote:
That doesn't look right. A lazy-save is required when the caller has ZA state
and the callee does not share ZA state and is a Private-ZA function.
That said, I think that we might be too restrictive (here and in LLVM) to not
allow inlining of functions that would otherwise require a lazy-save.
https://github.com/llvm/llvm-project/pull/77936
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
@@ -15,13 +15,13 @@ #define LLVM_LIB_TARGET_AARCH64_AARCH64ISELLOWERING_H #include "AArch64.h" -#include "Utils/AArch64SMEAttributes.h" #include "llvm/CodeGen/CallingConvLower.h" #include "llvm/CodeGen/MachineFunction.h" #include "llvm/CodeGen/SelectionDAG.h" #include "llvm/CodeGen/TargetLowering.h" #include "llvm/IR/CallingConv.h" #include "llvm/IR/Instruction.h" +#include "llvm/Support/AArch64SMEAttributes.h" sdesmalen-arm wrote: This change is no longer necessary? https://github.com/llvm/llvm-project/pull/77936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
@@ -814,6 +821,42 @@ Address AArch64ABIInfo::EmitMSVAArg(CodeGenFunction ,
Address VAListAddr,
/*allowHigherAlign*/ false);
}
+void AArch64TargetCodeGenInfo::checkFunctionCallABI(
+CodeGenModule , SourceLocation CallLoc, const FunctionDecl *Caller,
+const FunctionDecl *Callee, const CallArgList ) const {
+ if (!Callee->hasAttr())
+return;
+
+ auto GetSMEAttrs = [](const FunctionDecl *F) {
+llvm::SMEAttrs FAttrs;
+if (F->hasAttr())
+ FAttrs.set(llvm::SMEAttrs::Mask::SM_Enabled);
+if (auto *NewAttr = F->getAttr()) {
+ if (NewAttr->isNewZA())
+FAttrs.set(llvm::SMEAttrs::Mask::ZA_New);
+}
+if (const auto *T = F->getType()->getAs()) {
+ if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMEnabledMask)
+FAttrs.set(llvm::SMEAttrs::Mask::SM_Enabled);
+ if (T->getAArch64SMEAttributes() &
+ FunctionType::SME_PStateSMCompatibleMask)
+FAttrs.set(llvm::SMEAttrs::Mask::SM_Compatible);
+}
+return FAttrs;
+ };
+
+ auto CalleeAttrs = GetSMEAttrs(Callee);
+ auto CallerAttrs = GetSMEAttrs(Caller);
+
+ if (CallerAttrs.requiresSMChange(CalleeAttrs, true))
+CGM.getDiags().Report(CallLoc,
+ diag::err_function_always_inline_attribute_mismatch)
+<< Caller->getDeclName() << Callee->getDeclName() << "streaming";
+ if (CalleeAttrs.hasNewZABody())
+CGM.getDiags().Report(CallLoc, diag::err_function_always_inline_new_za)
+<< Callee->getDeclName();
+}
SamTebbs33 wrote:
It is, thanks!
https://github.com/llvm/llvm-project/pull/77936
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
sdesmalen-arm wrote: > > Separately, it's probably worth ensuring that the LLVM inlining passes > > don't actually perform illegal inlining for functions marked always_inline; > > looking at the code, it looks like we might end up skipping the relevant > > checks. > > The `TargetTransformInfo::areInlineCompatible` function in llvm makes these > checks. I think that Eli's point is that at the moment `areInlineCompatible` is not being called when using `always_inline`, which is something that needs changing. https://github.com/llvm/llvm-project/pull/77936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
sdesmalen-arm wrote: It seems wrong to move the header files to llvm/include/llvm/Support, but to leave the implementation in llvm/lib/AArch64 (which ends up in a different library). If you move the corresponding .cpp file to llvm/lib/Support then you don't need to link with `AArch64Utils`. https://github.com/llvm/llvm-project/pull/77936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
https://github.com/SamTebbs33 edited https://github.com/llvm/llvm-project/pull/77936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
@@ -812,6 +819,24 @@ Address AArch64ABIInfo::EmitMSVAArg(CodeGenFunction ,
Address VAListAddr,
/*allowHigherAlign*/ false);
}
+void AArch64TargetCodeGenInfo::checkFunctionCallABI(
+CodeGenModule , SourceLocation CallLoc, const FunctionDecl *Caller,
+const FunctionDecl *Callee, const CallArgList ) const {
+ if (!Callee->hasAttr())
+return;
+
+ auto CalleeStreamingMode = Sema::getArmStreamingFnType(Callee);
+ auto CallerStreamingMode = Sema::getArmStreamingFnType(Caller);
+
+ // The caller can inline the callee if their streaming modes match or the
+ // callee is streaming compatible
+ if (CalleeStreamingMode != CallerStreamingMode &&
SamTebbs33 wrote:
It is indeed valid to inline a streaming function into a locally streaming
function since at the inline point streaming is guaranteed to be on. We don't
need the streaming mode save and restore that would come from a locally
streaming function's prologue and epilogue.
https://github.com/llvm/llvm-project/pull/77936
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
https://github.com/jroelofs approved this pull request. https://github.com/llvm/llvm-project/pull/77936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
@@ -153,6 +155,11 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
}
return TargetCodeGenInfo::isScalarizableAsmOperand(CGF, Ty);
}
+
+ void checkFunctionCallABI(CodeGenModule , SourceLocation CallLoc,
SamTebbs33 wrote:
It's called from CodeGenFunction::EmitCall in CGCall.cpp. x86 implements this
function as well.
https://github.com/llvm/llvm-project/pull/77936
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [Clang][SME] Detect always_inline used with mismatched streaming attributes (PR #77936)
@@ -812,6 +819,24 @@ Address AArch64ABIInfo::EmitMSVAArg(CodeGenFunction ,
Address VAListAddr,
/*allowHigherAlign*/ false);
}
+void AArch64TargetCodeGenInfo::checkFunctionCallABI(
+CodeGenModule , SourceLocation CallLoc, const FunctionDecl *Caller,
+const FunctionDecl *Callee, const CallArgList ) const {
+ if (!Callee->hasAttr())
+return;
+
+ auto CalleeStreamingMode = Sema::getArmStreamingFnType(Callee);
+ auto CallerStreamingMode = Sema::getArmStreamingFnType(Caller);
+
+ // The caller can inline the callee if their streaming modes match or the
+ // callee is streaming compatible
+ if (CalleeStreamingMode != CallerStreamingMode &&
SamTebbs33 wrote:
Done, thanks for the idea.
https://github.com/llvm/llvm-project/pull/77936
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
