[PATCH] D69578: [AIX] Add support for lowering int, float and double formal arguments.

[Zarko Todorovski via Phabricator via llvm-commits]

ZarkoCA marked 9 inline comments as done.
ZarkoCA added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6789
+                                                    bool IsPPC64) {
+  switch (SVT) {
+  default:
----------------
cebowleratibm wrote:
> I'd rather see this switch written:
>   assert((IsPPC64 || SVT != MVT::i64) && "i64 should have been split for 32-bit codegen.");
> 
>   switch (SVT) {
>   ...
>   case MVT::i1:
>   case MVT::i32:
>   case MVT::i64:    
>     return IsPPC64 ? &PPC::G8RCRegClass : &PPC::GPRCRegClass;
>   ...  
>   }
> It's easier for me to follow that all ints in PPC64 are G8RC and all ints in PPC32 are GPR and the complexity of the i64 in PPC32 assertion stays out of the switch.
Thanks, agreed this makes it much simpler and also moves the assert to the earliest possible positin. 


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6886
+  // Area that is at least reserved in the caller of this function.
+  unsigned MinReservedArea =
+      std::max(CCInfo.getNextStackOffset(), LinkageSize + MinParameterSaveArea);
----------------
sfertile wrote:
> Since you have allocated the linkage area and the parameter save area `CCInfo.getNextStackOffset()` will be at least `LinkageSize + MinParameterSaveArea`, so you can omit taking the max.
Thanks, fixed. 


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6889
+
+  // Set the size that is at least reserved in caller of this function.  Tail
+  // call optimized function's reserved stack space needs to be aligned so
----------------
jasonliu wrote:
> nit: Extra space before "Tail".
Removed, thanks.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6892
+  // that taking the difference between two stack areas will result in an
+  // aligned stack. reserved for caller
+  MinReservedArea =
----------------
jasonliu wrote:
> nit: Since we have "Set the size that is at least reserved in caller of this function" in the beginning of this paragraph, "reserved for caller" at the end could be removed?
Thanks for catching that. 


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

https://reviews.llvm.org/D69578