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

[Zarko Todorovski via Phabricator via llvm-commits]

ZarkoCA marked 3 inline comments as done.
ZarkoCA added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix_cc_abi.ll:122
+
+define void @call_test_ints() {
+entry:
----------------
sfertile wrote:
> 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`.
Thanks, removed and renamed. 


================
Comment at: llvm/test/CodeGen/PowerPC/aix_cc_abi.ll:260
+
+define void @test_i1(i1 %a) {
+entry:
----------------
sfertile wrote:
> 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
> }
> ```
> 
Moved and updated the function to store a global. 


================
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)
----------------
sfertile wrote:
> 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.
Fixed, that's a good point, I'm using variables for these now. 


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

https://reviews.llvm.org/D69578