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

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 27 13:23:40 PDT 2021


dexonsmith added inline comments.


================
Comment at: llvm/lib/IR/Verifier.cpp:3430-3437
+    if (EnableSwiftTailCCMustTailCheck &&
+        CI.getCallingConv() == CallingConv::SwiftTail &&
+        CI.getCaller()->getCallingConv() == CallingConv::SwiftTail &&
+        isa_and_nonnull<ReturnInst>(CI.getNextNode())) {
+      Assert(false,
+             "tail call from swifttail->swiftail should be marked musttail",
+             &CI);
----------------
varungandhi-apple wrote:
> dexonsmith wrote:
> > I wonder if the verifier failure message would be more helpful if some of what's being asserted is in the assertion. Maybe, at least the `isa_and_nonnull` check; maybe also the calling convention checks, but I'm not sure.
> So it would be like
> 
> ```
> Assert(!(CI.getCallingConv() == CallingConv::SwiftTail || 
>          CI.getCaller()->getCallingConv() == CallingConv::SwiftTail ||
>           isa_and_nonnull<ReturnInst>(CI.getNextNode()),
> ```
> 
> That's okay with me. The reason I wrote it this way is that since the conditions are kinda' long, I found the textual description to be more concise and direct.
Yeah, that seems fine to me, or even just `Assert(!isa_and_nonnull<ReturnInst>(...))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103275



More information about the llvm-commits mailing list