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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks accepted this revision.
aeubanks added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroCleanup.cpp:57
-bool Lowerer::lowerRemainingCoroIntrinsics(Function &F) {
-  bool Changed = false;
-
----------------
ChuanqiXu wrote:
> 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.
oh sorry, I forgot this was now a module pass


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

https://reviews.llvm.org/D124362