[PATCH] D89881: [ARM] Alter t2DoLoopStart to define lr

[Dave Green via Phabricator via llvm-commits]

dmgreen marked 3 inline comments as done.
dmgreen added a comment.

In D89881#2344611 <https://reviews.llvm.org/D89881#2344611>, @samparker wrote:

> A heroic effort to change all those tests! Are you planning on also updating WLS and the test_set intrinsic too..?

Yeah, I was having fun :-/ I think all the tests are still OK (baring one that I removed, as I don't believe we can get into the state it found itself in any more). Some of them may no longer test what they originally tested though.

WLS - maybe. It really needs to be some sort of terminator that produces a value. I will look into it.

This will need an extra patch to not cause regressions with tail predication. Perhaps something like the patch that moved the t2DoLoopStart to the end of the loop, but moving things around at this stage can be difficult to get really right.



================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1079
 
     // Find an insertion point:
     // - Is there a (mov lr, Count) before Start? If so, and nothing else
----------------
samparker wrote:
> I seem to remember that these mov lr searches only affect DoLoopStart, as the WhileLoopStart would usually/always be in the predecessor block, so you might be able to remove the two searches without having to change any other tests.
I am always worried about what checks we still need and whether there are enough to not cause problems. No tests changed, which is a good sign. I'll try and run some testing too.


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

https://reviews.llvm.org/D89881