[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

[Sean Fertile via Phabricator via cfe-commits]

sfertile added a comment.

A couple minor comments, but patch is almost ready otherwise.



================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4249
   uint64_t Size;
 
   // -msvr4-struct-return puts small aggregates in GPR3 and GPR4.
----------------
Pedantic nit: Can we emit a fatal error if `-msvr4-struct-return` is specified on AIX. Then we can add a lit test which checks for the error, which will change once we verify the options behavior on AIX and enable it. 

Note: FWIW both Zarko and I have verified the option is accepted and work as expected by passing in r3/r4, I'm just not sure if any padding is handled the same way on AIX as described below.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4483
+  // AIX does not have SPE registers so we don't set 111 - 113.
+  if (getABIInfo().getTarget().getTriple().isOSAIX()) {
+    AssignToArrayRange(Builder, Address, Four8, 109, 110);
----------------
Minor nit:

Can we restructure like this:

```
 // 109: vrsave
// 110: vscr
AssignToArrayRange(Builder, Address, Four8, 109, 110);

  // AIX does not have SPE registers so we don't set 111 - 113.
  if (getABIInfo().getTarget().getTriple().isOSAIX()) 
  return false;

// 111: spe_acc
  // 112: spefscr
  // 113: sfp
  AssignToArrayRange(Builder, Address, Four8, 111, 113);

return false;
```


================
Comment at: clang/test/CodeGen/ppc64-aix32-dwarf.c:1
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix=PPC64
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm  %s -o - | FileCheck %s --check-prefix=AIX32
----------------
Minor nit: How about naming the test file ppc-dwarf.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360