[PATCH] D91267: [ARM] Remove copies from low overhead phi inductions.

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVETPAndVPTOptimisationsPass.cpp:161
 
-bool MVETPAndVPTOptimisations::RevertLoopWithCall(MachineLoop *ML) {
+bool MVETPAndVPTOptimisations::MergeLoopEnd(MachineLoop *ML) {
   LLVM_DEBUG(dbgs() << "MergeLoopEnd on loop " << ML->getHeader()->getName()
----------------
How about splitting this up in 2 separate functions: RevertLoopWithCall, and MergeLoopEnd? The latter is not really descriptive anymore if it still revert loops.


================
Comment at: llvm/lib/Target/ARM/MVETPAndVPTOptimisationsPass.cpp:183
 
-  return false;
+  // Remove any copies from the loop, to ensure the phi that remains is simpler.
+  Register PhiReg = LoopPhi->getOperand(0).getReg();
----------------
I do need to ramp up on this again, but it wasn't immediately clear what problem we are solving here.
First, a bit more details here in the comments would help I guess. Second, I am wondering if MIR tests would have been helpful, or if that doesn't add much in addition to the tests that needed changing.


================
Comment at: llvm/lib/Target/ARM/MVETPAndVPTOptimisationsPass.cpp:189
+  SmallVector<MachineInstr *, 4> Copies;
+  auto CheckUsers = [&Copies](Register BaseReg, ArrayRef<MachineInstr *> OK,
+                              MachineRegisterInfo *MRI) {
----------------
nit: perhaps a more descriptive name than `OK`.


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

https://reviews.llvm.org/D91267