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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to