Skip to content

Conversation

@kmclaughlin-arm
Copy link
Contributor

@kmclaughlin-arm kmclaughlin-arm commented Oct 30, 2024

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

Out << (hasSharedState(FunctionType::getArmZAState(SMEAttrs)) ? "Lj1E"
: "Lj0E");
Out << (hasSharedState(FunctionType::getArmZT0State(SMEAttrs)) ? "Lj1E"
: "Lj0E");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kmclaughlin-arm kmclaughlin-arm marked this pull request as ready for review November 25, 2024 15:46
@llvmbot 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
@github-actions
Copy link

github-actions bot commented Nov 25, 2024

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

@llvmbot
Copy link
Member

llvmbot commented Nov 25, 2024

@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&lt;normal_function_type, sme_state&gt;

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") {}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use the word encode instead of get?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Comment on lines 3546 to 3550
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
None = 0b000,
In = 0b001,
Out = 0b010,
InOut = 0b011,
Preserves = 0b100
NoState = 0b000,
ArmIn = 0b001,
ArmOut = 0b010,
ArmInOut = 0b011,
ArmPreserves = 0b100

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for return;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
unsigned Bitmask = SMEState::Normal;
unsigned Bitmask = 0;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove comment (same below).

Comment on lines 3577 to 3579
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kmclaughlin-arm kmclaughlin-arm merged commit 34d4cd8 into llvm:main Dec 2, 2024
8 checks passed
@kmclaughlin-arm kmclaughlin-arm deleted the sme-type-mangling branch December 13, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants