Support non-initialisable memories

[ghost]

In order to use memory types that don't support initialisation (e.g. iCE40 SPRAM), we need to be able to create memories without initialising them.

In a1e2c75, we check for a truthy memory.init, as the default value (None) is coalesced to [] in the setter:

amaranth/amaranth/hdl/mem.py

Lines 66 to 68 in b1cce87

	@init.setter
	def init(self, new_init):
	self._init = [] if new_init is None else list(new_init)

We could equally check if len(memory.init) > 0.

Potential for breaking behaviour?

This does have the potential for wider-reaching impact on behaviour, for memory types that do support initialisation. Previously, a None or empty init would explicitly zero-initialise the entire memory, and indeed, in simulation this will still be the case, but now no $meminit cell would be emitted and the exact behaviour is defined by the backend/target. To explicitly force zero-init as currently happens, one would need to supply at minimum Memory(init=[0]).

In practice, after Yosys is done, e.g. for SB_RAM40_4K, the INIT_0–INIT_F parameters to the target memory cell become xxxx.... By the time nextpnr is done with it, the corresponding .ram_data in the bitstream have all zeroes.

Changed meaning of init values

I've included a second commit 1021e34, which is even more of a suggestion than the first, which updates the docstring/tests and modifies the behaviour of the attribute slightly. This moves the boundary of behaviour change; we now distinguish between:

• init=None (no explicit initialisation), and
• init=[] (zero-init).

The type of Memory.init now also admits None, which might bite users whose code relied on reading back their memory.init assuming it would always be a list[int].

Assuming it's OK to rely on the assumption "initialisable RAM cells that aren't explicitly initialised will get zero-initted anyway later on in the process by the backend" like above, I think the reasonable choice is to keep the default init=None. The API is conceptually simple: no init specified means no specified initialisation.

Note on possible futures

There could be surprising consequences if that assumption stops holding. Continuing with the SB_RAM40_4K example, even though currently initial data is always included in the bitstream output by the Yosys/nextpnr flow, Memory Usage Guide for iCE40 Devices (p6) suggests this is optional. If this optionality were realised down the track, in sum these changes would mean EBR that was previously reliably zero-initialised would cease to be so.

Thus, depending on our tolerance for possibly breaking changes, we may prefer a default of init=[], which will zero-initialise all storage elements like the current default. Uninitialised memory would be opt-in only. There would still be the potential to break existing Amaranth designs (i.e. under the above scenario where the backend behaviour changes), as users may be explicitly passing None expecting zero-init.

Alternative solutions

• Add a sentinel value for the init parameter (e.g. mem.UNINITIALIZED) and retain all existing behaviour.
  • This maxes out on backwards compatibility, at the cost of adding an edge to the API.
  • It's clean, and reinforces that Amaranth has a separate semantics to its backends; i.e. memories are zero-init unless explicitly requested otherwise.
    • This is good in the "self-consistent and predictable by default" sense, bad in the "more to learn/be surprised by for old-timers" sense.
    • It seems to me like we want this property especially when it comes to interop with the native Python environment (hence treatment of indexing, exclusive ranges, sequence order, etc.), but I'm not clear on e.g. just how abstract we want something like a Memory to seem.
• Do nothing.
  • Uninitialisable memories continue to require the use of Instance and are not usable with Amaranth memory primitives.

[codecov]

Codecov Report

Merging #824 (1021e34) into main (b1cce87) will decrease coverage by 0.12%.
The diff coverage is 28.57%.

@@            Coverage Diff             @@
##             main     #824      +/-   ##
==========================================
- Coverage   82.11%   82.00%   -0.12%     
==========================================
  Files          50       50              
  Lines        6873     6874       +1     
  Branches     1673     1674       +1     
==========================================
- Hits         5644     5637       -7     
- Misses       1034     1041       +7     
- Partials      195      196       +1     

Impacted Files	Coverage Δ
amaranth/back/rtlil.py	77.83% <0.00%> (-1.25%)	⬇️
amaranth/hdl/mem.py	98.16% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[whitequark]

This has been discussed before and is clearly necessary for both ASIC and SPRAM/URAM support, but it is a major breaking change that also doesn't fit the current model Amaranth has (where every storage bit has a defined value after reset). It would require semantic changes to support uninitialized 'x in simulation and synthesis, and an RFC to define the abovementioned semantics.

Please see #270 for the previous discussion.

[ghost]

Thanks! I searched around a fair bit looking at previous discussions and PRs but missed that one. I'll have a careful read.