This PR fixes something which bothered me for a long time. I couldn't wrap my head around why @lekcyjna123's MemoryBank implementation is so weird and complex. Finally, I understood the problem: in the version of Amaranth we are using, the read ports are transparent by default (!), and @lekcyjna123 basically tried to make non-transparent reads from a transparent read port.
What was changed:
The safe_writes argument was replaced with transparent, which controls the read port transparency.
The weird use of ArgumentsToResultsZipper and the risky use of an internal transaction which should always be able to run was removed. An explicit overflow management was added instead.
The test was changed so that it no longer uses implementation details of MemoryBank and handles transparency more naturally.
The test was changed so that it is able to check consecutive reads and consecutive writes.
Integrated the pipelining check into the main test.
This PR fixes something which bothered me for a long time. I couldn't wrap my head around why @lekcyjna123's
MemoryBank
implementation is so weird and complex. Finally, I understood the problem: in the version of Amaranth we are using, the read ports are transparent by default (!), and @lekcyjna123 basically tried to make non-transparent reads from a transparent read port.What was changed:
safe_writes
argument was replaced withtransparent
, which controls the read port transparency.ArgumentsToResultsZipper
and the risky use of an internal transaction which should always be able to run was removed. An explicit overflow management was added instead.MemoryBank
and handles transparency more naturally.