Skip to content

Commit

Permalink
hdl.mem: remove WritePort(priority=) argument.
Browse files Browse the repository at this point in the history
The write port priority in Yosys is derived directly from the order
in which the ports are declared in the Verilog frontend. It is being
removed for several reasons:
  1. It is not clear if it works correctly for all cases (FFRAM,
     LUTRAM, BRAM).
  2. Although it is roundtripped via Verilog with correct simulation
     semantics, the resulting code has a high chance of being
     interpreted incorrectly by Xilinx tools.
  3. It cannot be roundtripped via FIRRTL, which is an alternative
     backend that is an interesting future option. (FIRRTL leaves
     write collision completely undefined.)
  3. It is a niche feature that, if it is needed, can be completely
     replaced using an explicit comparator, priority encoder, and
     write enable gating circuit. (This is what Xilinx recommends
     for handling this case.)

In the future we should extend nMigen's formal verification to assert
that a write collision does not happen.
  • Loading branch information
whitequark committed Sep 28, 2019
1 parent e3a1d05 commit a02e375
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 5 deletions.
5 changes: 2 additions & 3 deletions nmigen/hdl/mem.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def elaborate(self, platform):


class WritePort(Elaboratable):
def __init__(self, memory, *, domain="sync", priority=0, granularity=None):
def __init__(self, memory, *, domain="sync", granularity=None):
if granularity is None:
granularity = memory.width
if not isinstance(granularity, int) or granularity < 0:
Expand All @@ -147,7 +147,6 @@ def __init__(self, memory, *, domain="sync", priority=0, granularity=None):

self.memory = memory
self.domain = domain
self.priority = priority
self.granularity = granularity

self.addr = Signal.range(memory.depth,
Expand All @@ -164,7 +163,7 @@ def elaborate(self, platform):
p_WIDTH=self.data.width,
p_CLK_ENABLE=1,
p_CLK_POLARITY=1,
p_PRIORITY=self.priority,
p_PRIORITY=0,
i_CLK=ClockSignal(self.domain),
i_EN=Cat(Repl(en_bit, self.granularity) for en_bit in self.en),
i_ADDR=self.addr,
Expand Down
2 changes: 0 additions & 2 deletions nmigen/test/test_hdl_mem.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def test_write_port(self):
wrport = mem.write_port()
self.assertEqual(wrport.memory, mem)
self.assertEqual(wrport.domain, "sync")
self.assertEqual(wrport.priority, 0)
self.assertEqual(wrport.granularity, 8)
self.assertEqual(len(wrport.addr), 2)
self.assertEqual(len(wrport.data), 8)
Expand All @@ -94,7 +93,6 @@ def test_write_port_granularity(self):
wrport = mem.write_port(granularity=2)
self.assertEqual(wrport.memory, mem)
self.assertEqual(wrport.domain, "sync")
self.assertEqual(wrport.priority, 0)
self.assertEqual(wrport.granularity, 2)
self.assertEqual(len(wrport.addr), 2)
self.assertEqual(len(wrport.data), 8)
Expand Down

0 comments on commit a02e375

Please sign in to comment.