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

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau 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")
----------------
sdesmalen wrote:
> 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?
In the end I realised what you meant and I uploaded another patch set with the StringRef constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159186