implement and test SCFG->AST conversion #127
Conversation
This implements the creation of Python source code from a potentially
restructured SCFG.
The main entry-point is:
```
from numba_rvsdg import SCFG2AST
```
And the round-trip test for the entry-points shows how to use the API:
```
class TestEntryPoints(TestCase):
def test_rondtrip(self):
def function() -> int:
x = 0
for i in range(2):
x += i
return x, i
scfg = AST2SCFG(function)
scfg.restructure()
ast_ = SCFG2AST(function, scfg)
exec_locals = {}
exec(ast.unparse(ast_), {}, exec_locals)
transformed = exec_locals["transformed_function"]
assert function() == transformed()
```
Special attention was paid to the testing of the transform. For all the
previously defined testing functions to test the AST -> SCFG direction,
the tests were augmented to also test the SCFG -> AST direction. To test
a transformed function, we assert behavioral equivalence by running the
original and the transformed through a set of given arguments and ensure
they always produce the same result. Additionally we ensure that all
lines of the test function are covered using a custom `sys.monitoring`
setup. (As a result the package now needs at least 3.12 for testing).
This ensures that that the set of arguments covers the original function
and also that the transformation back to Python doesn't create any dead
code.
Special thanks to @stuartarchibald for the stater patch for the custom
`sys.monitoring` based tracer.
Overall the iteration over the SCFG is still somewhat unprincipled,
however, the tests and overall coverage do seem to suggest the approach
works. Now that we can transform Python programs and have solid tests
too, more thought ought to be invested into storing the SCFG in a
non-recursive data-structure and developing a more elegant API to
traverse the graph and it's regions depending on the use-case.
Typing and annotations are a nbit haphazard, there are multiple issues
in the existing classes so some parts of this code just choose to use
`Any` and `# type: ignore` pragmas.
| if_break_node = ast.If( | ||
| test=if_beak_node_test, | ||
| body=[ast.Continue()], | ||
| orelse=[ast.Break()], | ||
| ) |
There was a problem hiding this comment.
I found a way to avoid the use of continue and break.
Instead of:
while True:
cont = <loop_body>
if cont:
continue
else:
breakemit this:
cont = True
while cont:
cont = <loopbody>There was a problem hiding this comment.
does that we now eliminate the use of break and continue altogether for all restructured programs?
As title
As title
|
@sklam one idea I had was to use https://peps.python.org/pep-0622/ instead of generating the if-cascade? It might make it a bit simpler. What do you think? Or is it better to code generate programs with the smallest subset of the available syntax? |
|
@sklam the other thought I had was to surround all the introduced variables using dunder |
Maybe even with a prefix like |
That's a good idea, it would make it easy to then ban any input program that uses variables that begin with |
As title
Done in 40b9daa |
As title
As title
There was a problem hiding this comment.
I've been testing this PR against my pyasir work---latest working commit sklam/pyasir@40bf0ed
I got to 2d loops working and emitting correct LLVM; e.g.
def sum1d(n: Int64) -> Int64:
c = 0
for i in range(n):
for j in range(i):
c += i + j
return cI was hoping to get to code like below but I'm still messing up variable dependency in the if-statement:
def sum1d(n: Int64) -> Int64:
c = 0
for i in range(n):
for j in range(i):
c += i * j
if c > 100:
break
return cAs far as I can tell, output from SCFG2ASTTransformer for the example above is correct. All previously mentioned issues are resolved. This PR is good to merge!
For future work, I'll suggest allowing the user of SCFG2ASTTransformer specify how certain things are handled. Currently, I'm fixing in after the fact with custom ASTTransformers (see https://github.com/sklam/pyasir/blob/40bf0eda7dde93acacfbefc6c4ffe630d40dd153/test_scfg_frontend.py#L227-L233).
|
@sklam thank you for the review! |
This implements the creation of Python source code from a potentially restructured SCFG.
The main entry-point is:
And the round-trip test for the entry-points shows how to use the API:
Special attention was paid to the testing of the transform. For all the previously defined testing functions to test the AST -> SCFG direction, the tests were augmented to also test the SCFG -> AST direction. To test a transformed function, we assert behavioral equivalence by running the original and the transformed through a set of given arguments and ensure they always produce the same result. Additionally we ensure that all lines of the test function are covered using a custom
sys.monitoringsetup. (As a result the package now needs at least 3.12 for testing). This ensures that that the set of arguments covers the original function and also that the transformation back to Python doesn't create any dead code.Special thanks to @stuartarchibald for the stater patch for the custom
sys.monitoringbased tracer.Overall the iteration over the SCFG is still somewhat unprincipled, however, the tests and overall coverage do seem to suggest the approach works. Now that we can transform Python programs and have solid tests too, more thought ought to be invested into storing the SCFG in a non-recursive data-structure and developing a more elegant API to traverse the graph and it's regions depending on the use-case.
Typing and annotations are a nbit haphazard, there are multiple issues in the existing classes so some parts of this code just choose to use
Anyand# type: ignorepragmas.