Migrate to ast_tools ssa #433
Conversation
Pull Request Test Coverage Report for Build 1973
💛 - Coveralls |
|
Still working through some issues in the peak/lassen flows so we should wait to pull until those repos are working with this branch. |
| @ast_tools.passes.end_rewrite() | ||
| @m.circuit.combinational() | ||
| @ast_tools.passes.begin_rewrite() | ||
| def {circuit_name}_comb({circuit_combinational_args}) -> ({circuit_combinational_output_type}): |
There was a problem hiding this comment.
Could we build the AST and directly call m.circuit.combinational()? This would avoid needing to inject ast_tools into env.
| ) | ||
| combinational = m.circuit.combinational(decorators=[sequential]) | ||
| return wrapped_sequential(True, cls, combinational) | ||
| if "DefineRegister" not in env: |
There was a problem hiding this comment.
I know this is a bit paranoid but all of these should should be injecting with a free name then passing the generated names around. e.g.:
def gen_register_instances(def_reg_name, initial_value_map, async_reset):
register_instances = []
for name, (value, type_, eval_type, eval_value) in initial_value_map.items():
if isinstance(eval_type, m.Kind):
if isinstance(eval_type, m._BitKind):
n = None
if isinstance(eval_value, (bool, int)):
init = bool(eval_value)
else:
assert eval_value.name.name in ["GND", "VCC"], eval_value.name
init = 0 if eval_value.name.name == "GND" else 1
else:
n = len(eval_type)
init = int(eval_value)
register_instances.append(f"{name} = {def_reg_name}({n}, init={init}, has_async_reset={async_reset})()") # noqa
else:
value = astor.to_source(value).rstrip()
register_instances.append(f"{name} = {value}")
return register_instances
magma/syntax/combinational.py
Outdated
| decorators=decorators) | ||
| return wrapped_combinational(fn) | ||
| return wrapped | ||
| phi_name = ast_tools.gen_free_name(tree, env, |
magma/syntax/combinational.py
Outdated
| return env[mux_name]([arg1, arg0], cond) | ||
|
|
||
| env.locals[phi_name] = phi | ||
| tree, env, metadata = ast_tools.passes.if_to_phi(phi_name).rewrite( |
There was a problem hiding this comment.
if_to_phi will generate a name for you with an optional prefix with the same logic that you have. Just call:
ast_tools.passes.if_to_phi(phi, 'magma_combinational_phi').rewrite(...)
https://github.com/leonardt/ast_tools/blob/master/tests/test_if_to_phi.py#L23
https://github.com/leonardt/ast_tools/blob/master/ast_tools/passes/if_to_phi.py#L42
|
Closing in favor of #668 |
Depends on leonardt/ast_tools#11
Also, there's still some cleanup to do w.r.t. the handling of the environment/names (mainly that the combinational and sequential passes rely on injecting some names in to the environment, we should make sure we're being safe about how we're doing this or use a different strategy).