Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include SME attributes in the name mangling of types #290

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kmclaughlin-arm
Copy link
Contributor

This change extends the name mangling of types to include the SME streaming
and ZA interface. This will avoid naming conflicts which can currently arise such
as in the following example:

  void foo(void (*f)()) { f(); }
  void foo(void (*f)() __arm_streaming) { f(); }

Without this change, both functions 'foo' above will mangle to the same
name, despite the function pointers being different.

SME streaming-mode or ZA attributes which apply to types should be
reflected in the name mangling. This will avoid conflicts such as
in the following example:

  void foo(void (*f)()) { f(); }
  void foo(void (*f)() __arm_streaming) { f(); }

Without this change, both functions 'foo' above will mangle to the
same name despite the function pointers being different.
@kmclaughlin-arm
Copy link
Contributor Author

@rsandifo-arm

@@ -3111,6 +3112,34 @@ instead.
The SVE tuple types are mangled using their ``arm_sve.h`` names
(``svBASExN_t``).

Types which have an SME streaming or ZA interface should include an additional suffix as described in the table below:
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding suffixes like this would conflict with the existing mangling scheme for types. E.g. sm would normally indicate an extra short argument followed by an extra unsigned long argument (spec):

$ c++filt
_Z1fPFu10__SVInt8_tsmE
f(__SVInt8_t (*)(short, unsigned long))

I wondered whether we could use a vendor-specific qualifier (U…), but function argument types are understandably mangled without a qualifier. (For example, there is no difference between f(const int) and f(int).)

For __attribute__((arm_sve_vector_bits(N))), we used a fake template type __SVE_VLS<…, N> to attach the vector length N to the underlying standard SVE vector type. Perhaps we could do the same sort of thing here: have a fake template type that wraps the unadorned function type and adds information about the streaming mode and shared state? This would probably mean encoding that information as C++ expressions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I hopelessly confused myself while writing and rewriting the comment above, so gave the wrong reason. The problem isn't that function argument types have no qualifiers, but that function types themselves have only cv-qualifiers. The spec hard-codes markup for transactional safety (Tx), but AFAIK there's no general mechanism for adding vendor extensions at the function type level.

The conclusion is the same though :-)

Choose a reason for hiding this comment

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

We don't need to qualify functions; we don't support defining both a streaming and non-streaming function with the same signature. We only need to qualify function pointers (which, as pointers, do support qualifiers).

@kmclaughlin-arm
Copy link
Contributor Author

…ling

  the SME attributes in the same way as a template, similar to how the
  arm_sve_vector_bits attribute is handled.
kmclaughlin-arm added a commit to kmclaughlin-arm/llvm-project that referenced this pull request Oct 30, 2024
…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

.. code-block:: c++

// Mangled as fP11__SME_ATTRSIFu10__SVInt8_tELj1ELj0ELj0EE
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's a missing v after the function's return type:

Empty parameter lists, whether declared as () or conventionally as (void), are encoded with a void parameter specifier (v). Therefore function types always encode at least one parameter type, and function manglings can always be distinguished from data manglings by the presence of the type. Member functions do not encode the types of implicit parameters, either this or the VTT parameter.

Also, we should probably include the leading _Z1. I think that gives: _Z1fP11__SME_ATTRSIFu10__SVInt8_tvELj1ELj0ELj0EE (verified with c++filt and compiling a dummy workalike).

@@ -3111,6 +3112,65 @@ instead.
The SVE tuple types are mangled using their ``arm_sve.h`` names
(``svBASExN_t``).

Function types which have SME streaming, ZA interface or ZT0 interface attributes must include these attributes in the name mangling of the type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Like @efriedma-quic says, the intention was to include this mangling if and only if the mangling would normally include the return type of the function. We should probably say that explicitly.

+========================+==========================+
| No ZA State (default) | 0 |
+------------------------+--------------------------+
| Shared ZA | 1 |
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason for mangling the ZA information is to deal with the fact that the sharing attributes create distinct function types for overloading purposes. I think this means that we need to represent all five possibilities:

  • normal
  • __arm_in("za")
  • __arm_inout("za")
  • __arm_out("za")
  • __arm_preserves("za")

Similarly for zt0.

Since that classification is only described in the ACLE, we should probably describe the mangling there too. Sorry for only realising that now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @rsandifo-arm, I've updated this to represent each of the shared ZA/ZT0 attributes separately and moved these changes to the ACLE (ARM-software/acle#358).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants