implementation of an AST -> SCFG transformer #114
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AST: Python Abstract Synatx Tree
(S)CFG: (Structured) Control Flow Graph
Items to note for reviewers:
* Most of changes are in the `ast_transforms.py` module which contains
the meat of the implementation.
* Temporary data structures are created for a pure AST based CFG
(`ASTCFG`) with blocks that are writable. This made it much easier to
implement the builder pattern. They can easily be converted to a SCFG
using the `ASTCFG.to_SCFG()` function.
* The `AST2SCFGTranformer` is stateful and is populated with
housekeeping data structures post transform.
* The tests are all in `test_ast_transforms.py`. They are of the usual
format: Python function as input and serialized graph to compare to.
* The tests also check the values of the unreachable and empty nodes
that have been pruned from the CFG.
* The for-loop decomposition is a bit involved, hence there is an
extensive docstring to describe what it does.
* Some minor changes had to be made to the `scfg.py` module to support
displaying "broken" SCFGs during development. This makes it easier to
visually debug the graph upon making programming mistakes.
* The modules `basic_block.py` and `rendering.py` were modified to
support construction and visualization of the `PythonASTBlock`, which
is a new block type for the SCFG that contains a block derived from
Python AST.
* The central entrypoint is: `AST2CFG(function) -> SCFG `. This will
turn a Python function into an SCFG.
* A stub has been added for the invserse, SCFG -> AST transformer.
Example:
```python
from numba_rvsdg import AST2SCFG
def fun():
...
scfg = AST2SCFG(fun)
scfg.render()
scfg.restructure()
scfg.render()
```
|
@brandonwillard have a look at the above ☝️ may be of interest to you. |
This is fantastic! |
As title
As title
As title
This reduces the graph even more, since blocks with a single break or continue become empty blocks and are pruned
|
Coverage report for caa5ea2 |
As title
The jump_targets for the else-branch of the for-loop must be set after recursion, not before. Otherwise nested flow-control constructs in the else-branch will fail to codegen correctly, leading to a broken CFG with blocks that point to non-existing blocks. The test exposes this and demonstrates the fix is valid.
sklam
reviewed
Apr 23, 2024
This test exposes a bug in a missing seal for using continue in the nested for-else clause. Specifically, one needs to have an else-clause with a series of linear statements and a continue to hit this. If you try to break, the break index will be the same as the exit index for the seal_inside_of_loop as the default_index is also the exit_index. So you can only hit this with exit_index. In that case the block with the continue does not point to the loop header but to the incorrect exit instead. The test has been constructed such that a potential reconstruction would also fail execution with an incorrect return value.
As title
As title
As title
As title
|
@sklam I have addressed all the review comments and added most of the features we spoke about OOB. The only thing I don't know how to decide on is how to further structure the API. In any case, the transformations are implemented and work so maybe we need to defer decisions about the API to a later stage. |
sklam
reviewed
Apr 29, 2024
|
As per an OOB discussion with @sklam I have addressed all the requested improvements and am merging this now! Thank you for the review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
AST: Python Abstract Synatx Tree
(S)CFG: (Structured) Control Flow Graph
Items to note for reviewers:
ast_transforms.pymodule which contains the meat of the implementation.ASTCFG) with blocks that are writable. This made it much easier to implement the builder pattern. They can easily be converted to a SCFG using theASTCFG.to_SCFG()function.AST2SCFGTranformeris stateful and is populated with housekeeping data structures post transform.test_ast_transforms.py. They are of the usual format: Python function as input and serialized graph to compare to.scfg.pymodule to support displaying "broken" SCFGs during development. This makes it easier to visually debug the graph upon making programming mistakes.basic_block.pyandrendering.pywere modified to support construction and visualization of thePythonASTBlock, which is a new block type for the SCFG that contains a block derived from Python AST.AST2CFG(function) -> SCFG. This will turn a Python function into an SCFG.Example: