[PATCH] D69578: [AIX] Add support for lowering int, float and double formal arguments.

[Sean Fertile via Phabricator via llvm-commits]

sfertile added a comment.

Patch is very close. I've added a few inline comments on the test.



================
Comment at: llvm/test/CodeGen/PowerPC/aix_cc_abi.ll:47
+; 32BIT:       liveins:
+; 32BIT:  - { reg: '$r3', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r4', virtual-reg: '' }
----------------
very minor nit: indentation should match the following lines.


================
Comment at: llvm/test/CodeGen/PowerPC/aix_cc_abi.ll:122
+
+define void @call_test_ints() {
+entry:
----------------
Minor nit: This test and `@test_ints_64bit` overlap enough we just need 1. I would suggest getting rid of this one, and renaming '@test_ints_64bit` to just `@test_ints`.


================
Comment at: llvm/test/CodeGen/PowerPC/aix_cc_abi.ll:260
+
+define void @test_i1(i1 %a) {
+entry:
----------------
Move this test to beside the zero extended  i1 case, and add checks for the body that show we end up with a clear instruction before the strore.

Minor nit: dead store. We can store to a global instead.

```
@global_i1 = global i8 0, align 1

define  void @test_i1(i1 %b)  {
entry:
  %frombool = zext i1 %b to i8
  store i8 %frombool, i8* @global_i1, align 1, !tbaa !2
  ret void
}
```



================
Comment at: llvm/test/CodeGen/PowerPC/aix_cc_abi.ll:559
+
+; 32BIT:      renamable $r3 = LWZtoc @f1, $r2 :: (load 4 from got)
+; 32BIT-NEXT: renamable $r4 = LWZtoc @d1, $r2 :: (load 4 from got)
----------------
r3 and r4 used in the 'LWZtoc` instructions should be variables. They are renamable scratch registers in these uses. The same variables should be used in place of hardcoded gprs in the `LFS` and `LFD` instructions. 

The `LI` is right in useing the hardcoded r4 and r7 registers since we are setting up ABI specified registers for the call with those instructions.

And the same holds for the equivalent 64-bit test.


================
Comment at: llvm/test/CodeGen/PowerPC/aix_cc_abi.ll:579
+
+define i32 @test_mix(float %f, i32 signext %i, double %d, i8 signext %c) {
+entry:
----------------
This is a very good test:  Documents how a float argument only shadows 1 gpr for 32-bit codegen despite being passed as a double in fpr1.


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

https://reviews.llvm.org/D69578