[PATCH] D154084: [AArch64] Modify SVE Pseudo appends

Mon Jul 3 02:55:33 PDT 2023

paulwalker-arm added inline comments.

================
Comment at: llvm/unittests/Target/AArch64/AArch64SvePseudoTest.cpp:83
+
+TEST(AArch64SVESchedPseudoTest, IsCorrect) {
+  // TODO : Add more CPUs that support SVE/SVE2
----------------
dmgreen wrote:
> paulwalker-arm wrote:
> > dmgreen wrote:
> > > Can you make this two tests, one for each CPU. It can make the tests easier to work with if they fail, where it is better to have each test more independant.
> > > 
> > > The neoverse-v2 part may want to be moved to D154232 to keep that patch as an NFC. This patch could then be to improve A510.
> > @dmgreen - The AArch64SchedNeoverseV2.td changes are functional are they not? As in this patch is about adding scheduling information for SVE pseudo instructions, which coveres multiple scheduling models.  Or have I misunderstood your comment?
> The V2 scheduling model was already matching `(ABS|CNOT|NEG)_ZPmZ_UNDEF_[BHSD]$`. It needs to be changed to match `(ABS|CNOT|NEG)_ZPmZ_[BHSD]_UNDEF$` (or `(ABS|CNOT|NEG)_ZPmZ_[BHSD]` as is done here) in order to keep it matching the same instructions.
> 
> I'm not sure if there were other missing UNDEF instructions? I might just include them in D154232 if they were, but it may be better to pull it out into a different patch.
I see. Thanks.  So I was thinking D154232 would just be a literal change to move the `UNDEF` matching to where it needs to be.  Then this patch can unify the patterns as it's already doing, which would then catch any missing ones.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154084