rjmccall added a reviewer: varungandhi-apple.
rjmccall added a comment.
CC'ing Varun Gandhi.
Is `musttail` actually supported generically on all LLVM backends, or does this
need a target restriction?
You should structure this code so it's easy to add exceptions for certain
calling conventions that can support tail calls with weaker restrictions
(principally, callee-pop conventions). Mostly that probably means checking the
calling convention first, or extracting the type restriction checks into a
different function that you can skip. For example, I believe x86's `fastcall`
convention can logically support any combination of prototypes as `musttail` as
long as the return types are vaguely compatible.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:5318-5319
+ EHCleanupScope *Cleanup = dyn_cast<EHCleanupScope>(&*it);
+ assert(Cleanup && Cleanup->getCleanup()->isCallLifetimeEnd() &&
+ "found unexpected cleanup generating musttail exit");
+ }
----------------
rsmith wrote:
> Given the potential for mismatch between the JumpDiagnostics checks and this
> one, especially as new more exotic kinds of cleanup are added, I wonder if we
> should use an `ErrorUnsupported` here instead of an `assert`.
>
> I strongly suspect we can still reach the problematic case here for a tail
> call in a statement expression. I don't think it's feasible to check for all
> the ways that an arbitrary expression context can have pending cleanups,
> which we'd need in order to produce precise `Sema` diagnostics for that, so
> either we handle that here or we blanket reject all `musttail` returns in
> statement expressions. I think either approach is probably acceptable.
Yes, I think ErrorUnsupported is a much better idea.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:631
+ // Caller is an Obj-C block decl: ^(void) { /* ... */ }
+ assert(dyn_cast<BlockDecl>(CurContext) && "unexpected decl context");
+ Diag(St->getBeginLoc(), diag::err_musttail_from_block_forbidden) << &MTA;
----------------
rsmith wrote:
> There are a couple of other contexts that can include a return statement: the
> caller could also be an `ObjCMethodDecl` (an Objective-C method) or a
> `CapturedDecl` (the body of a `#pragma omp` parallel region). I'd probably
> use a specific diagnostic ("cannot be used from a block" / "cannot be used
> from an Objective-C function") for the block and ObjCMethod case, and a
> nonsepcific-but-correct "cannot be used from this context" for anything else.
Blocks ought to be extremely straightforward to support. Just validate that
the tail call is to a block pointer and then compare the underlying function
types line up in the same way. You will need to be able to verify that there
isn't a non-trivial conversion on the return types, even if the return type
isn't known at this point in the function, but that's a problem in C++ as well
due to lambdas and `auto` deduced return types.
Also, you can use `isa<...>` for checks like this instead of `dyn_cast<...>`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99517/new/
https://reviews.llvm.org/D99517
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
