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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks like a good change to me, and this looked reasonable:

> I'm not sure about doing it in general. We are replacing a phi that accepts many registers with one that only accepts 1. We are being very careful here that it is OK to do that, but in general that might not always be true.

But perhaps wait a day with committing in case @samparker has more ideas/comments about this:



================
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()
----------------
dmgreen wrote:
> SjoerdMeijer wrote:
> > How about splitting this up in 2 separate functions: RevertLoopWithCall, and MergeLoopEnd? The latter is not really descriptive anymore if it still revert loops.
> This is one of those cases where splitting up the patch has caused more trouble than it has helped! I was trying to make the later patch simpler but it has made these ones more confusing as a result.
> 
> We don't really need to revert calls here _unless_ we are converting to a t2LoopEndDec. If we do convert, we need to ensure that there is nothing else using LR (so no calls), and that there is no other uses of the induction variable, so all the copies are removed. They are really just parts of converting to a t2LoopEndDec that I was trying to make work on their own, so split them into separate patches.
> 
> I've added a comment to the start of the function to explain this (which isn't true quite yet, but will be once the other patches are in). Hopefully that makes it OK? We can still split it out if you think that's better, but there is at least some method to this madness.
Thanks, this makes it a lot better!


================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/count_dominates_start.mir:128
   ; CHECK:   [[COPY5:%[0-9]+]]:gpr = COPY [[t2MOVi2]]
-  ; CHECK:   [[COPY6:%[0-9]+]]:gpr = COPY [[t2DoLoopStart]]
-  ; CHECK:   [[COPY7:%[0-9]+]]:gprnopc = COPY [[COPY]]
+  ; CHECK:   [[COPY6:%[0-9]+]]:rgpr = COPY [[COPY]]
+  ; CHECK:   [[t2DoLoopStartTP:%[0-9]+]]:gprlr = t2DoLoopStartTP [[COPY4]], [[COPY6]]
----------------
Ah, sorry, must have overlooked this MIR test in my first review.


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

https://reviews.llvm.org/D91267