[PATCH] D124362: [NFC] [Pipelines] Hoist CoroCleanup as Module Pass (3/5)

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroCleanup.cpp:57
-bool Lowerer::lowerRemainingCoroIntrinsics(Function &F) {
-  bool Changed = false;
-
----------------
aeubanks wrote:
> why are we removing `Changed` and always invalidating analyses? if we have a module where most functions don't call any of these intrinsics, we're going to unnecessarily invalidate a lot more analyses which will be bad for compile times.
My thought is:
(1) We already checked if there is any coroutine intrinsic in the module before we start the pass.
(2) So there must be some changes for some functions.
(3) So the Changed variable would always be true. So I think it is redundant.

Here is the case that `M` owns the declaration of some coroutine intrinsics but not a function uses it actually. But I think it wouldn't  happen in real cases.


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

https://reviews.llvm.org/D124362