class Memory: add reset_less parameter that will not initialize memories with 0 value in generated RTL.

[nmigen-issue-migration]

Issue by Fatsie
Saturday Nov 16, 2019 at 13:21 GMT
Originally opened as m-labs/nmigen#270

---

I am using nmigen for generating RTL to be implemented on an ASIC. In an ASIC the content of a memory block is random at startup. Currently generated RTL by nmigen initializes memories by default with 0, which does not match behavior of a memory on an ASIC.
I am using external simulators and not pysim as my designs currently contain external verilog and VHDL code.
Some comments on the code:

• I named the parameter reset_less after the same parameter name for Signal. I am open for suggestion for other/better name.
• I added some unit test code but the feature would actually only be tested fully when generated rtlil code is read into yosys. Did not know how to best implement that in the nmigen unit test framework.
• For pysim it may be good if memories could be initialized with random values if reset_less is set to True. I suppose pysim does not want to support 'X' or 'U' values for signals.

---

Fatsie included the following code: https://github.com/m-labs/nmigen/pull/270/commits

[nmigen-issue-migration]

Comment by codecov[bot]
Saturday Nov 16, 2019 at 13:23 GMT

---

Codecov Report

Merging #270 into master will increase coverage by <.01%.
The diff coverage is 85.71%.

@@            Coverage Diff            @@
##           master    #270      +/-   ##
=========================================
+ Coverage   82.29%   82.3%   +<.01%     
=========================================
  Files          34      34              
  Lines        5598    5601       +3     
  Branches     1201    1202       +1     
=========================================
+ Hits         4607    4610       +3     
  Misses        851     851              
  Partials      140     140

Impacted Files	Coverage Δ
nmigen/hdl/mem.py	98.14% <100%> (+0.05%)	⬆️
nmigen/back/rtlil.py	83.25% <66.66%> (ø)	⬆️
nmigen/tracer.py	94.59% <0%> (ø)	⬆️
nmigen/hdl/ir.py	94.43% <0%> (ø)	⬆️
nmigen/hdl/ast.py	86.83% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 834fe3c...8bf6551. Read the comment docs.

[nmigen-issue-migration]

Comment by whitequark
Saturday Nov 16, 2019 at 13:25 GMT

---

Currently generated RTL by nmigen initializes memories by default with 0, which does not match behavior of a memory on an ASIC.

This is true. However, nMigen initializes signals to 0 as well (even reset_less Signals), which does not match behavior of registers on an ASIC. How are you dealing with that?

[nmigen-issue-migration]

Comment by Fatsie
Saturday Nov 16, 2019 at 13:34 GMT

---

This is true. However, nMigen initializes signals to 0 as well (even reset_less Signals), which does not match behavior of registers on an ASIC. How are you dealing with that?

I don't use reset_less signals at the moment. Difference is that for registers (asynchronous) reset flipflops will be used and get thus the right value using the reset. This is not the case for memories.

Actually I have been thinking of doing that 'X' initialization also for Signal but the testing required to have it right for both synchronous as combinatorial logic, slicing etc. scared me off.

[nmigen-issue-migration]

Comment by whitequark
Saturday Nov 16, 2019 at 13:42 GMT

---

Difference is that for registers (asynchronous) reset flipflops will be used and get thus the right value using the reset. This is not the case for memories.

But unlike on an FPGA, on an ASIC the flip-flops won't be reset unless you explicitly do so. So I see the challenges here as quite similar: both FPGA registers and (in most cases) memories are initialized, and neither ASIC registers nor memories are initialized.

I think adding this parameter to Memory is not appropriate, because (specifically in your case) this is not a property of the Memory, but rather a property of the platform. I believe it would be appropriate to alter the RTLIL backend such that it has an "ASIC mode" where it does not emit initial statements for either signals or memories.

[nmigen-issue-migration]

Comment by Fatsie
Saturday Nov 16, 2019 at 14:06 GMT

---

I believe it would be appropriate to alter the RTLIL backend such that it has an "ASIC mode" where it does not emit initial statements for either signals or memories.

Actually it's introducing back a feature from Migen where RTL generation actually was made altered for ASIC. I thought one of the goals for nMigen was to get rid of this difference...

Anyway so what do you think about having an extra emit_initial parameter to (rtilil|verilog).convert?

[nmigen-issue-migration]

Comment by whitequark
Saturday Nov 16, 2019 at 14:19 GMT

---

Actually it's introducing back a feature from Migen where RTL generation actually was made altered for ASIC.

That feature was removed from Migen because it was thought that no one used it, and it bitrotted. I do not see a way to generate the same code for FPGA and ASIC because FPGA and ASIC inherently have different behavior.

[nmigen-issue-migration]

Comment by Fatsie
Saturday Nov 16, 2019 at 19:06 GMT

---

Actually one of the reasons for using (n)Migen is to avoid the problem that designs that have been tested and are working on a FPGA don't work as an ASIC. Ideally nMigen code would always behave the same on FPGA and ASIC. If that is not possible at least the flow should error out if a feature is used that is not supported on a certain platform. It should not be the case that the behavior is dependent on the platform.
That's why I would like to implement the following:

• reset_less synchronous signals are not initialized in the initial section of generated RTL code independent of platform. To me setting reset_less to True indicates that one does not care what the actual value is at start-up.
• memory that is not explicitly initialized by passing values in init of Memory.__init__() is also not initialized
• pysim uses random values as start for both synchronous reset_less signals and memory not explicitly initialized.
• when one tries to implement a RAM on an ASIC where memory is initialized explicitly the flow will error. One difficulty is that ROMs, e.g. memories with only read ports, typically are possible on an ASIC by using a ROM compiler or even as synthesized logic as I do now for ao68000.

[nmigen-issue-migration]

Comment by whitequark
Saturday Nov 16, 2019 at 19:10 GMT

---

• reset_less synchronous signals are not initialized in the initial section of generated RTL code independent of platform. To me setting reset_less to True indicates that one does not care what the actual value is at start-up.

reset_less has never meant this in the entire history of (n)Migen. The (n)Migen reset rules for synchronous signals are:

• all signals are reset (to the reset value) at power-on reset;
• non-reset-less signals are reset (to the reset value) at clock domain reset (for the clock domain that drives them).

Changing the behavior to what you suggest will break all (n)Migen code in existence.

[nmigen-issue-migration]

Comment by Fatsie
Saturday Nov 16, 2019 at 19:20 GMT

---

Bugger, due to lack of good documentation this was not clear to me up to now.

Can at least this expectation be adapted for memories only ? POR (power-on-reset) for ASIC and flip-flops can be implemented if really needed. For memories it would mean adding a ROM and loading that into the RAM at POR.

Can you give also example of code that actually depends on the fact that value is set at POR but not at clock domain reset ?

[nmigen-issue-migration]

Comment by whitequark
Saturday Nov 16, 2019 at 19:41 GMT

---

Can you give also example of code that actually depends on the fact that value is set at POR but not at clock domain reset ?

Any FPGA design that is synthesized without an explicit reset (either external or generated inside the design) by relying on the ability to initialize flip-flops.

For example, Xilinx specifically recommends against adding a global reset unless it is unavoidable: rather than resetting the design after power-on, they recommend gating the clock until end of startup. By default, nMigen generates code that complies with these recommendations.

Can at least this expectation be adapted for memories only ?

It would have to be, if only because some FPGAs have memories that cannot be initialized (e.g. iCE40 SPRAM; IIRC, Xilinx UltraRAM). But it is not clear to me what is the ideal path forward here. One of the major design goals behind (n)Migen is that every design you write (without using instances) should be deterministic (modulo asynchronous clocks), so it tries very hard to make sure there are no uninitialized design elements.

Whatever is the language-level solution here, I think that it should be applied to both signals and memories. Otherwise, it would lead to simulation-synthesis mismatch on ASICs, which is just as bad as platform-dependent behavior, if not worse.

I do not like the idea of using random values in place of uninitialized ones in simulation. In case of registers, there is no guarantee that at power-on, the flip-flop is not in a metastable state. That can violate invariants in downstream logic. In all cases, using random values may give the impression that the downstream logic must behave correctly if it passes enough simulation runs, but of course that is not true, e.g. let's say such downstream logic breaks only if a 32-bit register has one particular value. So it can hide bugs.

Are you using property checking with SymbiYosys? If you are, it is easy to set it up so that every possible power-on state will be checked. Of course, this does not address the issue of metastability.

Overall, I am leaning towards implementing conservative X-propagation in pysim to handle uninitialized registers and memories. But it is a sizable task.

[nmigen-issue-migration]

Comment by whitequark
Saturday Nov 16, 2019 at 19:45 GMT

---

Bugger, due to lack of good documentation this was not clear to me up to now.

It is documented in the docstring for Signal, at least in nMigen:

https://github.com/m-labs/nmigen/blob/834fe3c700e420586b7c299bd42d2417966fa1c2/nmigen/hdl/ast.py#L809-L812

But I agree that the documentation is currently not very accessible and must be improved.

[nmigen-issue-migration]

Comment by Fatsie
Saturday Nov 16, 2019 at 19:56 GMT

---

It is documented in the docstring for Signal, at least in nMigen

An ASIC designer like me sees POR also as reset logic; so 'no reset logic for this Signal in synchronous statements' I interprete than also as no init during power on if reset_less is given. Combinatorial signals are not a problem as that is fully deterministic after synthesis and no difference between FPGA and ASIC.

[nmigen-issue-migration]

Comment by whitequark
Saturday Nov 16, 2019 at 20:01 GMT

---

An ASIC designer like me sees POR also as reset logic; so 'no reset logic for this Signal in synchronous statements' I interprete than also as no init during power on if reset_less is given.

Indeed that is a very FPGA-centric statement. Before nMigen explicitly declares ASIC support all such statements should be clarified.

[nmigen-issue-migration]

Comment by Fatsie
Saturday Nov 16, 2019 at 20:42 GMT

---

Any FPGA design that is synthesized without an explicit reset (either external or generated inside the design) by relying on the ability to initialize flip-flops.

For example, Xilinx specifically recommends against adding a global reset unless it is unavoidable: rather than resetting the design after power-on, they recommend gating the clock until end of startup. By default, nMigen generates code that complies with these recommendations.

That should in itself not be a problem. It should be possible to use global reset for ASIC and rely on initialization for FPGA. The problem is when you do want reset_less to have POR but not set/reset on the clock domain reset signal. So it would be good to know the current use cases where reset_less is set to True.

BTW, on ASICs external reset signals are going through a reset synchronizer. The synchronous reset deassertion is to make sure all set/reset signals for all the flip-flops are deasserted in the same clock cycle. Otherwise it could mess up state machines.

In case of registers, there is no guarantee that at power-on, the flip-flop is not in a metastable state.

Metastability is only a problem for CDC, during power-on flip-flops will always initialize to 0 or 1. The metastable state is not stable due to noise in transistors and the live time is much shorter than the time taken for power ramp up.
Actually metastability is a kind of a misnomer when used for CDC problems. It is actually the fact that when a signal is connected to different gates and it is in transition some of the gates may still 'see' the old value and some the new value. If at this time the output of the gates are latched in another domain inconsistent values will be clocked in. Thus vectors can have wrong values.

[nmigen-issue-migration]

Comment by Fatsie
Saturday Nov 16, 2019 at 21:01 GMT

---

I do not like the idea of using random values in place of uninitialized ones. In case of registers, there is no guarantee that at power-on, the flip-flop is not in a metastable state. That can violate invariants in downstream logic. In all cases, using random values may give the impression that the downstream logic must behave correctly if it passes enough simulation runs, but of course that is not true, e.g. let's say such downstream logic breaks only if a 32-bit register has one particular value. So it can hide bugs.

Actually I was only talking about random values for pysim. For external simulators like verilator/icarus I would use classic 'X' during simulation. I agree that also implementing unitialized value support in pysim would be the best solution but this would have a performance impact.