[PATCH] D71013: [AIX] Allow vararg calls when all arguments reside in registers.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

Some minor nits that can be fixed on the commit, but otherwise LGTM.



================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:637
+; 32BIT-NEXT: renamable $f1 = LFS 0, killed renamable $r[[REG]] :: (dereferenceable load 4 from @f1)
+; 32BIT-NEXT: renamable $r[[REG:[0-9]+]] = LWZtoc @d1, $r2 :: (load 4 from got)
+; 32BIT-NEXT: STFD renamable $f1, 0, %stack.[[SLOT1:[0-9]+]] :: (store 8 into %stack.[[SLOT1]])
----------------
Reusing `REG` is okay, but the 64-bit test does no do so. I think being consistent is better.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:706
+; 32BIT-NEXT: renamable $f1 = LFS 0, killed renamable $r[[REG]] :: (dereferenceable load 4 from @f1)
+; 32BIT-NEXT: renamable $r[[REG:[0-9]+]] = LWZtoc @d1, $r2 :: (load 4 from got)
+; 32BIT-NEXT: STFD renamable $f1, 0, %stack.[[SLOT1:[0-9]+]] :: (store 8 into %stack.[[SLOT1]])
----------------
Same comment about reuse.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:777
+; 32BIT-NEXT: renamable $f1 = LFS 0, killed renamable $r[[REG]] :: (dereferenceable load 4 from @f1)
+; 32BIT-NEXT: renamable $r[[REG:[0-9]+]] = LWZtoc @d1, $r2 :: (load 4 from got)
+; 32BIT-NEXT: STFD renamable $f1, 0, %stack.[[SLOT1:[0-9]+]] :: (store 8 into %stack.[[SLOT1]])
----------------
Same comment about reuse.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:846
+
+; 32BIT:      renamable $r3 = LWZtoc @f1, $r2 :: (load 4 from got)
+; 32BIT-NEXT: renamable $f1 = LFS 0, killed renamable $r3 :: (dereferenceable load 4 from @f1)
----------------
The scratch register should not be hardcoded.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:855
+
+; 64BIT:      renamable $x3 = LDtoc @f1, $x2 :: (load 8 from got)
+; 64BIT-NEXT: renamable $f1 = LFS 0, killed renamable $x3 :: (dereferenceable load 4 from @f1)
----------------
Same comment about hardcoding.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:864
+
+; ASM32PWR4:      stwu 1, -64(1)
+; ASM32PWR4-NEXT: lwz [[REG:[0-9]+]], LC1(2)
----------------
The 32-bit tests checks are grouped together for the other tests but not here.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-altivec.ll:1
+; RUN: not llc < %s -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr8 2>&1 | FileCheck %s
+
----------------
I don't think it hurts to have the 32-bit RUN line here now as well.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-altivec.ll:6
+; should compile clean and fail in order to alert the author to validate the
+; instructions emitted to initialize the GPR for the double vararg.
+; The mfvsrwz and mfvsrd instructions should be used to initialize the GPR for
----------------
I think that this should be testing the passing of floats too when the time comes.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-altivec.ll:16
+  %conv = fpext float %0 to double
+  call void (i32, ...) @test_vararg(i32 42, double %conv)
+  ret void
----------------
I don't think it hurts to have a float parameter here now as well.


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

https://reviews.llvm.org/D71013