[PATCH] D101357: [CodeGen][AArch64][SVE] Substitute [rdffr, ptest] => rdffrs

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

Looks good to me.  I've a few comments that you're free to ignore.



================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1313-1321
+  const TargetRegisterInfo *TRI = &getRegisterInfo();
+
+  // If another instruction between the propagation and test sets the
+  // flags, don't remove the ptest.
+  MachineBasicBlock::iterator I = Pred, E = PTest;
+  ++I; // Skip past the predicate op itself.
+  for (; I != E; ++I)
----------------
I have no proof on this so feel free to ignore if you disagree.

Hoisting the `Pred->getParent() != PTest->getParent()` part offers a faster route to termination so I'm good with that.

However, I feel iterating across the BB's instructions may not and so that part might be better to keep until after we've decided `Mask` and `Pred` are interesting.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:1389
+      //                                  `rdffrs p1.b, p0/z` above.
+      MachineInstr *RDFFR = Pred;
+      auto *RDFFRMask = MRI->getUniqueVRegDef(RDFFR->getOperand(1).getReg());
----------------
Given there's only a single use, which matches the style of the previous blocks, I don't see much value in creating this alias to Pred.  Especially as the comment describes the situation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101357