[PATCH] D154695: [Coroutines] Add an O(n) algorithm for computing the cross suspend point information.

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

Thanks for running the out-of-tree tests. I know it is not easy. The failure tests look not related to coroutines. Another method to test this is to add an `assert(false)` in your code and re-run tests. So that you know you won't get things wrong.

Then let's focus on readability in this review. It looks better now but there is still confusing points like:

  if constexpr (Iteration > 1)

Also we'd better to not use the term `Indegree` so that we can avoid:

  Indegrees[I]--;

I suggest to replace the current `computeBlockData` with 2 functions: collectConsumingBlock<bool SearchBackEdges>,  collectKillingBlock. Then the semantics should be much more clear.



================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:124
+  /// As we all know that topological sorting need to be used on DAG. But CFG
+  /// maybe not a DAG as CFG may have loop, so we need to break the loop.
+  /// If current CFG is a DAG, we just need to visit once. If current CFG is not
----------------
In the algorithm, I prefer to use the term `back edge` than `loop`. Since loop has many more meanings and attributes so that the back edges would be more precise.

Also with the new term `back edge`, we can describe the algorithm much more simpler:
1. In the initial iterations, all the consume information along the forward edges are collected.
2. (If there is any back edges), we can iterate again to get all the consume information along the back edges.
3. We can compute the Kill informations by the consume informations.

In this way, it is much easier to understand too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154695