nickdesaulniers added a comment.

I still think this would be easier to review if the `isArgumentRegister` 
tablegen changes were separated out into a distinct parent patch and then the 
existing x86 implementation updated to use, then this would rebased on top of 
as a child patch.



================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:798
+
+  if (STI.hasSVE()) {
+    for (MCRegister PReg :
----------------
Reuse `HasSVE` from L771?


================
Comment at: llvm/test/CodeGen/AArch64/zero-call-used-regs.ll:2-3
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-unknown-unknown | FileCheck %s 
--check-prefix=DEFAULT
+; RUN: llc < %s -mtriple=aarch64-unknown-unknown -mattr=+sve | FileCheck %s 
--check-prefix=SVE
+
----------------
If you use `--check-prefixes=CHECK,<unique>` (ie. 
`--check-prefixes=CHECK,DEFAULT` and `--check-prefixes=CHECK,SVE`) then when 
`DEFAULT` and `SVE` match, you can just use `CHECK`.

That should help reduce the number of checks in this test significantly. 
Otherwise it's hard to tell what's different between the two cases, if anything 
at all.

update_llc_test_checks should work with --check-prefixes IME.


================
Comment at: llvm/test/CodeGen/AArch64/zero-call-used-regs.ll:259-262
+; SVE-NEXT:    pfalse p0.b
+; SVE-NEXT:    pfalse p1.b
+; SVE-NEXT:    pfalse p2.b
+; SVE-NEXT:    pfalse p3.b
----------------
N00b question about SVE: do we need `pfalse` for each of the numbered p 
registers corresponding to the x registers we zeroed? i.e. here we have pfalse 
for p0-3, yet we zero z0-7.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to