[PATCH] D83464: [MachineOutliner][AArch64] Fix for noreturn functions

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:6162
     } else {
       SetCandidateCallInfo(MachineOutlinerDefault, 12);
     }
----------------
plotfi wrote:
> From what I can tell, bailing here with a `RepeatedSequenceLocs.clear(); return outliner::OutlinedFunction();` also works. It seems to be if the candidate does not have an available LR or is noreturn, doesn't have available registers, and isn't using using the SP is is probably highly likely to get set to `MachineOutlinerDefault` unless the candidate size doesn't exceed the 12 bytes of a standard LR/BL/restore LR. 
You don't always want to bail out here.

You only want to bail out here when the candidates contain a call, or `IsNoReturn == true`.

This just says that we'll save LR to the stack.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:6198-6205
+      // We don't fix up the stack twice.
+      if (std::any_of(RepeatedSequenceLocs.begin(), RepeatedSequenceLocs.end(),
+                      [](const outliner::Candidate &C) {
+                        return C.CallConstructionID == MachineOutlinerDefault;
+                      })) {
+        RepeatedSequenceLocs.clear();
+        return outliner::OutlinedFunction();
----------------
I think this should only happen if we have MachineOutlinerDefault *and* the candidate requires stack fixups.

In the code above:

```
      // Is SP used in the sequence at all? If not, we don't have to modify
      // the stack, so we are guaranteed to get the same frame.
      else if (C.UsedInSequence.available(AArch64::SP)) {
        NumBytesNoStackCalls += 12;
        C.setCallInfo(MachineOutlinerDefault, 12);
        CandidatesWithoutStackFixups.push_back(C);
      }
```

I don't think you want to remove candidates like these.

Technically, the commit message is inaccurate here, because this is really checking for situations where we fix things up twice. noreturn functions are one such case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83464