sfertile added inline comments.

================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements
----------------
jasonliu wrote:
> So for AIX we are taking PowerPC32ABIInfo right now. And we know EmitVAArg of 
> this target does the right thing on AIX after the change. 
> But for other functions, for example, getParamTypeAlignment, 
> initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?
> If we are not sure, is there anything we want to do (etc, put a comment on 
> where it gets used or at the function definition)? Or are we fine to just 
> leave it as it is and have a TODO in our head?
Looking at the values in `initDwarfEHRegSizeTable` it has the right sizes for 
all the registers. Even though the OS is different the underlying hardware is 
the same. I'm not sure it's something that makes sense to support for AIX 
though, in which case I think its valid to return `true` to indicate its not 
supported. 

`getParamTypeAlignment` is used only in the context of the alignment for vaarg, 
we should make sure its correct for AIX since supporting vaarg is the scope of 
this patch.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4232
+Address PowerPC32ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAList,
                                       QualType Ty) const {
+  // TODO: Add AIX ABI Info. Currently, we are relying on PowerPC32ABIInfo to
----------------
Make sure to address the formatting here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



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

Reply via email to