[PATCH] D156850: [NFC][Coroutines] Use a reverse post-order to guide the computation about cross suspend infomation to reach a fixed point faster.

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

This reads much better! Thanks.

How long would the patch compile for the case?

  #include <cassert>
  #include <cstdio>
  #include <cstdlib>
  
  int main(int argc, char **argv) {
    assert(argc == 2);
    int n = atoi(argv[1]);
    printf("#include <cstdio>\n");
    printf("#include <vector>\n");
    printf("#include \"task.h\"\n");
    printf("extern bool count(int);\n");
    printf("task t() {\n");
    printf("std::vector<int> v(%d);\n", n);
    printf("int val = 0;\n");
    printf("for (int i = 0; i < %d; ++i) {;\n", n);
    for (int i = 1; i <= n; i++) printf("if (count(v[i])) val++;\n");
    printf("}\n");
    printf("printf(\"%%d\\n\", val);\n");
    printf("co_return;\n");
    printf("}\n");
    return 0;
  }



================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:117
   /// the initial case a little bit by manual loop switch.
-  template <bool Initialize = false> bool computeBlockData();
+  /// The parameter "RPOT" is a reverse post order.
+  /// Returns whether the BlockData changes in this iteration.
----------------
The comment reads slightly odd. Also I feel it may not be necessary to require RPO in `computeBlockData` since it simply iterates the given range. So may be the following signature may be better:

```
template <bool Initialize = false, class BBRangeTy>
bool computeBlockData(const BBRangeTy &BBRange);
```


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:330-331
 
-  computeBlockData</*Initialize=*/true>();
-
-  while (computeBlockData())
+  /// Use reverse post order to guide the computation. It will lead to reach
+  /// fixed point faster.
+  ReversePostOrderTraversal<Function *> RPOT(&F);
----------------
Maybe it is better to say: it is considered to be faster to use RPO traversal for forward-edges dataflow analysis. It may be best to cite this but this may not be required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156850