jasonliu added inline comments.

================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7091
+
+    static const MCPhysReg Gpr_32[] = {PPC::R3, PPC::R4, PPC::R5, PPC::R6,
+                                       PPC::R7, PPC::R8, PPC::R9, PPC::R10};
----------------
I'd prefer us to keep the original naming convention(GPR_32) for these two 
arrays, so that it's easy for us to common them up later in an NFC patch. 


================
Comment at: llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll:17
+    call void @llvm.va_start(i8* nonnull %0)
+    call void @llvm.va_copy(i8* nonnull %0, i8* nonnull %0)
+    %argp.cur = load i8*, i8** %arg, align 4
----------------
I think it would be more clear if we actually have another va_list to copy 
instead of just va_copying itself. The same applies to all the places we call 
va_copy. 
Also it's not very clear to me which part in the check result actually tells me 
about the effect of the va_copy though.


================
Comment at: llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll:97
+  
+  declare void @llvm.stackprotector(i8*, i8**)
+  
----------------
llvm.stackprotector could be removed?


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

Reply via email to