AUCOHL / DFFRAM

Standard Cell Library based Memory Compiler using FF/Latch cells
Apache License 2.0
134 stars 33 forks source link

Documentation error? #146

Open donnie-j opened 2 years ago

donnie-j commented 2 years ago

Caveat: I've not simulated the netlist...

Is it not the case that for 1RW1R RAM:

Again, I've not looked carefully at the model or run a simulation on the RTL or netlist, but my suspicion would be the first is true and the second is false, and the 1RW timing diagram is wrong because read happens in the same cycle as write. If I am correct about that, then the next question is, is this the written value or the previous value (is it read before or after write)?

In any case, the implied behavior seems inconsistent. J.

donnie-j commented 2 years ago

my suspicion would be the first is true and the second is false, and the 1RW timing diagram is wrong because read happens in the same cycle as write.

This turns out to be correct. Here are waveforms from a simulation of the netlist RAM8.nl.v from a run of 8x8_default ram, with no options. Screenshot 2022-03-30 at 20 59 44

There is an important caveat for latch based RAM, however: During a write cycle, all signals must be stable before clock falls. If this doesn't happen, the gate for the latch cells shows up in the next clock cycle, which obviously doesn't write what and where we expect. Screenshot 2022-03-30 at 20 54 14

donnie-j commented 2 years ago

For completeness, here is the same test bench run against a macro with flops USE_LATCH=0 instead of latch cells. The change in size is not that significant, but I suspect clock tree skew might be more problematic in the array itself. Write timing is relaxed, but looks to me unsurprisingly read is still async, the same as with the latch implementation.

In both cases, read is async, as shown below after the cursor where Do changes immediately when A (or EN) changes, not after clock edge.

The docs are definitely in need of update, or users of the RAM modules will likely be unpleasantly surprised that data appears one cycle earlier than expected. I also suggest the model might benefit from an option to insert output flops `define SYNC_READ perhaps.

Screenshot 2022-03-31 at 21 45 53