Repository navigation
Conversation
Add `src/cfg/wto.h` with `WeakTopologicalOrdering` (`WTO`) and `WTOWorklist` built on top of `DomTree`. In a reducible CFG ordered in reverse postorder, every cycle is a natural loop headed by a block that dominates all blocks in the cycle, allowing a Bourdoncle Weak Topological Ordering to be constructed directly from the dominator tree and natural loops of the CFG. Include unit tests in `test/gtest/wto.cpp` and `TODO` comments noting follow-on optimizations.
Wait, I thought you were saying we didn't need Bourdoncle's algorithm in the end? |
|
Bourdoncle defined weak topological ordering in the same paper where he introduced his algorithm to compute it. We're using his definition of weak topological ordering, but not his algorithm. |
|
Hahaha I'm not sure I've seen a clang crash on CI before |
| // parenthesized into nested cycles. The first element of each cycle is its | ||
| // "head" (loop header), and the ordering satisfies two properties: | ||
| // | ||
| // 1. Every non-cycle edge u -> v goes forward in the flattened ordering |
There was a problem hiding this comment.
What is a "non-cycle edge"? I can imagine two things
- An edge that does not return to the head, i.e., does not literally cycle
- An edge that goes outside of the loop, i.e., to places that are not in the cycle
There was a problem hiding this comment.
Any edge that is not a backedge, i.e. does not go to the head of an enclosing cycle.
There was a problem hiding this comment.
I tightened up this definition.
| for (auto* pred : blocks[h]->in) { | ||
| Index p = pred->contents.index; | ||
| if (dominates(h, p)) { | ||
| nodes[h].isLoopHeader = true; |
There was a problem hiding this comment.
I follow this up to here. The worklist processed on line 173, however, is unclear to me. Maybe add some comments on what is happening here?
| std::get<typename WeakTopologicalOrdering<BasicBlock>::Cycle>(elem); | ||
| BasicBlock* head = cycle.head(); | ||
| do { | ||
| self(self, cycle.elems); |
There was a problem hiding this comment.
Can we avoid this recursion? Or is the idea that loop recursion is limited?
There was a problem hiding this comment.
We probably could avoid this recursion, but loop depth should be relatively limited. I suggest we leave it unless it causes a problem for someone.
There was a problem hiding this comment.
sgtm, then perhaps a comment to mention that?
|
|
||
| namespace { | ||
|
|
||
| struct TestCFG { |
There was a problem hiding this comment.
Maybe add a comment here? At a glance this looks like a mini version of the real thing..? (but surely it can't be that?)
There was a problem hiding this comment.
It is a mini version of the real thing! Maybe this is overkill, will investigate.
There was a problem hiding this comment.
Yeah, at first glance... that feels like overkill.
There was a problem hiding this comment.
I think this is reasonable after all. See new comment.
|
Oh wow, separate ICEs on CI in both clang and gcc 🤯 |
Add
src/cfg/wto.hwithWeakTopologicalOrdering(WTO) andWTOWorklistbuilt on top ofDomTree. In a reducible CFG ordered in reverse postorder, every cycle is a natural loop headed by a block that dominates all blocks in the cycle, allowing a Bourdoncle Weak Topological Ordering to be constructed directly from the dominator tree and natural loops of the CFG.Include unit tests in
test/gtest/wto.cppandTODOcomments noting follow-on optimizations.