[PATCH] D78062: Adding Comment Annotations to Outlined Functions

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:6376-6377
     const outliner::OutlinedFunction &OF) const {
   // For thunk outlining, rewrite the last instruction from a call to a
   // tail-call.
+
----------------
Can we move this comment into the thunk case?

Having it above the `OutlinerString` is somewhat out of place IMO.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:6397-6398
+    OutlinerString = "Thunk";
+  } else if (OF.FrameConstructionID == MachineOutlinerTailCall)
+    OutlinerString = "Tail Call";
 
----------------
Can we have this be the first case to make the braces a bit nicer?

```
if (OF.FrameConstructionID == MachineOutlinerTailCall)
    OutlinerString = "Tail Call";
else if (OF.FrameConstructionID == MachineOutlinerThunk) {
  // .. stuff
}
```


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.ll:1
+; RUN: llc -verify-machineinstrs -enable-machine-outliner -mtriple=aarch64-linux-gnu < %s | FileCheck %s
+
----------------
Is it possible to write this as a MIR test? In general, MIR tests are better for late passes like the outliner because they let us know exactly which instructions the outliner will work on. With IR tests, we are fragile against other changes in the code generator.

To generate MIR, you can run:

`llc (all the flags you want) -stop-before=machine-outliner -simplify-mir`

Then you can create a testcase with a `RUN` line similar to below:

`llc (all the flags you want) -run-pass=machine-outliner ...`



================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.ll:31
+}
\ No newline at end of file

----------------
Missing newline?


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

https://reviews.llvm.org/D78062