[PATCH] D159186: [AArch64][SME] Enable TPIDR2 lazy-save for za_preserved

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 31 00:14:35 PDT 2023


sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/Utils/AArch64SMEAttributes.h:79
   bool hasPrivateZAInterface() const { return !hasSharedZAInterface(); }
-  bool preservesZA() const { return Bitmask & ZA_Preserved; }
+  bool hasPreservedZAInterface() const { return Bitmask & ZA_Preserved; }
   bool hasZAState() const {
----------------
MattDevereau wrote:
> sdesmalen wrote:
> > Could you keep the original name please?
> Can I do it as a separate change at least? It doesn't match the rest of the class's interface at all.
The `preserving ZA` is not really an interface in the sense that the ABI defines e.g. a SharedZA/PrivateZA/Streaming interface. The 'interface' part of the name refers to the caller having to be aware of the callee's interface when generating code for the call.

For example, a private-ZA interface requires the caller to set up the lazy-save mechanism when the caller has ZA state.
Or a streaming interface requires the caller to ensure that streaming-SVE mode is enabled before the call.

The `__arm_preserves_za` attribute (or `aarch64_pstate_za_preserved` as it's named in LLVM IR) is more of a 'hint' to the compiler to generate more efficient code. It is a property that can be ignored by the compiler and the generated code would still be correct.

Note that I pushed a change yesterday to rename `hasNewZAInterface()` in rG0a32a999aea07e373e1b3c9e6f0d20d3f7f1a7b8, because 'new ZA' is not part of the interface (externally visible to callers), but rather an implementation property of the function's body/definition.

As such, I'd rather stick with `preservesZA` as the name of this interface.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159186/new/

https://reviews.llvm.org/D159186



More information about the llvm-commits mailing list