[Clang][AArch64] Include SME attributes in the name mangling of function types #114209

kmclaughlin-arm · 2024-10-30T11:30:50Z

Similar to arm_sve_vector_bits, the mangling of function types is implemented as a pseudo template if there are any SME attributes present, i.e.

__SME_ATTRS<normal_function_type, sme_state>

For example, the following function:

void f(svint8_t (*fn)() __arm_streaming) { fn(); }

would be mangled as:

_Z1fP11__SME_ATTRSIFu10__SVInt8_tELj1EE

See ARM-software/acle#358

efriedma-quic · 2024-10-30T21:26:26Z

clang/lib/AST/ItaniumMangle.cpp

+  Out << (hasSharedState(FunctionType::getArmZAState(SMEAttrs)) ? "Lj1E"
+                                                                : "Lj0E");
+  Out << (hasSharedState(FunctionType::getArmZT0State(SMEAttrs)) ? "Lj1E"
+                                                                 : "Lj0E");


Sema currently accepts the following:

__arm_new("zt0") void fn_zt0_in(int (*foo)() __arm_in("zt0")) { foo(); } __arm_new("zt0") void fn_zt0_in(int (*foo)() __arm_inout("zt0")) { foo(); }

So either we can't unify the manglings like this, or Sema needs to reject this. (I think the only reasonable way to get Sema to reject is to make the canonical FunctionType indistinguishable. Which seems a little weird.)

Hi @efriedma-quic, these should now receive different mangled names after the changes to ARM-software/acle#358, as all of attributes are represented individually.

github-actions · 2024-11-25T15:46:37Z

✅ With the latest revision this PR passed the C/C++ code formatter.

llvmbot · 2024-11-25T15:46:52Z

@llvm/pr-subscribers-clang

Author: Kerry McLaughlin (kmclaughlin-arm)

Changes

Similar to arm_sve_vector_bits, the mangling of function types is implemented as a pseudo template if there are any SME attributes present, i.e.

__SME_ATTRS<normal_function_type, sme_state>

For example, the following function:

void f(svint8_t (*fn)() __arm_streaming) { fn(); }

would be mangled as:

_Z1fP11__SME_ATTRSIFu10__SVInt8_tELj1EE

See ARM-software/acle#358

Full diff: https://github.com/llvm/llvm-project/pull/114209.diff

2 Files Affected:

(modified) clang/lib/AST/ItaniumMangle.cpp (+70)
(added) clang/test/CodeGenCXX/aarch64-mangle-sme-atts.cpp (+74)

diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b3e46508cf596d..23c8f255c7c16e 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -575,6 +575,7 @@ class CXXNameMangler {
   static StringRef getCallingConvQualifierName(CallingConv CC);
   void mangleExtParameterInfo(FunctionProtoType::ExtParameterInfo info);
   void mangleExtFunctionInfo(const FunctionType *T);
+  void mangleSMEAttrs(unsigned SMEAttrs);
   void mangleBareFunctionType(const FunctionProtoType *T, bool MangleReturnType,
                               const FunctionDecl *FD = nullptr);
   void mangleNeonVectorType(const VectorType *T);
@@ -3535,6 +3536,68 @@ void CXXNameMangler::mangleExtFunctionInfo(const FunctionType *T) {
   // FIXME: noreturn
 }
 
+unsigned getZAState(unsigned SMEAttrs) {
+  switch (SMEAttrs) {
+  case FunctionType::ARM_In:
+    return 1;
+  case FunctionType::ARM_Out:
+    return 2;
+  case FunctionType::ARM_InOut:
+    return 3;
+  case FunctionType::ARM_Preserves:
+    return 4;
+  default:
+    return 0;
+  }
+}
+
+// The mangling scheme for function types which have SME attributes is implemented as
+// a "pseudo" template:
+//
+//   '__SME_ATTRS<<normal_function_type>, <sme_state>>'
+//
+// Combining the function type with a bitmask representing the streaming and ZA properties
+// of the function's interface. The bits of sme_state are defined as follows:
+//    0:  Streaming Mode
+//    1:  Streaming Compatible
+//    2:  ZA Agnostic
+//  3-5:  ZA State
+//  6-8:  ZT0 State
+//  9-63: 0, reserved for future type attributes.
+//
+// For example:
+//  void f(svint8_t (*fn)() __arm_streaming_compatible __arm_inout("za")) { fn(); }
+//
+// The function fn is described as '__SME_ATTRS<Fu10__SVInt8_tvE, 26u>' and mangled as:
+//
+//  "11__SME_ATTRSI" + function type mangling + "Lj" + bitmask + "EE"
+//
+//  i.e. "11__SME_ATTRSIFu10__SVInt8_tvELj26EE"
+//
+void CXXNameMangler::mangleSMEAttrs(unsigned SMEAttrs) {
+  if (!SMEAttrs)
+    return;
+
+  // Streaming Mode
+  unsigned Bitmask = 0;
+  if (SMEAttrs & FunctionType::SME_PStateSMEnabledMask)
+    Bitmask |= 1;
+  else if (SMEAttrs & FunctionType::SME_PStateSMCompatibleMask)
+    Bitmask |= 1 << 1;
+
+  // TODO: Must represent __arm_agnostic("sme_za_state")
+
+  // ZA-State
+  Bitmask |= getZAState(FunctionType::getArmZAState(SMEAttrs)) << 3;
+
+  // ZT0 State
+  Bitmask |= getZAState(FunctionType::getArmZT0State(SMEAttrs)) << 6;
+
+  Out << "Lj" << Bitmask << "EE";
+
+  return;
+}
+
 void
 CXXNameMangler::mangleExtParameterInfo(FunctionProtoType::ExtParameterInfo PI) {
   // Vendor-specific qualifiers are emitted in reverse alphabetical order.
@@ -3572,6 +3635,11 @@ CXXNameMangler::mangleExtParameterInfo(FunctionProtoType::ExtParameterInfo PI) {
 // <function-type> ::= [<CV-qualifiers>] F [Y]
 //                      <bare-function-type> [<ref-qualifier>] E
 void CXXNameMangler::mangleType(const FunctionProtoType *T) {
+  unsigned SMEAttrs = T->getAArch64SMEAttributes();
+
+  if (SMEAttrs)
+    Out << "11__SME_ATTRSI";
+
   mangleExtFunctionInfo(T);
 
   // Mangle CV-qualifiers, if present.  These are 'this' qualifiers,
@@ -3606,6 +3674,8 @@ void CXXNameMangler::mangleType(const FunctionProtoType *T) {
   mangleRefQualifier(T->getRefQualifier());
 
   Out << 'E';
+
+  mangleSMEAttrs(SMEAttrs);
 }
 
 void CXXNameMangler::mangleType(const FunctionNoProtoType *T) {
diff --git a/clang/test/CodeGenCXX/aarch64-mangle-sme-atts.cpp b/clang/test/CodeGenCXX/aarch64-mangle-sme-atts.cpp
new file mode 100644
index 00000000000000..09db59ac621a22
--- /dev/null
+++ b/clang/test/CodeGenCXX/aarch64-mangle-sme-atts.cpp
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -target-feature +sme -target-feature +sme2 %s -emit-llvm -o - | FileCheck %s
+
+typedef __attribute__((neon_vector_type(2))) int int32x2_t;
+
+//
+// Streaming-Mode Attributes
+//
+
+// CHECK: define dso_local void @_Z12fn_streamingP11__SME_ATTRSIFvvELj1EE
+void fn_streaming(void (*foo)() __arm_streaming) { foo(); }
+
+// CHECK: define dso_local void @_Z23fn_streaming_compatibleP11__SME_ATTRSIFivELj2EE(
+void fn_streaming_compatible(int (*foo)() __arm_streaming_compatible) { foo(); }
+
+//
+// ZA Attributes
+//
+
+// CHECK: define dso_local void @_Z15fn_za_preservedP11__SME_ATTRSIF11__Int32x2_tvELj32EE(
+__arm_new("za") void fn_za_preserved(int32x2_t (*foo)() __arm_preserves("za")) { foo(); }
+
+// CHECK: define dso_local void @_Z8fn_za_inP11__SME_ATTRSIFvu13__SVFloat64_tELj8EES_(
+__arm_new("za") void fn_za_in(void (*foo)(__SVFloat64_t) __arm_in("za"), __SVFloat64_t x) { foo(x); }
+
+// CHECK: define dso_local noundef i32 @_Z9fn_za_outP11__SME_ATTRSIFivELj16EE(
+__arm_new("za") int fn_za_out(int (*foo)() __arm_out("za")) { return foo(); }
+
+// CHECK: define dso_local void @_Z11fn_za_inoutP11__SME_ATTRSIFvvELj24EE(
+__arm_new("za") void fn_za_inout(void (*foo)() __arm_inout("za")) { foo(); }
+
+
+//
+// ZT0 Attributes
+//
+
+// CHECK: define dso_local void @_Z16fn_zt0_preservedP11__SME_ATTRSIFivELj256EE(
+__arm_new("zt0") void fn_zt0_preserved(int (*foo)() __arm_preserves("zt0")) { foo(); }
+
+// CHECK: define dso_local void @_Z9fn_zt0_inP11__SME_ATTRSIFivELj64EE(
+__arm_new("zt0") void fn_zt0_in(int (*foo)() __arm_in("zt0")) { foo(); }
+
+// CHECK: define dso_local void @_Z10fn_zt0_outP11__SME_ATTRSIFivELj128EE(
+__arm_new("zt0") void fn_zt0_out(int (*foo)() __arm_out("zt0")) { foo(); }
+
+// CHECK: define dso_local void @_Z12fn_zt0_inoutP11__SME_ATTRSIFivELj192EE(
+__arm_new("zt0") void fn_zt0_inout(int (*foo)() __arm_inout("zt0")) { foo(); }
+
+//
+// Streaming-mode, ZA & ZT0 Attributes
+//
+
+// CHECK: define dso_local void @_Z17fn_all_attr_typesP11__SME_ATTRSIFivELj282EE(
+__arm_new("za") __arm_new("zt0")
+void fn_all_attr_types(int (*foo)() __arm_streaming_compatible __arm_inout("za") __arm_preserves("zt0"))
+{ foo(); }
+
+//
+// No SME Attributes
+//
+
+// CHECK: define dso_local void @_Z12no_sme_attrsPFvvE(
+void no_sme_attrs(void (*foo)()) { foo(); }
+
+// CHECK: define dso_local void @_Z24locally_streaming_callerPFvvE(
+__arm_locally_streaming void locally_streaming_caller(void (*foo)()) { foo(); }
+
+// CHECK: define dso_local void @_Z16streaming_callerv(
+void streaming_caller() __arm_streaming {}
+
+// CHECK: define dso_local void @_Z16za_shared_callerv(
+void za_shared_caller() __arm_in("za") {}
+
+// CHECK: define dso_local void @_Z17zt0_shared_callerv(
+void zt0_shared_caller() __arm_out("zt0") {}

sdesmalen-arm · 2024-11-25T15:45:27Z

clang/lib/AST/ItaniumMangle.cpp

@@ -3536,35 +3536,64 @@ void CXXNameMangler::mangleExtFunctionInfo(const FunctionType *T) {
  // FIXME: noreturn
 }

-bool hasSharedState(unsigned SMEAttrs) {
+unsigned getZAState(unsigned SMEAttrs) {


nit: use the word encode instead of get?

sdesmalen-arm · 2024-11-25T15:53:55Z

clang/lib/AST/ItaniumMangle.cpp

  if (SMEAttrs & FunctionType::SME_PStateSMEnabledMask)
-    Out << "Lj1E";
+    Bitmask |= 1;


Could you create an enum for these mask/shift values? That would also remove the need for the lengthy comment above (because the code would now represent it), and you can then link to the AAPCS for the documentation of those bits.

I've removed details of what each bit represents from the comment now that there is an enum with this information. I've still left in a short description of the template, similar to what has been written for the arm_sve_vector_bits attribute.

sdesmalen-arm · 2024-11-25T16:05:04Z

clang/lib/AST/ItaniumMangle.cpp

  }
 }

+// The mangling scheme for function types which have SME attributes is implemented as


Could you add a link to the (corresponding section in the) AAPCS for this?

sdesmalen-arm · 2024-11-25T16:08:31Z

clang/lib/AST/ItaniumMangle.cpp

  default:
-    return false;
+    return 0;


I think only case ARM_None should return 0. Any other value (default) should be an llvm_unreachable.

sdesmalen-arm · 2024-11-25T17:08:15Z

clang/lib/AST/ItaniumMangle.cpp

@@ -3535,6 +3536,74 @@ void CXXNameMangler::mangleExtFunctionInfo(const FunctionType *T) {
  // FIXME: noreturn
 }

+enum SMEState {


Please make this an enum class instead. Also, I'd recommend renaming this class so that it's clear that this relates to AAPCS type mangling.

sdesmalen-arm · 2024-11-25T17:12:41Z

clang/lib/AST/ItaniumMangle.cpp

+  Preserves = 0b100
+};
+
+unsigned encodeZAState(unsigned SMEAttrs) {


Suggested change

unsigned encodeZAState(unsigned SMEAttrs) {

static unsigned encodeAAPCSZAState(unsigned SMEAttrs) {

sdesmalen-arm · 2024-11-25T17:13:30Z

clang/lib/AST/ItaniumMangle.cpp

@@ -3535,6 +3536,74 @@ void CXXNameMangler::mangleExtFunctionInfo(const FunctionType *T) {
  // FIXME: noreturn
 }

+enum SMEState {
+  Normal = 0,


nit: I'd remove Normal and just keep these values for bits that would be set.

sdesmalen-arm · 2024-11-25T17:15:25Z

clang/lib/AST/ItaniumMangle.cpp

+  SM_Enabled = 1 << 0,
+  SM_Compatible = 1 << 1,
+  ZA_Agnostic = 1 << 2,
+  ZA_Shift = 3,
+  ZT0_Shift = 6,


Suggested change

SM_Enabled = 1 << 0,

SM_Compatible = 1 << 1,

ZA_Agnostic = 1 << 2,

ZA_Shift = 3,

ZT0_Shift = 6,

ArmStreamingBit = 1 << 0,

ArmStreamingCompatibleBit = 1 << 1,

ArmAgnosticSMEZAStateBit << 2,

ZA_Shift = 3,

ZT0_Shift = 6,

sdesmalen-arm · 2024-11-25T17:19:19Z

clang/lib/AST/ItaniumMangle.cpp

+  None = 0b000,
+  In = 0b001,
+  Out = 0b010,
+  InOut = 0b011,
+  Preserves = 0b100


Suggested change

None = 0b000,

In = 0b001,

Out = 0b010,

InOut = 0b011,

Preserves = 0b100

NoState = 0b000,

ArmIn = 0b001,

ArmOut = 0b010,

ArmInOut = 0b011,

ArmPreserves = 0b100

sdesmalen-arm · 2024-11-25T17:19:43Z

clang/lib/AST/ItaniumMangle.cpp

+  case FunctionType::ARM_Preserves:
+    return SMEState::Preserves;
+  }
+  llvm_unreachable("Unrecognised SME attribute");


This is missing a default (would otherwise result in warning diagnostic), you could move the llvm_unreachable under the default case.

sdesmalen-arm · 2024-11-25T17:21:38Z

clang/lib/AST/ItaniumMangle.cpp

+
+  Out << "Lj" << Bitmask << "EE";
+
+  return;


no need for return;

sdesmalen-arm · 2024-11-25T17:22:00Z

clang/lib/AST/ItaniumMangle.cpp

+    return;
+
+  // Streaming Mode
+  unsigned Bitmask = SMEState::Normal;


Suggested change

unsigned Bitmask = SMEState::Normal;

unsigned Bitmask = 0;

sdesmalen-arm · 2024-11-25T17:29:45Z

clang/lib/AST/ItaniumMangle.cpp

+
+  // TODO: Must represent __arm_agnostic("sme_za_state")
+
+  // ZA-State


nit: remove comment (same below).

sdesmalen-arm · 2024-11-25T17:35:47Z

clang/lib/AST/ItaniumMangle.cpp

+// The mangling scheme is otherwise defined in the appendices to the Procedure
+// Call Standard for the Arm Architecture, see
+// https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#appendix-c-mangling


Sorry, I got confused and thought this was added to the AAPCS. Could you add the link to the ACLE section instead?

sdesmalen-arm · 2024-11-28T13:54:42Z

clang/lib/AST/ItaniumMangle.cpp

+static unsigned encodeAAPCSZAState(unsigned SMEAttrs) {
+  switch (SMEAttrs) {
+  case FunctionType::ARM_None:
+    return static_cast<unsigned>(AAPCSBitmaskSME::NoState);


You can remove the static_cast if you use BitmaskEnum, see LLVM_MARK_AS_BITMASK_ENUM or LLVM_DECLARE_ENUM_AS_BITMASK.

Unfortunately I found that operator<< is not supported (needed below in mangleSMEAttrs), so I've created #118007.

…ion types. Similar to arm_sve_vector_bits, the mangling of function types is implemented as a pseudo template if there are any SME attributes present, i.e. __SME_ATTRS<normal_function_type, streaming_mode, za_state, zt0_state> For example, the following function: void f(svint8_t (*fn)() __arm_streaming) { fn(); } is mangled as: fP9__SME_ATTRSIFu10__SVInt8_tELj1ELj0ELj0EE See ARM-software/abi-aa#290

represented separately. - Removed hasSharedState.

- Renamed getZAState -> encodeZAState - Added llvm_unreachable to encodeZAState - Added a link to the AAPCS in the comment above mangleSMEAttrs

- Renamed encodeZAState -> encodeAAPCSZAState - Removed Normal from SMEState enum - Added a link to the relevant section of the AArch64 ACLE doc

…APCSZAState

sdesmalen-arm · 2024-11-29T15:14:44Z

clang/lib/AST/ItaniumMangle.cpp

+
+  unsigned Bitmask = 0;
+  if (SMEAttrs & FunctionType::SME_PStateSMEnabledMask)
+    Bitmask |= static_cast<unsigned>(AAPCSBitmaskSME::ArmStreamingBit);


You can remove the static_cast if you change Bitmask to AAPCSBitmaskSME. Then the only place that still requires a bitcast is where you print Bitmask to Out.

… static_casts

efriedma-quic reviewed Oct 30, 2024

View reviewed changes

kmclaughlin-arm requested review from huntergr-arm and sdesmalen-arm November 25, 2024 15:43

kmclaughlin-arm marked this pull request as ready for review November 25, 2024 15:46

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 25, 2024

sdesmalen-arm reviewed Nov 25, 2024

View reviewed changes

sdesmalen-arm reviewed Nov 28, 2024

View reviewed changes

kmclaughlin-arm added 5 commits November 29, 2024 14:29

- Attributes are now encoded as one bitmask, with each of the attributes

153c45e

represented separately. - Removed hasSharedState.

- Added SMEState enum

cada212

- Renamed getZAState -> encodeZAState - Added llvm_unreachable to encodeZAState - Added a link to the AAPCS in the comment above mangleSMEAttrs

- Changed name of SMEState & changed to enum class

b264d84

- Renamed encodeZAState -> encodeAAPCSZAState - Removed Normal from SMEState enum - Added a link to the relevant section of the AArch64 ACLE doc

- Added LLVM_MARK_AS_BITMASK_ENUM & removed static_casts from encodeA…

e84a781

…APCSZAState

kmclaughlin-arm force-pushed the sme-type-mangling branch from cf2b646 to e84a781 Compare November 29, 2024 14:49

sdesmalen-arm reviewed Nov 29, 2024

View reviewed changes

- Changed LargestValue for LLVM_MARK_AS_BITMASK_ENUM and removed more…

3a9e2d4

… static_casts

sdesmalen-arm approved these changes Nov 29, 2024

View reviewed changes

kmclaughlin-arm merged commit 34d4cd8 into llvm:main Dec 2, 2024
8 checks passed

kmclaughlin-arm deleted the sme-type-mangling branch December 13, 2024 11:46

	unsigned encodeZAState(unsigned SMEAttrs) {
	static unsigned encodeAAPCSZAState(unsigned SMEAttrs) {


		// TODO: Must represent __arm_agnostic("sme_za_state")

		// ZA-State

[Clang][AArch64] Include SME attributes in the name mangling of function types #114209

[Clang][AArch64] Include SME attributes in the name mangling of function types #114209

Uh oh!

Conversation

kmclaughlin-arm commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 25, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kmclaughlin-arm commented Oct 30, 2024 •

edited

Loading

github-actions bot commented Nov 25, 2024 •

edited

Loading