[PATCH] D74225: [AIX] Implement formal arguments passed in stack memory

[Chris Bowler via Phabricator via llvm-commits]

cebowleratibm added a comment.

Did a passthrough of the test changes with a number of comments.  I expect you wanted to make some test updates, nonetheless I thought I'd document what I'd like to see in the test coverage.  Will review code changes next.



================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1302
+; 32BIT:       liveins:
+; 32BIT-NEXT:  - { reg: '$r3', virtual-reg: '' }
+; 32BIT-NEXT:  - { reg: '$r4', virtual-reg: '' }
----------------
suggest using 32BIT-DAG for the liveins.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1311
+; 32BIT:       fixedStack:
+; 32BIT:       - { id: 0, type: default, offset: 92, size: 4
+; 32BIT:       - { id: 1, type: default, offset: 88, size: 4
----------------
I suggest 32BIT-DAG and reorder the memory locations in growing offset.  IMO it makes it easier to read and understand the test.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1327
+; 64BIT:       liveins:
+; 64BIT-NEXT:  - { reg: '$x3', virtual-reg: '' }
+; 64BIT-NEXT:  - { reg: '$x4', virtual-reg: '' }
----------------
DAG.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1336
+; 64BIT:       fixedStack:
+; 64BIT:       - { id: 0, type: default, offset: 168, size: 8 
+; 64BIT:       - { id: 1, type: default, offset: 160, size: 8 
----------------
DAG and reorder.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1346
+; 64BIT-NEXT:    bb.0.entry:
+; 64BIT-NEXT:      liveins: $x3, $x4, $x5, $x6, $x7, $x8, $x9, $x10
+
----------------
I would like to see the expected output for the assembly added.  We should validate that the instructions emitted locate the arguments in the correct memory location.  We should also strive for consistency with the other tests, which have expected assembly.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1374
+; CHECK:      liveins:
+; CHECK-NEXT: - { reg: '$f1', virtual-reg: '' }
+; CHECK-NEXT: - { reg: '$f2', virtual-reg: '' }
----------------
suggest DAG.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1387
+; CHECK-NEXT: - { reg: '$f13', virtual-reg: '' }
+
+; CHECK:      fixedStack:
----------------
remove blank line or add blank line in the earlier test for the sake of consistency.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1400
+; CHECK-NEXT:     liveins: $f1, $f2, $f3, $f4, $f5, $f6, $f7, $f8, $f9, $f10, $f11, $f12, $f13
+
+
----------------
I would also like to see the expected assembly here as well.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi.ll:1401
+
+
----------------
Additional test requests:
-1 call that passes 1, 2, 4, 8 byte ints and 4, 8 byte floats in memory (mixed with all sizes represented.)
-burn the first 8 GPR on float args, then confirm integer args passed in memory.
  ex: foo(double, double, double, double, char, short, int)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74225