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

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 7 07:40:32 PST 2020


dmgreen 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()
----------------
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.


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

https://reviews.llvm.org/D91267



More information about the llvm-commits mailing list