jasonliu added inline comments.
================
Comment at: clang/lib/Basic/Targets/PPC.h:373
+ // This is the ELF definition, and is overridden by the Darwin and AIX
+ // sub-target
return TargetInfo::PowerABIBuiltinVaList;
----------------
nit: add '.' to the comment while we are at it.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7067
+
+ const static MCPhysReg GPR_32[] = {PPC::R3, PPC::R4, PPC::R5, PPC::R6,
+ PPC::R7, PPC::R8, PPC::R9, PPC::R10};
----------------
The style in this file is "static const" instead of "const static".
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7029
continue;
+ } else if (VA.isMemLoc()) {
+ const unsigned LocSize = LocVT.getStoreSize();
----------------
Would it be better if we remove the "else if" and make it
if (VA.isMemloc()){
...
continue;
}
Since this matches the last if statement, and also the style in LowerCall_AIX.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7038
+ CurArgOffset += LocSize - ValSize;
+ MachineFrameInfo &MFI = MF.getFrameInfo();
+ // Potential tail calls could cause overwriting of argument stack slots.
----------------
We could remove this definition if we have one defined outside.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7077
+
+ FuncInfo->setVarArgsNumGPR(
+ CCInfo.getFirstUnallocated(IsPPC64 ? GPR_64 : GPR_32));
----------------
This only gets used by 32bit-SVR4 target, I don't think we need to set it.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7082
+ SDValue FIN = DAG.getFrameIndex(FuncInfo->getVarArgsFrameIndex(), PtrVT);
+ // The fixed integer arguments of a variadic function are stored to the
+ // VarArgsFrameIndex on the stack so that they may be loaded by
----------------
start a new line before the comment.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7088
+ GPRIndex < NumGPArgRegs; ++GPRIndex) {
+ unsigned VReg = MF.getRegInfo().getLiveInVirtReg(
+ IsPPC64 ? GPR_64[GPRIndex] : GPR_32[GPRIndex]);
----------------
Just curious, in what scenario would we already have the GPR stored in a
virtual register before we store that GPR on that stack?
================
Comment at: llvm/test/CodeGen/PowerPC/aix-cc-abi-va_args-32.ll:9
+; Function Attrs: argmemonly nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #1
+
----------------
I'm not seeing any attribute here, so I'm assuming every "#number" and comments
about "Function Attrs" in the test could be removed.
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
