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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/Utils/AArch64SMEAttributes.cpp:34
+    StringRef FuncName = F->getName();
+    if (FuncName == "__arm_tpidr2_save" || FuncName == "__arm_sme_state" ||
+        FuncName == "__arm_tpidr2_restore")
----------------
MattDevereau wrote:
> sdesmalen wrote:
> > Perhaps it's worth adding a new SMEAttrs constructor, such that you can use that both here and in AArch64ISelLowering.cpp
> > 
> >   SMEAttrs(StringRef funcname) {  /* handle SME ABI functions here */ }
> What benefit is this supposed to give? If you just use a StringRef param in a constructor you will have to make many assumptions to satisfy the mutex asserts in `set` and probably just end up doing Bitmask |= Za_NoLazySave with whatever comes out of that constructor anyway. Isn't getting this functionality from `AArch64ISelLowering.cpp` is what `getCalleeAttrsFromExternalFunction` is providing anyway?
My suggestion is to avoid having to write this `if (FuncName == "__arm_tpidr2_save" || ..` code in multiple places.

> you will have to make many assumptions to satisfy the mutex asserts in set and probably just end up doing Bitmask |= Za_NoLazySave with whatever comes out of that constructor anyway
What assumptions will you have to make?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159186