|
| 1 | +- Start Date: (fill me in with today's date, YYYY-MM-DD) |
| 2 | +- RFC PR: [amaranth-lang/rfcs#0000](https://github.com/amaranth-lang/rfcs/pull/0000) |
| 3 | +- Amaranth Issue: [amaranth-lang/amaranth#0000](https://github.com/amaranth-lang/amaranth/issues/0000) |
| 4 | + |
| 5 | +# Deprecate non-FWFT FIFOs |
| 6 | + |
| 7 | +## Summary |
| 8 | +[summary]: #summary |
| 9 | + |
| 10 | +Deprecate non-first-word-fall-through FIFOs. |
| 11 | + |
| 12 | +## Motivation |
| 13 | +[motivation]: #motivation |
| 14 | + |
| 15 | +Currently, FIFOs in `amaranth.lib.fifo` have two incompatible interfaces: FWFT (first word fallthrough) and non-FWFT. The incompatibility concerns only the read half. FWFT FIFOs have `r_data` valid when `r_rdy` is asserted. Non-FWFT FIFOs have `r_data` valid only after strobing `r_en`, if the FIFO was empty previously. |
| 16 | + |
| 17 | +Non-FWFT interface is awkward and is essentially never used. It is a holdover from Migen and its implementation details that was included for compatibility. There are three downsides to having it: |
| 18 | +1. Having non-FWFT FIFOs requires every consumer of the FIFO interface to check for `fwft` when interacting with the FIFO and either asserting that it is `True`, or adding a code path to handle it. No one does this. |
| 19 | +2. The FWFT interface is directly compatible with streams and allows us to add e.g. `r_stream` and `w_stream` to existing FIFOs without adding a wrapper such as `stream.FIFO`. It also makes any custom FIFOs defined downstream of Amaranth stream-enabled. |
| 20 | +3. The notion of FWFT vs non-FWFT FIFOs is confusing and difficult to understand. E.g. the author of this RFC wrote both `lib.fifo` and the Glasgow FIFO code, and she misused the `fwft` argument in the latter. |
| 21 | + |
| 22 | +## Guide-level and reference-level explanation |
| 23 | +[guide-level-explanation]: #guide-level-explanation |
| 24 | + |
| 25 | +In the next version, instantiating `SyncFIFO(fwft=False)` emits a deprecation warning. In addition, `FIFOInterface`'s `fwft` parameter now defaults to `True`. Other FIFOs have no non-FWFT variant in the first place. |
| 26 | + |
| 27 | +In the version after that, there is no way to instantiate `SyncFIFO(fwft=False)`. The feature and all references to it are removed in their entirety. |
| 28 | + |
| 29 | +## Implementation considerations |
| 30 | +[reference-level-explanation]: #reference-level-explanation |
| 31 | + |
| 32 | +At the moment, `SyncFIFOBuffered` is implemented as a register in the output of `SyncFIFO(fwft=False)`. The implementation will need to be rewritten. |
| 33 | + |
| 34 | +## Drawbacks |
| 35 | +[drawbacks]: #drawbacks |
| 36 | + |
| 37 | +- Churn. |
| 38 | +- There will be no alternative to `SyncFIFO(fwft=False)`. |
| 39 | + |
| 40 | +## Rationale and alternatives |
| 41 | +[rationale-and-alternatives]: #rationale-and-alternatives |
| 42 | + |
| 43 | +- It is feasible to extract `SyncFIFO(fwft=False)` into its own module that may be used by downstream code that needs non-FWFT FIFOs. It would not implement `FIFOInterface`. |
| 44 | + - There is no reason the `SyncFIFO` class could not be copied into downstream code as it is. |
| 45 | +- It is possible to wrap FIFOs in the stream library in a way that ensures only FWFT FIFOs are used. |
| 46 | + - Let's not create pointless wrappers. |
| 47 | + |
| 48 | +## Prior art |
| 49 | +[prior-art]: #prior-art |
| 50 | + |
| 51 | +Not relevant. |
| 52 | + |
| 53 | +## Unresolved questions |
| 54 | +[unresolved-questions]: #unresolved-questions |
| 55 | + |
| 56 | +None. |
| 57 | + |
| 58 | +## Future possibilities |
| 59 | +[future-possibilities]: #future-possibilities |
| 60 | + |
| 61 | +This RFC primarily exists to enable better stream interface integration. |
0 commit comments