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

Tue Apr 14 08:00:08 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
+ 
----------------
jasonliu wrote:
> 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. 
Just to avoid ambiguity, I meant I'm not sure if we really need this *option* on AIX.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360