Add support for poke/expect using a list of values for nested arrays #36
Conversation
Pull Request Test Coverage Report for Build 290
💛 - Coveralls |
fault/tester.py
Outdated
| @@ -1,4 +1,5 @@ | |||
| import magma | |||
| import magma as m | |||
There was a problem hiding this comment.
Let's just use one of these imports; impartial to if we use as m or not.
fault/tester.py
Outdated
| def poke(self, port, value): | ||
| value = make_value(port, value) | ||
| self.actions.append(actions.Poke(port, value)) |
There was a problem hiding this comment.
I think this logic should be delegated to the targets -- tester should ideally not be implementing logic to split a poke(array_port, [...]) into several actions. I would argue these functions should be unchanged, and in the targets we do a:
if isinstance(value, fault.Array):
...Because, the magma/coreir simulator may support directly set_value(array_port, [...]) where as of course for verilator we need to blow it up.
There was a problem hiding this comment.
Good point, that's actually how I initially implemented it. I'll make this change.
tests/test_tester.py
Outdated
| @@ -52,6 +52,22 @@ def test_tester_nested_arrays(): | |||
| check(tester.actions[i], exp) | |||
|
|
|||
|
|
|||
| def test_tester_nested_arrays_list_of_vals(): | |||
There was a problem hiding this comment.
Can we copy this test for the actual targets and ensure that it works?
… arrays" This reverts commit 771e4de.
|
Review comments addressed, ready for another pass. The logic has been moved to the targets. |
fault/magma_simulator_target.py
Outdated
| isinstance(action.port.T, (m._BitType, m._BitKind)): | ||
| got = BitVector(got).as_uint() | ||
| else if isinstance(action.port, (m.ArrayType, m.ArrayKind)): | ||
| raise NotImplementedError("Printing complex nested arrays") |
There was a problem hiding this comment.
I think we should defer this feature for now. The user can print elements of the array fine, but there's not a simple way to map a complex nested array to a standard format string.
|
Looks good, thanks for addressing the comments. |
Support passing a list of values as an argument to
pokeandexpectfor nested arrays. This still supports passing a single value, which is convenient if you want to zero all the children of the nested arrays, so you can say something liketester.poke(circuit.nested_array, 0).Also includes a default
__repr__method for actions that uses__str__for now (useful for debugging when printing a list of actions, which calls repr)