migen icon indicating copy to clipboard operation
migen copied to clipboard

Ensure Memory that has a port with async_read=True is actually implemented as distributed ram

Open nakengelhardt opened this issue 6 years ago • 0 comments

Sometimes, when declaring a large memory with async_read=True, it gets placed in BRAM anyway. Most notably, when instantiating a large SyncFIFO. This is because the read address comes directly from a register, so it gets interpreted as the address register of a BRAM in write_first mode.

However, correct behavior of SyncFIFO depends on the address collision behavior of DRAM. The 'undefined' behavior of Xilinx BRAM in write_first mode is, in practice, the wrong behavior.

Since this unexpected size-dependent change of behavior is most likely undesirable in many cases where the user asks for an asynchronous read port, this patch adds a "ram_style = distributed" attribute to any memory that has one, to force it to be implemented in DRAM.

Unfortunately, "ram_style = distributed" is a Xilinx-only attribute, so as is this patch breaks Altera compatibility. There is currently nothing that specifies the platform in this context (and I'm not using platform in the first place), so this needs some big-picture rethinking of how to specify and hand down "is this for altera or xilinx" from verilog.convert().

nakengelhardt avatar Mar 29 '18 12:03 nakengelhardt