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

Chuanqi Xu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 10 02:08:02 PDT 2023


ChuanqiXu added a comment.

I got your point roughly. And it makes sense to me. But the patch itself is still not so readable. It will be better to refactor it to make it more informative.

For example,

1. `VisitingSave` is not different with `VisitingTmp` really.
2. We can't understand the meaning of things like `TopVisiting`. In case it is hard to give a very meaningful name, we should add a comment to describe the meaning of the variable.
3. It is confusing that the variables with names like `Indegree` and `HasLoop` can change during the iteration. Since such names indicates that these variables tries to describe the attributes of the graph. So that they shouldn't change if the graph don't change.
4. The names `J` and the `first`, `second`, `third` iterations are really confusing. We should split them into phases with formal name.
5. Also it is bad to put things in a big loop. We can split them with different functionalities. e.g., we can have a phase to collect `consume` information and another phase to calculate the `kill` information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154695



More information about the llvm-commits mailing list