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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
