numba / numba-rvsdg

Numba compatible RVSDG (Regionalized Value State Dependence Graph) utilities.
https://numba-rvsdg.readthedocs.io/
BSD 2-Clause "Simplified" License
18 stars 7 forks source link

Multi-header assertion error #5

Open sklam opened 1 year ago

sklam commented 1 year ago

This is a variant on the SCC algorithm, mainly the yield statement are replaced: https://gist.github.com/sklam/dbe21c6df5b882bc41a27ff3e6cf79a0

Error:

Traceback (most recent call last):
  File "/path/to/numba-rvsdg/test_fig_scc.py", line 72, in <module>
    lflow = cflow._restructure_loop()
            ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/numba-rvsdg/byteflow2.py", line 637, in _restructure_loop
    restructure_loop(region.subregion)
  File "/path/to/numba-rvsdg/byteflow2.py", line 928, in restructure_loop
    extract_region(bbmap, loop, "loop")
  File "/path/to/numba-rvsdg/byteflow2.py", line 1002, in extract_region
    assert len(headers) == 1
AssertionError
esc commented 1 year ago

After merging #20 this now passes the loop restructure phase but then a different exception is raised in the branch restructure phase:

Traceback (most recent call last):
  File "/Users/esc/git/numba-rvsdg/test_fig_scc.py", line 79, in <module>
    bflow = lflow._restructure_branch()
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/esc/git/numba-rvsdg/numba_rvsdg/core/datastructures/byte_flow.py", line 43, in _restructure_branch
    restructure_branch(region.subregion)
  File "/Users/esc/git/numba-rvsdg/numba_rvsdg/core/transformations.py", line 345, in restructure_branch
    head_region_blocks = find_head_blocks(bbmap, begin)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/esc/git/numba-rvsdg/numba_rvsdg/core/transformations.py", line 244, in find_head_blocks
    assert len(jt) == 1
AssertionError
esc commented 1 year ago

With #40 I was able to fix this issue too, and was able to render the graph for this code for the first time.

Even with the monitor set to vertical orientation, the nodes are quite small:

Screen Shot 2023-04-06 at 20 06 05
esc commented 1 year ago

With #61 the scc is now fully working and the test case has been added to the test-suite.

Here is s screenshot:

Screen Shot 2023-06-15 at 18 04 35

To note, that #61 eliminates the last bugs in the tail detection, as now all blocks are inside their tails. We still need to check if the translation is indeed correct manually.

esc commented 1 year ago

@sklam can we close this now, are you satisfied?

sklam commented 1 year ago

Do we have a test for this for both loop and branch restructing?