rjmccall added a comment.
This is a kindof weird extension conceptually. "Emulate the C ABI that
Microsoft would use on the current architecture"? I guess we're lucky that
they never support two different C ABIs on the same ISA. But okay, whatever,
it's a thing.
A couple comments:
================
Comment at: lib/CodeGen/CGCall.cpp:3598
@@ -3599,1 +3597,3 @@
+Address CodeGenFunction::EmitVAArg(Address VAListAddr, QualType Ty, bool IsMS)
{
+ return CGM.getTypes().getABIInfo().EmitVAArg(*this, VAListAddr, Ty, IsMS);
}
----------------
rnk wrote:
> I think keeping the va_arg logic in TargetInfo.cpp is good, but we don't need
> to thread IsMS through every EmitVAArg override. Instead, we can do something
> like this here:
> if (IsMS)
> return CGM.getTypes().getABIInfo().EmitMSVAArg(*this, VAListAddr, Ty);
> return CGM.getTypes().getABIInfo().EmitVAArg(*this, VAListAddr, Ty);
I agree, especially because TargetInfo.cpp is partially maintained by backend
authors; let's not force them all to know about this weird extension.
================
Comment at: lib/CodeGen/CGExprAgg.cpp:966
@@ -965,1 +965,3 @@
+ : CGF.EmitVAListRef(VE->getSubExpr());
+ Address ArgPtr = CGF.EmitVAArg(ArgValue, VE->getType(),
VE->isMicrosoftABI());
----------------
We have exactly this same pattern of code in three different places, and it's
kindof begging for somebody to add another emitter that's missing the MS ABI
site. Maybe instead of having EmitVAListRef as a separate function, we should
just have this:
Address CGF::EmitVAArgExpr(VAArgExpr *E)
and it can check the ABI and do the right combination of things.
http://reviews.llvm.org/D1623
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
