[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:1547
 
   // Otherwise, if the type contains an SSE vector type, the alignment is 16.
+  if (Align >= 16 && (isSIMDVectorType(getContext(), Ty) ||
----------------
Also update the comment?


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4365
+
+  Ty = useFirstFieldIfTransparentUnion(Ty);
+
----------------
As in doc says [[ https://www.ibm.com/support/knowledgecenter/SSGH2K_13.1.0/com.ibm.xlc131.aix.doc/language_ref/type_attr_transp_union.html | The transparent_union type attribute ]]:
 `float _Complex, double _Complex or vector types can be members of a transparent union, but they cannot be the first member. `  That means the first field still could be something like Integral Complex etc., which falls into the category `isAnyComplexType`.

So I guess `Ty = useFirstFieldIfTransparentUnion(Ty);` should be at the first line.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4374
+    CharUnits ABIAlign = getParamTypeAlignment(Ty);
+    CharUnits TyAlign = getContext().getTypeAlignInChars(Ty);
+
----------------
If we want to be consistent with other part of alignment implementation, `getContext().getTypeAlignInChars(Ty)` gives `ABIAlign`. I would suggest we name `ABIAlign` here to something like `CCAlign` or `ArgumentAlign`.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4684
     return false;
   case CodeGenOptions::SRCK_InRegs: // -msvr4-struct-return
     return true;
----------------
I noticed that in patch https://reviews.llvm.org/D76360, Zarko added a check to emit an error for using this option within cc1. But in your patch, this option only emit error when invoked by the driver. Does that mean we are pretty sure this option is doing what we want on AIX?


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

https://reviews.llvm.org/D79035