[PATCH] D36721: [MachineOutliner] AArch64: Avoid saving + restoring LR if possible

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:4548-4561
+  auto LRIsUnavailable = [&LRU](MachineInstr &MI) {
+    bool Ret = true;
+    if (LRU.available(AArch64::LR))
+      Ret = false;
+    LRU.stepBackward(MI);
+    return Ret;
+  };
----------------
This still checks for LR being available for the whole range between the end of the basic block and `CallInsertionPt`. I would expect it to be enough to `stepBackward()` till CallInsertionPt and only then check if LR is unused.


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:4592-4593
 
-  // Is the last instruction in this class a terminator?
-  if (CandidateClass[0].second->isTerminator())
-    return std::make_pair(0, MachineOutlinerTailCallFn);
+  else if (std::all_of(RepeatedSequenceLocs.begin(), RepeatedSequenceLocs.end(),
+                       DoesntNeedLRSave)) {
+    CallID = MachineOutlinerNoLRSave;
----------------
llvms version of `all_of` even supports ranges I think, so you could do: `all_of(RepeatedSequenceLocs, DoesntNeedLRSave)` here.

And for the record: I personally prefer imperative for loops (at least in cases where you have to specifically create a lambda for the predicate); but that is subjective so please do not change it back or the next reviewer will just ask for `all_of` again and we circle around ;-)


================
Comment at: test/CodeGen/AArch64/machine-outliner.mir:112-139
+alignment:       2
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:       
----------------
This test can be simplified, look at http://llvm.org/docs/MIRLangRef.html#simplifying-mir-files for some tactics.


https://reviews.llvm.org/D36721