[PATCH] D78062: Adding Comment Annotations to Outlined Functions

[Jessica Paquette via Phabricator via llvm-commits]

paquette added a comment.

Added some comments on the testcase.

It'd be nice to just remove the IR entirely if possible, or at the very least simplify it down to a minimal form. Simple testcases are easier to fix +understand when they inevitably break someday. :)



================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:15-18
+  ; ModuleID = '<stdin>'
+  source_filename = "<stdin>"
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64-unknown-linux-gnu"
----------------
This can be deleted, I think.


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:21-25
+  entry:
+    call void @z(i32 1, i32 2, i32 3, i32 4)
+    %ptr = alloca i32, align 4
+    store i32 2, i32* %ptr, align 4
+    ret void
----------------
If you replace all instances of `bb.0.entry` with `bb.0`, you can delete the IR code here.

e.g. replace it with 

```
define void @a() { ret void }
```


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:30
+
+  define dso_local void @b(i32* nocapture readnone %p) {
+  entry:
----------------
This can probably just be `define void`?

In general, 99% of the IR isn't necessary in MIR tests. Only things that are directly referenced in the MIR are necessary (variables, function names, etc.)


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:32-35
+    call void @z(i32 1, i32 2, i32 3, i32 4)
+    %ptr = alloca i32, align 4
+    store i32 1, i32* %ptr, align 4
+    ret void
----------------
This IR code can be deleted as well, if you replace all instances of `bb.0.entry` in the MIR.


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:39
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**) #0
+
----------------
This can be deleted


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:41
+
+  attributes #0 = { nounwind }
+
----------------
This isn't used anywhere, so it can be deleted.


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:55-58
+stack:
+  - { id: 0, name: ptr, offset: -20, size: 4, alignment: 4, local-offset: -4 }
+  - { id: 1, type: spill-slot, offset: -8, size: 8, alignment: 8, callee-saved-register: '$x19' }
+  - { id: 2, type: spill-slot, offset: -16, size: 8, alignment: 8, callee-saved-register: '$lr' }
----------------
Can you delete this?


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:66
+    $sp = frame-setup SUBXri $sp, 32, 0
+    frame-setup STPXi killed $lr, killed $x19, $sp, 2 :: (store 8 into %stack.2), (store 8 into %stack.1)
+    frame-setup CFI_INSTRUCTION def_cfa_offset 32
----------------
You can replace `(store 8 into %stack.2)` with `(store 8)` to simplify the MIR a bit.


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:75
+    renamable $w19 = MOVZWi 2, 0
+    BL @z, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit killed $w3, implicit-def $sp
+    STRWui killed renamable $w19, $sp, 3 :: (store 4 into %ir.ptr)
----------------
Is it possible to get this testcase to work without a `BL`?

If it is, then I think you can delete all of the IR and leave only the MIR.


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:76
+    BL @z, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit killed $w3, implicit-def $sp
+    STRWui killed renamable $w19, $sp, 3 :: (store 4 into %ir.ptr)
+    $lr, $x19 = frame-destroy LDPXi $sp, 2 :: (load 8 from %stack.2), (load 8 from %stack.1)
----------------
Replace `(store 4 into %ir.ptr)` with `(store 4)`

(There are a few other places with code like this which can similarly be simplified.)


================
Comment at: llvm/test/CodeGen/AArch64/machine-outliner-function-annotate.mir:93-96
+stack:
+  - { id: 0, name: ptr, offset: -20, size: 4, alignment: 4, local-offset: -4 }
+  - { id: 1, type: spill-slot, offset: -8, size: 8, alignment: 8, callee-saved-register: '$x19' }
+  - { id: 2, type: spill-slot, offset: -16, size: 8, alignment: 8, callee-saved-register: '$lr' }
----------------
Can you delete this?


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

https://reviews.llvm.org/D78062