[PATCH] D39386: [Power9] Allow gpr callee saved spills in prologue to vector registers rather than stack

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added a comment.

Thank you! A few comments on the MIR test though. Thanks for putting up the test.



================
Comment at: llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir:3
+--- |
+  define signext i32 @test1BB(i32 signext %a, i32 signext %b) {
+  entry:
----------------
You should be able to remove the LLVM IR and keep the MIR only.

Usually the LLVM IR is kept around if there are any references to IR values from MI.


================
Comment at: llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir:49
+    $v20 = IMPLICIT_DEF
+    renamable $r3 = ADD4 renamable $r4, renamable $r3, implicit killed $x3, implicit killed $x4
+    renamable $x3 = EXTSW_32_64 killed renamable $r3
----------------
Is this needed?
I think you can either use `BLR8 implicit undef $lr8, implicit undef $rm` and get rid of the `$r3` and `$x3` defs, or:

```
$lr8 = IMPLICIT_DEF
$rm = IMPLICIT_DEF
BLR8 implicit $lr8, implicit $rm
```


================
Comment at: llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir:59
+liveins:
+  - { reg: '$x3' }
+  - { reg: '$x4' }
----------------
Same as the previous function.


================
Comment at: llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir:71
+
+    renamable $cr0 = CMPW renamable $r3, renamable $r4
+    BCC 4, killed renamable $cr0, %bb.2
----------------
`$cr0 = IMPLICIT_DEF`


================
Comment at: llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir:83
+    renamable $r3 = ISEL undef renamable $r3, killed renamable $r3, renamable $cr0lt, implicit killed $cr0
+    B %bb.3
+
----------------
I would use `IMPLICIT_DEF`s here as well to express the intent of the test better.


================
Comment at: llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir:99
+
+# CHECK: name:            test1BB
+# CHECK: body:             |
----------------
`CHECK-LABEL: name: `


================
Comment at: llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir:102
+# CHECK: $f1 = MTVSRD killed $x14
+# CHECK: $f2 = MTVSRD killed $x15
+# CHECK: $f3 = MTVSRD killed $x16
----------------
If these are expected to be generated next to each other, I would use `CHECK-NEXT:` here.


================
Comment at: llvm/test/CodeGen/MIR/PowerPC/prolog_vec_spills.mir:108
+
+# CHECK: name:            test2BB
+# CHECK: body:             |
----------------
`CHECK-LABEL: name: `


https://reviews.llvm.org/D39386