[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
Jason Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
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
More information about the cfe-commits
mailing list