[PATCH] D103275: Update musttail verification to check all swiftasync->swiftasync tail calls.

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D103275#2785642 <https://reviews.llvm.org/D103275#2785642>, @varungandhi-apple wrote:

> I used `Assert` for two reasons: (1) the surrounding code uses `Assert`, so it seemed like the right thing to use and (2) in case there's a failure, it will display the instruction missing the `musttail`.

Absolutely, no debate here!

> I'm using NDEBUG because we would like this check to not be turned on for release builds.

As @aschwaighofer pointed out, release builds can just avoid setting the enable flag.

In D103275#2785656 <https://reviews.llvm.org/D103275#2785656>, @varungandhi-apple wrote:

>> This doesn't look like a particularly expensive verifier check (although maybe I've missed something). What's the motivation for hiding it behind `EnableSwiftTailCCMustTailCheck`?
>
> Having a flag makes it easier to turn off from Swift if we need to turn it off for some reason. Missing the `musttail` is technically not a fatal error, but it increases the risk of stack overflow if Swift doesn't generate the right IR.

I guess I'm wondering what the trajectory is - if the goal is for this to eventually be an IR invariant for the calling convention, I think a verifier check makes sense; once the codegen bugs are sorted out, and the bitcode upgrade is done, the flag can be switched to on-by-default and eventually removed. But if this will never be an invariant of the IR, I'm not sure the LLVM IR verifier is really the right place for the check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103275/new/

https://reviews.llvm.org/D103275