[llvm] [Coroutines] Drop dead instructions more aggressively in addMustTailToCoroResumes() (PR #85271)

[via llvm-commits]

================
@@ -1203,10 +1203,6 @@ static bool simplifyTerminatorLeadingToRet(Instruction *InitialInst) {
       if (isa<BitCastInst>(I) || I->isDebugOrPseudoInst() ||
           I->isLifetimeStartOrEnd())
         I = I->getNextNode();
-      else if (isInstructionTriviallyDead(I))
-        // Duing we are in the middle of the transformation, we need to erase
-        // the dead instruction manually.
-        I = &*I->eraseFromParent();
----------------
zmodem wrote:

> understanding this a bit better would be good to see if we're doing extra unnecessary work, or if those instructions should be cleaned up earlier in the pass

+1 I'd like to understand this better too :)

In the original code I was debugging, the presplit coroutine has a return value:

```
169:                                              ; preds = %166, %163, %135, %68, %48
  %170 = getelementptr inbounds i8, ptr %17, i64 -16
  %171 = ptrtoint ptr %170 to i64
  %172 = call i1 @llvm.coro.end(ptr null, i1 false) #19                                                                                 
  ret i64 %171                     
}
```

corosplit clones the function into various versions, and the modifications of those clones are what causes dead code, makes some values constant, etc.

In my case, `CoroCloner::create()` will make the original return unreachable:

```
  switch (Shape.ABI) {                                                  
  // In these ABIs, the cloned functions always return 'void', and the
  // existing return sites are meaningless.  Note that for unique 
  // continuations, this includes the returns associated with suspends;
  // this is fine because we can't suspend twice.
  case coro::ABI::Switch:                                                
  case coro::ABI::RetconOnce:
    // Remove old returns.
    for (ReturnInst *Return : Returns)
      changeToUnreachable(Return);                                     
    break;
```

and `replaceCoroEnds()` will replace the `@llvm.coro.end` by `ret void`.

So perhaps CoroCloner is the one who should clean up these instructions.

On the other hand, it seems corosplit relies on later passes to clean up this stuff, which is maybe fine (especially since it also runs at `-O0`), except that it's a problem for `addMustTailToCoroResumes()`.

> simplify seems overkill

Yes, maybe it's too big a hammer. On the other hand, the ad-hoc dce and constant folding in `simplifyTerminatorLeadingToRet()` makes me a little nervous.


Instead of deleting those dead instructions, maybe we could just look past them. What the code is really trying to do is follow the control flow to a return instruction. I think it should be able to skip any side-effect free instructions on the way as long as the values aren't needed for a conditional branch or such. I'll push a version which does that. Let me know what you think.

https://github.com/llvm/llvm-project/pull/85271