[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-26 Thread Sander de Smalen via cfe-commits

https://github.com/sdesmalen-arm approved this pull request.


https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-26 Thread Benjamin Maxwell via cfe-commits


@@ -1143,30 +1146,63 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmSMEInlinability : uint8_t {
+  Ok = 0,
+  MismatchedStreamingCompatibility = 1 << 0,
+  IncompatibleStreamingModes = 1 << 1,
+  CalleeRequiresNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeRequiresNewZA),
+};
 
+/// Determines if there are any Arm SME ABI issues with inlining \p Callee into
+/// \p Caller. Returns the issues in the ArmSMEInlinability bit enum (multiple
+/// bits can be set).
+static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller,
+const FunctionDecl *Callee) {
   bool CallerIsStreaming =
   IsArmStreamingFunction(Caller, /*IncludeLocallyStreaming=*/true);
   bool CalleeIsStreaming =
   IsArmStreamingFunction(Callee, /*IncludeLocallyStreaming=*/true);
   bool CallerIsStreamingCompatible = isStreamingCompatible(Caller);
   bool CalleeIsStreamingCompatible = isStreamingCompatible(Callee);
 
+  ArmSMEInlinability Inlinability = ArmSMEInlinability::Ok;
+
   if (!CalleeIsStreamingCompatible &&
-  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible))
-CGM.getDiags().Report(
-CallLoc, CalleeIsStreaming
- ? diag::err_function_always_inline_attribute_mismatch
- : diag::warn_function_always_inline_attribute_mismatch)
-<< Caller->getDeclName() << Callee->getDeclName() << "streaming";
+  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible)) 
{
+Inlinability |= ArmSMEInlinability::MismatchedStreamingCompatibility;
+if (CalleeIsStreaming)
+  Inlinability |= ArmSMEInlinability::IncompatibleStreamingModes;
+  }

MacDue wrote:

Done :+1: 

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-26 Thread Benjamin Maxwell via cfe-commits

https://github.com/MacDue updated 
https://github.com/llvm/llvm-project/pull/116391

>From 90daf9c544bcb776c8a68ad504ba5eda50eafe8a Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell 
Date: Fri, 15 Nov 2024 14:35:41 +
Subject: [PATCH 1/6] [clang][SME] Ignore flatten for callees mismatched
 streaming attributes

If `__attribute__((flatten))` is used on a function don't inline any
callees with incompatible streaming attributes. Without this check,
clang may produce incorrect code when `flatten` is used in code with
streaming functions.

Note: The docs for flatten say it can be ignored when inlining is
impossible: "causes calls within the attributed function to be inlined
unless it is impossible to do so".
---
 clang/lib/CodeGen/CGCall.cpp  | 11 ++-
 clang/lib/CodeGen/TargetInfo.h|  9 +++
 clang/lib/CodeGen/Targets/AArch64.cpp | 64 +---
 .../AArch64/sme-flatten-streaming-attrs.c | 74 +++
 4 files changed, 143 insertions(+), 15 deletions(-)
 create mode 100644 clang/test/CodeGen/AArch64/sme-flatten-streaming-attrs.c

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 8f4f5d3ed81601..b8a968fdf4e9eb 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5112,9 +5112,10 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
 
   // 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(CurCodeDecl),
-  dyn_cast_or_null(TargetDecl), CallArgs, RetTy);
+  const FunctionDecl *CallerDecl = dyn_cast_or_null(CurCodeDecl);
+  const FunctionDecl *CalleeDecl = dyn_cast_or_null(TargetDecl);
+  CGM.getTargetCodeGenInfo().checkFunctionCallABI(CGM, Loc, CallerDecl,
+  CalleeDecl, CallArgs, RetTy);
 
   // 1. Set up the arguments.
 
@@ -5705,7 +5706,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
   // FIXME: should this really take priority over __try, below?
   if (CurCodeDecl && CurCodeDecl->hasAttr() &&
   !InNoInlineAttributedStmt &&
-  !(TargetDecl && TargetDecl->hasAttr())) {
+  !(TargetDecl && TargetDecl->hasAttr()) &&
+  !CGM.getTargetCodeGenInfo().wouldInliningViolateFunctionCallABI(
+  CallerDecl, CalleeDecl)) {
 Attrs =
 Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline);
   }
diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h
index 373f8b8a80fdb1..23ff476b0e33ce 100644
--- a/clang/lib/CodeGen/TargetInfo.h
+++ b/clang/lib/CodeGen/TargetInfo.h
@@ -98,6 +98,15 @@ class TargetCodeGenInfo {
 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
+  /// or flatten attributes).
+  virtual bool
+  wouldInliningViolateFunctionCallABI(const FunctionDecl *Caller,
+  const FunctionDecl *Callee) const {
+return false;
+  }
+
   /// Determines the size of struct _Unwind_Exception on this platform,
   /// in 8-bit units.  The Itanium ABI defines this as:
   ///   struct _Unwind_Exception {
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index 9320c6ef06efab..a9ea84b6575f92 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -177,6 +177,9 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
 const FunctionDecl *Callee, const CallArgList 
&Args,
 QualType ReturnType) const override;
 
+  bool wouldInliningViolateFunctionCallABI(
+  const FunctionDecl *Caller, const FunctionDecl *Callee) const override;
+
 private:
   // Diagnose calls between functions with incompatible Streaming SVE
   // attributes.
@@ -1143,12 +1146,20 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmStreamingInlinability : uint8_t {
+  Ok = 0,
+  IncompatibleStreamingModes = 1,
+  MismatchedStreamingCompatibility = 1 << 1,
+  CalleeHasNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeHasNewZA),
+};
 
+/// Determines if there are any streaming ABI issues with inlining \p Callee
+/// into \p Caller. Returns the issues in the ArmStreamingInlinability bit enum
+/// (multiple bits can be set).
+static ArmStreamingInlinability
+GetArmStreamingInlinability(const FunctionDecl *Caller,
+  

[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-26 Thread Sander de Smalen via cfe-commits


@@ -1143,30 +1146,63 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmSMEInlinability : uint8_t {
+  Ok = 0,
+  MismatchedStreamingCompatibility = 1 << 0,
+  IncompatibleStreamingModes = 1 << 1,
+  CalleeRequiresNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeRequiresNewZA),
+};
 
+/// Determines if there are any Arm SME ABI issues with inlining \p Callee into
+/// \p Caller. Returns the issues in the ArmSMEInlinability bit enum (multiple
+/// bits can be set).
+static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller,
+const FunctionDecl *Callee) {
   bool CallerIsStreaming =
   IsArmStreamingFunction(Caller, /*IncludeLocallyStreaming=*/true);
   bool CalleeIsStreaming =
   IsArmStreamingFunction(Callee, /*IncludeLocallyStreaming=*/true);
   bool CallerIsStreamingCompatible = isStreamingCompatible(Caller);
   bool CalleeIsStreamingCompatible = isStreamingCompatible(Callee);
 
+  ArmSMEInlinability Inlinability = ArmSMEInlinability::Ok;
+
   if (!CalleeIsStreamingCompatible &&
-  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible))
-CGM.getDiags().Report(
-CallLoc, CalleeIsStreaming
- ? diag::err_function_always_inline_attribute_mismatch
- : diag::warn_function_always_inline_attribute_mismatch)
-<< Caller->getDeclName() << Callee->getDeclName() << "streaming";
+  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible)) 
{
+Inlinability |= ArmSMEInlinability::MismatchedStreamingCompatibility;
+if (CalleeIsStreaming)
+  Inlinability |= ArmSMEInlinability::IncompatibleStreamingModes;
+  }
   if (auto *NewAttr = Callee->getAttr())
 if (NewAttr->isNewZA())
-  CGM.getDiags().Report(CallLoc, diag::err_function_always_inline_new_za)
-  << Callee->getDeclName();
+  Inlinability |= ArmSMEInlinability::CalleeRequiresNewZA;
+
+  return Inlinability;
+}
+
+void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
+CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
+const FunctionDecl *Callee) const {
+  if (!Caller || !Callee || !Callee->hasAttr())
+return;
+
+  ArmSMEInlinability Inlinability = GetArmSMEInlinability(Caller, Callee);
+
+  if ((Inlinability & ArmSMEInlinability::MismatchedStreamingCompatibility) !=
+  ArmSMEInlinability::Ok)
+CGM.getDiags().Report(
+CallLoc,
+(Inlinability & ArmSMEInlinability::IncompatibleStreamingModes) !=
+ArmSMEInlinability::Ok

sdesmalen-arm wrote:

nit:
```suggestion
(Inlinability & ArmSMEInlinability::IncompatibleStreamingModes) ==
ArmSMEInlinability::IncompatibleStreamingModes
```

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-26 Thread Sander de Smalen via cfe-commits


@@ -1143,30 +1146,63 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmSMEInlinability : uint8_t {
+  Ok = 0,
+  MismatchedStreamingCompatibility = 1 << 0,
+  IncompatibleStreamingModes = 1 << 1,
+  CalleeRequiresNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeRequiresNewZA),
+};
 
+/// Determines if there are any Arm SME ABI issues with inlining \p Callee into
+/// \p Caller. Returns the issues in the ArmSMEInlinability bit enum (multiple
+/// bits can be set).
+static ArmSMEInlinability GetArmSMEInlinability(const FunctionDecl *Caller,
+const FunctionDecl *Callee) {
   bool CallerIsStreaming =
   IsArmStreamingFunction(Caller, /*IncludeLocallyStreaming=*/true);
   bool CalleeIsStreaming =
   IsArmStreamingFunction(Callee, /*IncludeLocallyStreaming=*/true);
   bool CallerIsStreamingCompatible = isStreamingCompatible(Caller);
   bool CalleeIsStreamingCompatible = isStreamingCompatible(Callee);
 
+  ArmSMEInlinability Inlinability = ArmSMEInlinability::Ok;
+
   if (!CalleeIsStreamingCompatible &&
-  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible))
-CGM.getDiags().Report(
-CallLoc, CalleeIsStreaming
- ? diag::err_function_always_inline_attribute_mismatch
- : diag::warn_function_always_inline_attribute_mismatch)
-<< Caller->getDeclName() << Callee->getDeclName() << "streaming";
+  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible)) 
{
+Inlinability |= ArmSMEInlinability::MismatchedStreamingCompatibility;
+if (CalleeIsStreaming)
+  Inlinability |= ArmSMEInlinability::IncompatibleStreamingModes;
+  }

sdesmalen-arm wrote:

I would suggest splitting out both cases so that only one of the two values is 
ever set, and then use the mask suggested in my other comment to test if either 
of the values is set.

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-26 Thread Sander de Smalen via cfe-commits


@@ -1143,30 +1146,63 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmSMEInlinability : uint8_t {
+  Ok = 0,
+  MismatchedStreamingCompatibility = 1 << 0,
+  IncompatibleStreamingModes = 1 << 1,
+  CalleeRequiresNewZA = 1 << 2,

sdesmalen-arm wrote:

I find the names quite confusing, could you change it to:
```suggestion
  ErrorIncompatibleStreamingModes = 1 << 0,
  WarnIncompatibleStreamingModes = 1 << 1,
  ErrorCalleeRequiresNewZA = 1 << 2,
```

If you add an additional `IncompatibleStreamingModes = 3,` you can test for the 
case where either is set.

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-26 Thread Benjamin Maxwell via cfe-commits

https://github.com/MacDue updated 
https://github.com/llvm/llvm-project/pull/116391

>From 90daf9c544bcb776c8a68ad504ba5eda50eafe8a Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell 
Date: Fri, 15 Nov 2024 14:35:41 +
Subject: [PATCH 1/5] [clang][SME] Ignore flatten for callees mismatched
 streaming attributes

If `__attribute__((flatten))` is used on a function don't inline any
callees with incompatible streaming attributes. Without this check,
clang may produce incorrect code when `flatten` is used in code with
streaming functions.

Note: The docs for flatten say it can be ignored when inlining is
impossible: "causes calls within the attributed function to be inlined
unless it is impossible to do so".
---
 clang/lib/CodeGen/CGCall.cpp  | 11 ++-
 clang/lib/CodeGen/TargetInfo.h|  9 +++
 clang/lib/CodeGen/Targets/AArch64.cpp | 64 +---
 .../AArch64/sme-flatten-streaming-attrs.c | 74 +++
 4 files changed, 143 insertions(+), 15 deletions(-)
 create mode 100644 clang/test/CodeGen/AArch64/sme-flatten-streaming-attrs.c

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 8f4f5d3ed81601..b8a968fdf4e9eb 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5112,9 +5112,10 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
 
   // 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(CurCodeDecl),
-  dyn_cast_or_null(TargetDecl), CallArgs, RetTy);
+  const FunctionDecl *CallerDecl = dyn_cast_or_null(CurCodeDecl);
+  const FunctionDecl *CalleeDecl = dyn_cast_or_null(TargetDecl);
+  CGM.getTargetCodeGenInfo().checkFunctionCallABI(CGM, Loc, CallerDecl,
+  CalleeDecl, CallArgs, RetTy);
 
   // 1. Set up the arguments.
 
@@ -5705,7 +5706,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
   // FIXME: should this really take priority over __try, below?
   if (CurCodeDecl && CurCodeDecl->hasAttr() &&
   !InNoInlineAttributedStmt &&
-  !(TargetDecl && TargetDecl->hasAttr())) {
+  !(TargetDecl && TargetDecl->hasAttr()) &&
+  !CGM.getTargetCodeGenInfo().wouldInliningViolateFunctionCallABI(
+  CallerDecl, CalleeDecl)) {
 Attrs =
 Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline);
   }
diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h
index 373f8b8a80fdb1..23ff476b0e33ce 100644
--- a/clang/lib/CodeGen/TargetInfo.h
+++ b/clang/lib/CodeGen/TargetInfo.h
@@ -98,6 +98,15 @@ class TargetCodeGenInfo {
 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
+  /// or flatten attributes).
+  virtual bool
+  wouldInliningViolateFunctionCallABI(const FunctionDecl *Caller,
+  const FunctionDecl *Callee) const {
+return false;
+  }
+
   /// Determines the size of struct _Unwind_Exception on this platform,
   /// in 8-bit units.  The Itanium ABI defines this as:
   ///   struct _Unwind_Exception {
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index 9320c6ef06efab..a9ea84b6575f92 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -177,6 +177,9 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
 const FunctionDecl *Callee, const CallArgList 
&Args,
 QualType ReturnType) const override;
 
+  bool wouldInliningViolateFunctionCallABI(
+  const FunctionDecl *Caller, const FunctionDecl *Callee) const override;
+
 private:
   // Diagnose calls between functions with incompatible Streaming SVE
   // attributes.
@@ -1143,12 +1146,20 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmStreamingInlinability : uint8_t {
+  Ok = 0,
+  IncompatibleStreamingModes = 1,
+  MismatchedStreamingCompatibility = 1 << 1,
+  CalleeHasNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeHasNewZA),
+};
 
+/// Determines if there are any streaming ABI issues with inlining \p Callee
+/// into \p Caller. Returns the issues in the ArmStreamingInlinability bit enum
+/// (multiple bits can be set).
+static ArmStreamingInlinability
+GetArmStreamingInlinability(const FunctionDecl *Caller,
+  

[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-26 Thread Benjamin Maxwell via cfe-commits

https://github.com/MacDue updated 
https://github.com/llvm/llvm-project/pull/116391

>From 90daf9c544bcb776c8a68ad504ba5eda50eafe8a Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell 
Date: Fri, 15 Nov 2024 14:35:41 +
Subject: [PATCH 1/5] [clang][SME] Ignore flatten for callees mismatched
 streaming attributes

If `__attribute__((flatten))` is used on a function don't inline any
callees with incompatible streaming attributes. Without this check,
clang may produce incorrect code when `flatten` is used in code with
streaming functions.

Note: The docs for flatten say it can be ignored when inlining is
impossible: "causes calls within the attributed function to be inlined
unless it is impossible to do so".
---
 clang/lib/CodeGen/CGCall.cpp  | 11 ++-
 clang/lib/CodeGen/TargetInfo.h|  9 +++
 clang/lib/CodeGen/Targets/AArch64.cpp | 64 +---
 .../AArch64/sme-flatten-streaming-attrs.c | 74 +++
 4 files changed, 143 insertions(+), 15 deletions(-)
 create mode 100644 clang/test/CodeGen/AArch64/sme-flatten-streaming-attrs.c

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 8f4f5d3ed81601..b8a968fdf4e9eb 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5112,9 +5112,10 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
 
   // 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(CurCodeDecl),
-  dyn_cast_or_null(TargetDecl), CallArgs, RetTy);
+  const FunctionDecl *CallerDecl = dyn_cast_or_null(CurCodeDecl);
+  const FunctionDecl *CalleeDecl = dyn_cast_or_null(TargetDecl);
+  CGM.getTargetCodeGenInfo().checkFunctionCallABI(CGM, Loc, CallerDecl,
+  CalleeDecl, CallArgs, RetTy);
 
   // 1. Set up the arguments.
 
@@ -5705,7 +5706,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
   // FIXME: should this really take priority over __try, below?
   if (CurCodeDecl && CurCodeDecl->hasAttr() &&
   !InNoInlineAttributedStmt &&
-  !(TargetDecl && TargetDecl->hasAttr())) {
+  !(TargetDecl && TargetDecl->hasAttr()) &&
+  !CGM.getTargetCodeGenInfo().wouldInliningViolateFunctionCallABI(
+  CallerDecl, CalleeDecl)) {
 Attrs =
 Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline);
   }
diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h
index 373f8b8a80fdb1..23ff476b0e33ce 100644
--- a/clang/lib/CodeGen/TargetInfo.h
+++ b/clang/lib/CodeGen/TargetInfo.h
@@ -98,6 +98,15 @@ class TargetCodeGenInfo {
 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
+  /// or flatten attributes).
+  virtual bool
+  wouldInliningViolateFunctionCallABI(const FunctionDecl *Caller,
+  const FunctionDecl *Callee) const {
+return false;
+  }
+
   /// Determines the size of struct _Unwind_Exception on this platform,
   /// in 8-bit units.  The Itanium ABI defines this as:
   ///   struct _Unwind_Exception {
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index 9320c6ef06efab..a9ea84b6575f92 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -177,6 +177,9 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
 const FunctionDecl *Callee, const CallArgList 
&Args,
 QualType ReturnType) const override;
 
+  bool wouldInliningViolateFunctionCallABI(
+  const FunctionDecl *Caller, const FunctionDecl *Callee) const override;
+
 private:
   // Diagnose calls between functions with incompatible Streaming SVE
   // attributes.
@@ -1143,12 +1146,20 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmStreamingInlinability : uint8_t {
+  Ok = 0,
+  IncompatibleStreamingModes = 1,
+  MismatchedStreamingCompatibility = 1 << 1,
+  CalleeHasNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeHasNewZA),
+};
 
+/// Determines if there are any streaming ABI issues with inlining \p Callee
+/// into \p Caller. Returns the issues in the ArmStreamingInlinability bit enum
+/// (multiple bits can be set).
+static ArmStreamingInlinability
+GetArmStreamingInlinability(const FunctionDecl *Caller,
+  

[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Benjamin Maxwell via cfe-commits

MacDue wrote:

Note: I've extended this PR to cover the clang-only `[[clang::always_inline]]` 
statement attribute (which is currently broken for streaming functions too). 
It's documentation is more relaxed than the GNU 
`__attribute__((always_inline))` attribute (which says failing to inline is an 
error) -- relevant snippets included above.

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Sander de Smalen via cfe-commits


@@ -1143,30 +1146,62 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmStreamingInlinability : uint8_t {
+  Ok = 0,
+  IncompatibleStreamingModes = 1,
+  MismatchedStreamingCompatibility = 1 << 1,
+  CalleeHasNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeHasNewZA),
+};
 
+/// Determines if there are any streaming ABI issues with inlining \p Callee
+/// into \p Caller. Returns the issues in the ArmStreamingInlinability bit enum
+/// (multiple bits can be set).
+static ArmStreamingInlinability
+GetArmStreamingInlinability(const FunctionDecl *Caller,
+const FunctionDecl *Callee) {
   bool CallerIsStreaming =
   IsArmStreamingFunction(Caller, /*IncludeLocallyStreaming=*/true);
   bool CalleeIsStreaming =
   IsArmStreamingFunction(Callee, /*IncludeLocallyStreaming=*/true);
   bool CallerIsStreamingCompatible = isStreamingCompatible(Caller);
   bool CalleeIsStreamingCompatible = isStreamingCompatible(Callee);
 
+  ArmStreamingInlinability Inlinability = ArmStreamingInlinability::Ok;
+
   if (!CalleeIsStreamingCompatible &&
-  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible))
+  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible)) 
{
+Inlinability |= ArmStreamingInlinability::MismatchedStreamingCompatibility;
+if (CalleeIsStreaming)
+  Inlinability |= ArmStreamingInlinability::IncompatibleStreamingModes;
+  }
+  if (auto *NewAttr = Callee->getAttr())
+if (NewAttr->isNewZA())
+  Inlinability |= ArmStreamingInlinability::CalleeHasNewZA;
+
+  return Inlinability;
+}
+
+void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
+CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
+const FunctionDecl *Callee) const {
+  if (!Caller || !Callee || !Callee->hasAttr())
+return;
+
+  ArmStreamingInlinability Inlinability =
+  GetArmStreamingInlinability(Caller, Callee);
+
+  if (bool(Inlinability &

sdesmalen-arm wrote:

nit: `bool(...)` is unnecessary?

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Benjamin Maxwell via cfe-commits


@@ -1143,30 +1146,62 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmStreamingInlinability : uint8_t {

MacDue wrote:

Gone with `ArmSMEInlinability` (since this is only about SME ABI inlinability 
issues). 

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Benjamin Maxwell via cfe-commits


@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -emit-llvm -target-feature 
+sme %s -o - | FileCheck %s

MacDue wrote:

Done :+1: 

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Benjamin Maxwell via cfe-commits


@@ -1143,30 +1146,62 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmStreamingInlinability : uint8_t {
+  Ok = 0,
+  IncompatibleStreamingModes = 1,
+  MismatchedStreamingCompatibility = 1 << 1,
+  CalleeHasNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeHasNewZA),
+};
 
+/// Determines if there are any streaming ABI issues with inlining \p Callee
+/// into \p Caller. Returns the issues in the ArmStreamingInlinability bit enum
+/// (multiple bits can be set).
+static ArmStreamingInlinability
+GetArmStreamingInlinability(const FunctionDecl *Caller,
+const FunctionDecl *Callee) {
   bool CallerIsStreaming =
   IsArmStreamingFunction(Caller, /*IncludeLocallyStreaming=*/true);
   bool CalleeIsStreaming =
   IsArmStreamingFunction(Callee, /*IncludeLocallyStreaming=*/true);
   bool CallerIsStreamingCompatible = isStreamingCompatible(Caller);
   bool CalleeIsStreamingCompatible = isStreamingCompatible(Callee);
 
+  ArmStreamingInlinability Inlinability = ArmStreamingInlinability::Ok;
+
   if (!CalleeIsStreamingCompatible &&
-  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible))
+  (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible)) 
{
+Inlinability |= ArmStreamingInlinability::MismatchedStreamingCompatibility;
+if (CalleeIsStreaming)
+  Inlinability |= ArmStreamingInlinability::IncompatibleStreamingModes;
+  }
+  if (auto *NewAttr = Callee->getAttr())
+if (NewAttr->isNewZA())
+  Inlinability |= ArmStreamingInlinability::CalleeHasNewZA;
+
+  return Inlinability;
+}
+
+void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
+CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
+const FunctionDecl *Callee) const {
+  if (!Caller || !Callee || !Callee->hasAttr())
+return;
+
+  ArmStreamingInlinability Inlinability =
+  GetArmStreamingInlinability(Caller, Callee);
+
+  if (bool(Inlinability &

MacDue wrote:

Yes, but you can do `!= 0` without a cast (so I've switched to that).

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Benjamin Maxwell via cfe-commits

https://github.com/MacDue updated 
https://github.com/llvm/llvm-project/pull/116391

>From 90daf9c544bcb776c8a68ad504ba5eda50eafe8a Mon Sep 17 00:00:00 2001
From: Benjamin Maxwell 
Date: Fri, 15 Nov 2024 14:35:41 +
Subject: [PATCH 1/4] [clang][SME] Ignore flatten for callees mismatched
 streaming attributes

If `__attribute__((flatten))` is used on a function don't inline any
callees with incompatible streaming attributes. Without this check,
clang may produce incorrect code when `flatten` is used in code with
streaming functions.

Note: The docs for flatten say it can be ignored when inlining is
impossible: "causes calls within the attributed function to be inlined
unless it is impossible to do so".
---
 clang/lib/CodeGen/CGCall.cpp  | 11 ++-
 clang/lib/CodeGen/TargetInfo.h|  9 +++
 clang/lib/CodeGen/Targets/AArch64.cpp | 64 +---
 .../AArch64/sme-flatten-streaming-attrs.c | 74 +++
 4 files changed, 143 insertions(+), 15 deletions(-)
 create mode 100644 clang/test/CodeGen/AArch64/sme-flatten-streaming-attrs.c

diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 8f4f5d3ed81601..b8a968fdf4e9eb 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5112,9 +5112,10 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
 
   // 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(CurCodeDecl),
-  dyn_cast_or_null(TargetDecl), CallArgs, RetTy);
+  const FunctionDecl *CallerDecl = dyn_cast_or_null(CurCodeDecl);
+  const FunctionDecl *CalleeDecl = dyn_cast_or_null(TargetDecl);
+  CGM.getTargetCodeGenInfo().checkFunctionCallABI(CGM, Loc, CallerDecl,
+  CalleeDecl, CallArgs, RetTy);
 
   // 1. Set up the arguments.
 
@@ -5705,7 +5706,9 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
   // FIXME: should this really take priority over __try, below?
   if (CurCodeDecl && CurCodeDecl->hasAttr() &&
   !InNoInlineAttributedStmt &&
-  !(TargetDecl && TargetDecl->hasAttr())) {
+  !(TargetDecl && TargetDecl->hasAttr()) &&
+  !CGM.getTargetCodeGenInfo().wouldInliningViolateFunctionCallABI(
+  CallerDecl, CalleeDecl)) {
 Attrs =
 Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline);
   }
diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h
index 373f8b8a80fdb1..23ff476b0e33ce 100644
--- a/clang/lib/CodeGen/TargetInfo.h
+++ b/clang/lib/CodeGen/TargetInfo.h
@@ -98,6 +98,15 @@ class TargetCodeGenInfo {
 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
+  /// or flatten attributes).
+  virtual bool
+  wouldInliningViolateFunctionCallABI(const FunctionDecl *Caller,
+  const FunctionDecl *Callee) const {
+return false;
+  }
+
   /// Determines the size of struct _Unwind_Exception on this platform,
   /// in 8-bit units.  The Itanium ABI defines this as:
   ///   struct _Unwind_Exception {
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index 9320c6ef06efab..a9ea84b6575f92 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -177,6 +177,9 @@ class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
 const FunctionDecl *Callee, const CallArgList 
&Args,
 QualType ReturnType) const override;
 
+  bool wouldInliningViolateFunctionCallABI(
+  const FunctionDecl *Caller, const FunctionDecl *Callee) const override;
+
 private:
   // Diagnose calls between functions with incompatible Streaming SVE
   // attributes.
@@ -1143,12 +1146,20 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmStreamingInlinability : uint8_t {
+  Ok = 0,
+  IncompatibleStreamingModes = 1,
+  MismatchedStreamingCompatibility = 1 << 1,
+  CalleeHasNewZA = 1 << 2,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/CalleeHasNewZA),
+};
 
+/// Determines if there are any streaming ABI issues with inlining \p Callee
+/// into \p Caller. Returns the issues in the ArmStreamingInlinability bit enum
+/// (multiple bits can be set).
+static ArmStreamingInlinability
+GetArmStreamingInlinability(const FunctionDecl *Caller,
+  

[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Sander de Smalen via cfe-commits


@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -emit-llvm -target-feature 
+sme %s -o - | FileCheck %s

sdesmalen-arm wrote:

nit: Given that the tests are otherwise identical, could you merge these with 
`sme-flatten-streaming-attrs.c` and define the attributes under a macro (and 
define them in the RUN line) ?


https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Sander de Smalen via cfe-commits

https://github.com/sdesmalen-arm edited 
https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Sander de Smalen via cfe-commits

https://github.com/sdesmalen-arm commented:

Thanks for including the `clang::always_inline` statement attribute as well.

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Sander de Smalen via cfe-commits


@@ -1143,30 +1146,62 @@ void AArch64TargetCodeGenInfo::checkFunctionABI(
   }
 }
 
-void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
-CodeGenModule &CGM, SourceLocation CallLoc, const FunctionDecl *Caller,
-const FunctionDecl *Callee) const {
-  if (!Caller || !Callee || !Callee->hasAttr())
-return;
+enum class ArmStreamingInlinability : uint8_t {

sdesmalen-arm wrote:

nit: This is a bit of a misnomer, because NewZA has nothing to do with 
streaming mode. What about calling it `ArmInlineCompatibility` ?

https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][SME] Ignore flatten/clang::always_inline statements for callees with mismatched streaming attributes (PR #116391)

2024-11-21 Thread Benjamin Maxwell via cfe-commits

https://github.com/MacDue edited 
https://github.com/llvm/llvm-project/pull/116391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits