[PATCH] D87817: [LICM][Coroutine] Don't sink stores to coroutine suspend point.

Alina Sbirlea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 22 12:46:13 PDT 2020


asbirlea added a comment.

Can you please extend the testing to cover at least some of these cases: multiple predecesssors, multiple exit blocks, multiple predecessors with a coroutine switch?



================
Comment at: llvm/test/Transforms/LICM/sink-with-coroutine.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S < %s -licm | FileCheck %s
+
----------------
Add new pass manager testing using `-passes`.


================
Comment at: llvm/test/Transforms/LICM/sink-with-coroutine.ll:3
+; RUN: opt -S < %s -licm | FileCheck %s
+
+; LICM across a @coro.suspend.
----------------
Add memoryssa verification with `-verify-memoryssa`


================
Comment at: llvm/test/Transforms/LICM/sink-with-coroutine.ll:11
+; CHECK-NEXT:    br label [[BB0:%.*]]
+; CHECK:       bb0:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
----------------
Nit: update test to use the variables generated.
For example here, replace `bb0:` with [[BB0]], generated one line above.


================
Comment at: llvm/test/Transforms/LICM/sink-with-coroutine.ll:18
+; CHECK-NEXT:    switch i8 [[SUSPEND]], label [[BB2_SPLIT_LOOP_EXIT1:%.*]] [
+; CHECK-NEXT:    i8 0, label [[AWAIT_READY]]
+; CHECK-NEXT:    ]
----------------
Following up on the question of "is it always a switch", this looks like it could be have optimized to:
```
 %cond = icmp eq i8  [[SUSPEND]], 0
br i8 [[AWAIT_READY]], label [[BB2_SPLIT_LOOP_EXIT1:%.*]]
```
For example, simple loop unswitch may do such a change, and licm is in the same LPM, so perhaps this is possible.


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

https://reviews.llvm.org/D87817



More information about the llvm-commits mailing list