[PATCH] Add 'musttail' marker to call instructions
Reid Kleckner
rnk at google.com
Thu Apr 24 11:53:13 PDT 2014
On Tue, Apr 22, 2014 at 4:35 PM, Nick Lewycky <nlewycky at google.com> wrote:
>
> +static bool isTypeCongruent(Type *L, Type *R) {
>
> Function-level comment? "Congruency" of a type isn't a previously defined
> concept, you need to explain that this is testing the musttail
> compatibility property.
>
Yep, done.
> + FunctionType *CalleeTy = GetFnTy(CI.getCalledValue());
>
> What if the callee is musttail inline asm? Is this reachable for the
> invalid IR where the callee is not a function?
>
Turns out inline asm blobs have function types, so this "worked". I'm
going to reject musttail calls of inline asm, though, since we won't lower
it to anything useful. I don't think this is worth documenting in LangRef.
Let me know if you disagree.
> + for (int I = 0, E = CallerTy->getNumParams(); I != E; ++I) {
> + Assert1(
> + isTypeCongruent(CallerTy->getParamType(I),
> CalleeTy->getParamType(I)),
> + "cannot guarantee tail call due to mismatched parameter types",
> &CI);
> + }
>
> Optional: include which n-th argument is mismatched?
>
I approximated that by printing the operand of the call. I don't see a way
to print the index. Ideally we'd caret the argument, but we might not have
a .ll source file anyway.
As an aside, I'm don't like the wording "cannot guarantee tail call". The
> problem is that your IR doesn't match what LangRef requires, abstracted
> away from that other stuff. I don't have better wording, and it's *clear*
> as written.
>
I like the existing wording because it explains *why* we have this rule.
> + Assert1(!Ret->getReturnValue() || Ret->getReturnValue() == RetVal,
> + "ret must return the result from a preceding musttail call",
> Ret);
>
> I'd switch the object and subject, the rule isn't that a ret must return
> result from a musttail, it's that musttail's result must be returned.
>
Done.
> Overall, this patch looks really good!
>
Thanks, I'll commit and send you some inliner changes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140424/cce6ef68/attachment.html>
More information about the llvm-commits
mailing list