jasonliu added inline comments.
================ Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7076 + static const MCPhysReg GPR_64[] = {PPC::X3, PPC::X4, PPC::X5, PPC::X6, + PPC::X7, PPC::X8, PPC::X9, PPC::X10}; + unsigned const NumGPArgRegs = array_lengthof(GPR_32); ---------------- This two array gets used to in CC_AIX and here. It would be great if we only have one set of copy. ================ Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7077 + PPC::X7, PPC::X8, PPC::X9, PPC::X10}; + unsigned const NumGPArgRegs = array_lengthof(GPR_32); + ---------------- Could we add an assert to make sure "array_lengthof(GPR_32) == array_lengthof(GPR_64)" nit: unsigned const -> const unsigned ================ Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7087 + if (!IsPPC64) + VReg = MF.addLiveIn(GPR_32[GPRIndex], &PPC::GPRCRegClass); + else ---------------- Could we do ``` const unsigned VReg = IsPPC64 ? MF.addLiveIn(GPR_64[GPRIndex], &PPC::G8RCRegClass) : MF.addLiveIn(GPR_32[GPRIndex], &PPC::GPRCRegClass); ``` ================ Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi-va_args-32.ll:27 +; Function Attrs: nounwind +define i32 @va_arg1(i32 %a, ...) local_unnamed_addr { +entry: ---------------- These tests used for loops to call va_arg mutliple times. Maybe it's just me, but I found the test cases are much harder to read. I would suggest maybe just unroll the for loop and generate as much as the va_arg call that you need? For the cases below, I suppose you would want to call va_arg 9 times, and 2 times respectfully. Maybe it's not that bad. If the tests gets too long, you could consider split the tests into separate files. ================ Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi-va_args-32.ll:33 + call void @llvm.va_start(i8* nonnull %0) + %cmp7 = icmp sgt i32 %a, 0 + br i1 %cmp7, label %for.body.preheader, label %for.end ---------------- a is 1, so we would only call va_arg once, despite we have 9 vaarg argument. Is this the intention of the test? ================ Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi-va_args-32.ll:162 + %add7 = add nsw i32 %add6, %eight + %cmp15 = icmp sgt i32 %eight, 0 + br i1 %cmp15, label %for.body.preheader, label %for.end ---------------- We only have 2 va_arg arguments here, but we called va_arg 8 times? I think that's undefined behavior. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76130/new/ https://reviews.llvm.org/D76130 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits