[PATCH] Implement the __builtin_call_with_static_chain GNU extension.
Richard Smith
richard at metafoo.co.uk
Mon Dec 8 17:34:27 PST 2014
================
Comment at: include/clang/Basic/TokenKinds.def:366
@@ -365,2 +365,3 @@
KEYWORD(__PRETTY_FUNCTION__ , KEYALL)
+KEYWORD(__builtin_call_with_static_chain, KEYALL)
----------------
pcc wrote:
> rsmith wrote:
> > Why is this modeled as a keyword rather than as a builtin function?
> I thought it would be necessary to introduce a new AST node because of the custom type checking (I got this impression from some of the newer additions to `Expr.h` which do indeed model builtin functions as AST nodes), but I somehow overlooked that builtin functions can have custom type checking. I agree that this should have been modeled as a builtin function. If you like, I'll see if I can revise this patch to do so.
Yes, please do make this a normal builtin function; that should remove a lot of the boilerplate (template instantiation and serialization/deserialization in particular).
================
Comment at: lib/CodeGen/CGCall.cpp:86
@@ -85,3 +85,3 @@
return arrangeLLVMFunctionInfo(FTNP->getReturnType().getUnqualifiedType(),
- false, None, FTNP->getExtInfo(),
+ false, false, None, FTNP->getExtInfo(),
RequiredArgs(0));
----------------
pcc wrote:
> rsmith wrote:
> > Please add an enum for this flag rather than having two mysterious bool flags in a row.
> I've added comments to make it clear what the various flags pertain to. Or I can do the enum thing if you prefer.
This function is far from being the worst offender, and a comment is sufficient to make the code readable. If you feel motivated to make another change, I would prefer an enum, but not strongly.
================
Comment at: lib/CodeGen/CGCall.cpp:375-378
@@ +374,6 @@
+ // FIXME: Kill copy.
+ SmallVector<CanQualType, 16> argTypes;
+ for (const auto &Arg : args)
+ argTypes.push_back(CGT.getContext().getCanonicalParamType(Arg.Ty));
+ return CGT.arrangeLLVMFunctionInfo(GetReturnType(fnType->getReturnType()),
+ false, chainCall, argTypes,
----------------
Please pick either `argTypes` and `args`, or `ArgTypes` and `Args`.
http://reviews.llvm.org/D6332
More information about the cfe-commits
mailing list