[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

Tue Apr 14 08:00:07 PDT 2020

jasonliu added inline comments.

================
Comment at: clang/test/CodeGen/ppc32-struct-return.c:53
+
+// AIX-SVR4: fatal error: error in backend: -msvr4-struct-return not supported on AIX
+ 
----------------
sfertile wrote:
> jasonliu wrote:
> > If certain front end option is not supported on certain target, I think it makes more sense to have a standard diagnostic in the driver component, instead of "crash" in the backend. 
> > i.e. What if we specify this option on a Windows machine? Maybe we should pursue the same behavior. 
> Its not that we don't intend to support the option on AIX, but that support for the option takes further verification on AIX.  Since there is a difference how  AIX justifies subregister sized values in its argument passing, we can't just assume that this option will pack the return values the same way on AIX and Linux. 
> 
> The focus of this patch was originally to enable and verify the proper IR generation of va-arg/va-lis/va-start, however the scope of the patch has kept growing. If there are codegen differences in packing the return register with the svr4-return option enabled it will grow this patch once again. The fatal error lets us limit the scope of this patch, while not silently emiting bad codegen if there is a difference in how gcc initializes the return  registers. If during verification we decide we don't want to support the option on AIX, then adopting  a standard diagnostic in the driver component becomes the appropriate behavior. 
It wasn't clear for me from the code that this is not a permanent thing.  In that case, does it make sense to leave a TODO here and say we need to re-evaluate this in the future to decide if we want to support it or not on AIX?
On another note, I'm not sure if this is really needed on AIX target though. But I guess we could discuss it down the road. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360