[PATCH] D83923: [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:6171-6172
+    //
+    // Case 1: Outline resulting in a noreturn caller of an outlined callee that
+    //         has other calls requiring stack fixups.
+    //
----------------
Now that I've thought about this a bit more, I don't think we should mention noreturn at all.

The only case we actually care about is the case in the example below. Whether or not, say, `function_containing_sequence` is noreturn is not relevant. The noreturn case only makes it so that we can never have:

```
function_containing_sequence:
  ...
  bl OUTLINED_FUNCTION_N
  ...
```

The actual problem we want to address here is MachineOutlinerDefault + requiring a save + restore to SP.

noreturn functions can force us into that case, but they don't disallow say,

```
function_containing_sequence:
  ...
  mov xn, lr
  bl OUTLINED_FUNCTION_N
  mov lr, xn
  ...
```

So there's only one case here: when at the call-site, and inside the function itself, we must modify the stack.





================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:6178-6183
+    // In other words if function_containing_sequence in the following pseudo
+    // assembly is either:
+    // - noreturn
+    // - Requires that we save LR at the point of the call, but there are no
+    //   available registers. In this case, we save using SP.
+    //
----------------
considering the comment I left above, you can probably just remove this


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:6212
+      erase_if(RepeatedSequenceLocs, [](outliner::Candidate &C) {
+        if (std::any_of(C.front(), C.back(),
+                        [](const MachineInstr &MI) { return MI.isCall(); })) {
----------------
maybe use llvm's `any_of` from STLExtras?


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-noreturn-no-stack.mir:1
+# RUN: llc -mtriple=arm64-apple-ios -run-pass=prologepilog -run-pass=machine-outliner -verify-machineinstrs  %s -o - | FileCheck %s
+
----------------
Can you add a test for the non-noreturn case?

Also can you add a test which verifies that the stuff below works?

```
f1_doesnt_need_stack:
  ...
  seq containing call <- should be outlined
  ...
f2_doesnt_need_stack:
  ...
  seq containing call <- should be outlined
  ...
needs_stack_not_noreturn:
  ...
  seq containing call <- should not be outlined
  ...
needs_stack_noreturn:
  ...
  seq containing call <- should not be outlined
  ...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83923