[PATCH] D103593: [Coroutine] Sink lifetime markers after switch of suspend blocks to avoid disturbing must tail calls

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D103593#2887602 <https://reviews.llvm.org/D103593#2887602>, @lxfind wrote:

> I am OK with this. You may want to move those part of the checking code to a separate function for readability. (since this check is independent from lifetime sinking, we could also consider doing the check earlier for all corosuspends too.

In my opinion, I prefer to remove these checking codes in this diff or add the checking codes in another patch.
Here are reasons:

- This check is independent from this patch.
- It should be harmless.

Let me explain the second point more. First the algorithm of sinkLifetimeStartMarkers is to check if the candidate insert point is safe to insert lifetime markers.  Second, this patch would add the successors of the SuspendBlock as candidate insert points instead of the SuspendBlock itself. So I think this patch only reduce the range that the lifetimes could be inserted to affect. It doesn't touch the algorithm that judge the safeness. So I mean, as long as the algorithm is right, this patch should do nothing harmful. This algorithm looks good to me. And in case that the algorithm has bugs, we should fix the bug in the algorithm instead.

So I prefer to remove the checking codes simply.


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

https://reviews.llvm.org/D103593