[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2022-01-09 Thread Paweł Bylica via Phabricator via cfe-commits
chfast added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:5320 + if (!(Cleanup && Cleanup->getCleanup()->isRedundantBeforeReturn())) +CGM.ErrorUnsupported(MustTailCall, "tail call skipping over cleanups"); +} I reported a related

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-08-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. It's not generically true that "anything can be tail-called if it's `noreturn`". For one, `noreturn` doesn't imply that the function doesn't exit by e.g. throwing or calling `longjmp`. For another, the most important user expectation of tail calls is that a long

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-08-09 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment. Please also see https://bugs.llvm.org/show_bug.cgi?id=51416 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 ___ cfe-commits mailing

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-07-07 Thread David Chisnall via Phabricator via cfe-commits
theraven added a comment. Here's a minimal test: void tail(int, float); __attribute__((always_inline)) void caller(float x) { [[clang::musttail]] return tail(42, x); } void outer(int x, float y) { return caller(y); } This raises this error:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-07-06 Thread Josh Haberman via Phabricator via cfe-commits
haberman added a comment. @theraven: Can you post a minimal repro of your case? I don't follow your distinction between "caller" and "enclosing function." Regarding `noreturn` and `always_inline`: maybe the rules for `musttail` could be relaxed in cases like the one you mention, but it would

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-06-29 Thread David Chisnall via Phabricator via cfe-commits
theraven added a comment. The error message here is very confusing: /home/theraven/snmalloc2/src/mem/../ds/../aal/../ds/defines.h:122:27: error: cannot perform a tail call to function 'error' because its signature is incompatible with the calling function [[clang::musttail]] return

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-20 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment. That is a great feature, thank you. Compiling state machines and scheme programs to C is now much prettier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D99517#2693418 , @thakis wrote: > Looks like this breaks tests on mac/arm: > http://45.33.8.238/macm1/7552/step_7.txt Should be fixed by rGf7c9de0de5804498085af973dc6bfc934a18f000

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-15 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Looks like this breaks tests on mac/arm: http://45.33.8.238/macm1/7552/step_7.txt Please take a look and revert for now if it takes a while to fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG834467590842: Implemented [[clang::musttail]] attribute for guaranteed tail calls. (authored by haberman, committed by rsmith). Repository: rG

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-14 Thread Josh Haberman via Phabricator via cfe-commits
haberman added a comment. Ok I think this is ready to land. There are a few FIXME comments, I will follow up with some small changes to address them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-14 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 337597. haberman added a comment. - Fixed typo in comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files: clang/docs/ReleaseNotes.rst

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-14 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Thanks, cool :) 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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-14 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 337592. haberman added a comment. - Fixed several cases in CodeGen test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files: clang/docs/ReleaseNotes.rst

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-14 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 337590. haberman added a comment. - Fixed release note escaping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files: clang/docs/ReleaseNotes.rst

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-14 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 337589. haberman added a comment. - Added release note for [[clang::musttail]]. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files: clang/docs/ReleaseNotes.rst

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-14 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 337581. haberman added a comment. - More diagnostic wordsmithing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files: clang/include/clang/AST/IgnoreExpr.h

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-14 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 337576. haberman marked 6 inline comments as done. haberman added a comment. - Word-smithed diagnostics and addressed other review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:711 +if (CalleeQualType.isNull()) { + // The function callee is invalid and already triggered an error. + // Avoid compounding errors. haberman wrote: > rsmith wrote: > > Even in

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-13 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:711 +if (CalleeQualType.isNull()) { + // The function callee is invalid and already triggered an error. + // Avoid compounding errors. rsmith wrote: > Even in invalid code we

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-13 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Nice new feature! Please also update Release Notes for clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 ___ cfe-commits mailing

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Functionally this looks good to me. I've suggested some minor cleanups and I understand you're doing some wordsmithing on the diagnostics; I think once those are complete this will be ready to land. Thank you! Comment at:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-13 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:605-609 + while (!isa(Ex)) { +auto *PE = cast(Ex); +Ex = IgnoreImplicitAsWritten(PE->getSubExpr()); +PE->setSubExpr(Ex); + } rsmith wrote: > This loop is problematic: it's

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-13 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 337252. haberman marked 14 inline comments as done. haberman added a comment. - Addressed more review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:603-604 + ReturnStmt *R = cast(St); + R->setRetValue(IgnoreImplicitAsWritten(R->getRetValue())); + Expr *Ex = R->getRetValue(); + while (!isa(Ex)) { I think this would be clearer, assuming

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-12 Thread Josh Haberman via Phabricator via cfe-commits
haberman marked an inline comment as done. haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:697-699 + } else if (VD && dyn_cast(VD->getType())) { +// Call is: obj->*method_ptr or obj.*method_ptr +const auto *MPT = VD->getType()->castAs();

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-12 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336894. haberman added a comment. - Switch to isa<> for type check. - Merge branch 'main' into musttail Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:697-699 + } else if (VD && dyn_cast(VD->getType())) { +// Call is: obj->*method_ptr or obj.*method_ptr +const auto *MPT = VD->getType()->castAs(); I'm not certain if I should

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-09 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:636-637 + + if (!CE->getCalleeDecl()) { +assert(hasUncompilableErrorOccurred() && "expected previous error"); +return false; aaron.ballman wrote: > This worries me slightly -- not

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-09 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336511. haberman marked 9 inline comments as done. haberman added a comment. - Simplified some casts and type declarations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Mostly just nits from me, but the attribute portions look good to me. Comment at: clang/include/clang/AST/IgnoreExpr.h:127 + if (CCE && CCE->isElidable() && !isa(CCE)) { +auto NumArgs = CCE->getNumArgs(); +if ((NumArgs == 1 ||

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336316. haberman added a comment. - Rename and refine IgnoreElidableImplicitConstructorSingleStep(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2828-2829 + "%0 attribute requires that the return value, all parameters, and any " + "temporaries created by the expression are trivially destructible and " + "do not require ARC">;

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336310. haberman marked 3 inline comments as done. haberman added a comment. - Refined the implicit constructor skipping code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2828-2829 + "%0 attribute requires that the return value, all parameters, and any " + "temporaries created by the expression are trivially destructible and " + "do not require ARC">;

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman marked 2 inline comments as done. haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + }

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336203. haberman added a comment. - Formatted files with clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files: clang/include/clang/AST/Expr.h

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman added a comment. In D99517#2667025 , @rjmccall wrote: > 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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336153. haberman added a comment. - Moved calling convention check to happen as early as possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +}

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336141. haberman added a comment. - Factored duplicated code into a method on MustTailAttr. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336130. haberman added a comment. - Added FIXME for attribute refactoring. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files: clang/include/clang/AST/Expr.h

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-07 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:458 +same number of arguments as the caller. The types of the return value and all +arguments must be similar, including the implicit "this" argument, if any. +Any variables in scope, including all

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-07 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 336004. haberman marked 19 inline comments as done. haberman added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. - Returned validation to ActOnAttributedStmt() so it works with templates. - Merge branch 'main' into

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-05 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +}

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-05 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:588 + + const CallExpr *CE = dyn_cast(Ex->IgnoreUnlessSpelledInSource()); + haberman wrote: > rsmith wrote: > > `IgnoreUnlessSpelledInSource` is a syntactic check that's only really > >

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-04 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +}

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:561-568 + for (const auto *A : Attrs) { +if (A->getKind() == attr::MustTail) { + if (!checkMustTailAttr(SubStmt, *A)) { +return SubStmt; + } + setFunctionHasMustTail(); +}

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 335106. haberman added a comment. - Added missing S.setFunctionHasMustTail(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Josh Haberman via Phabricator via cfe-commits
haberman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:588 + + const CallExpr *CE = dyn_cast(Ex->IgnoreUnlessSpelledInSource()); + rsmith wrote: > `IgnoreUnlessSpelledInSource` is a syntactic check that's only really > intended for tooling use

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 335103. haberman marked 3 inline comments as done. haberman added a comment. - Addressed comments and tried moving check to SemaStmtAttr.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:458 +same number of arguments as the caller. The types of the return value and all +arguments must be similar, including the implicit "this" argument, if any. +Any variables in scope,

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D99517#2667088 , @rsmith wrote: > In D99517#2667025 , @rjmccall wrote: > >> You should structure this code so it's easy to add exceptions for certain >> calling conventions that can

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D99517#2667025 , @rjmccall wrote: > 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).

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-02 Thread John McCall via Phabricator via cfe-commits
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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:5315-5317 +// TODO(haberman): insert checks/assertions to verify that this early exit +// is safe. We tried to verify this in Sema but we should double-check +// here. rsmith

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Thanks, I think this is looking really good. @rjmccall, no explicit need to review; I just wanted to make sure you'd seen this and had a chance to express any concerns before we go ahead. Comment at: clang/include/clang/Basic/AttrDocs.td:452

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-02 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 334985. haberman added a comment. - Fixed unit test by running `opt` in a separate invocation. - Formatting fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-01 Thread Josh Haberman via Phabricator via cfe-commits
haberman added a comment. I added tests for all the cases you mentioned. PTAL. Comment at: clang/lib/CodeGen/CGCall.cpp:5315-5317 +// TODO(haberman): insert checks/assertions to verify that this early exit +// is safe. We tried to verify this in Sema but we should

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-01 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 334890. haberman marked 6 inline comments as done. haberman added a comment. - Addressed more comments for musttail. - Reject constructors and destructors from musttail. - Fixed a few bugs and fixed the tests. - Added Obj-C test. Repository: rG LLVM

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-04-01 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. I've tried to think of some more exotic corner cases. I'd like to see a test for this: void f(); struct A { ~A() { [[clang::musttail]] return f(); } }; ... even though we reject that for a reason unrelated to musttail. We should also reject this: struct B {};

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-03-31 Thread Josh Haberman via Phabricator via cfe-commits
haberman added a comment. I addressed nearly all of the comments. I have just a few more test cases to add: Obj-C blocks and ARC. I left one comment open re: a "regular" function. I'll dig into that more when I am adding the last few test cases. Comment at:

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-03-31 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 334556. haberman marked 37 inline comments as done. haberman added a comment. - Expanded and refined the semantic checks for musttail, per CR feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-03-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2829-2830 +def err_musttail_needs_call : Error< + "%0 attribute requires that the return value is a function call, which must " + "not create or destroy any temporaries.">; +def

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-03-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1356 +def MustTail : StmtAttr { + let Spellings = [Clang<"musttail">]; + let Documentation = [MustTailDocs]; You should add a `Subjects` list here. Comment

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-03-29 Thread Josh Haberman via Phabricator via cfe-commits
haberman updated this revision to Diff 333914. haberman added a comment. - Updated formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99517/new/ https://reviews.llvm.org/D99517 Files: clang/include/clang/Basic/Attr.td

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

2021-03-29 Thread Josh Haberman via Phabricator via cfe-commits
haberman created this revision. haberman added reviewers: rsmith, aaron.ballman. haberman requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is a Clang-only change and depends on the existing "musttail" support already implemented in