[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