Skip to content

Commit

Permalink
back.rtlil: set read port init to all-x.
Browse files Browse the repository at this point in the history
This is an unfortunate necessity needed to fix memory inference regressions
introduced when we switched to using v2 cells. A better approach, compatible
with RFC 54, will need to be figured out for Amaranth 0.6.

Fixes #1011.
  • Loading branch information
wanda-phi authored and whitequark committed May 9, 2024
1 parent 4b49284 commit 1d2b9c3
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
34 changes: 31 additions & 3 deletions amaranth/back/rtlil.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,21 @@
})


# Special hack to emit 'x consts for memory read port init.
class Undef:
def __init__(self, width):
self.width = width


def _signed(value):
if isinstance(value, str):
return False
elif isinstance(value, int):
return value < 0
elif isinstance(value, _ast.Const):
return value.shape().signed
elif isinstance(value, Undef):
return False
else:
assert False, f"Invalid constant {value!r}"

Expand All @@ -45,6 +53,8 @@ def _const(value):
elif isinstance(value, _ast.Const):
value_twos_compl = value.value & ((1 << len(value)) - 1)
return "{}'{:0{}b}".format(len(value), value_twos_compl, len(value))
elif isinstance(value, Undef):
return f"{value.width}'" + "x" * value.width
else:
assert False, f"Invalid constant {value!r}"

Expand Down Expand Up @@ -1070,9 +1080,27 @@ def emit_read_port(self, cell_idx, cell):
"WIDTH": cell.width,
"TRANSPARENCY_MASK": _ast.Const(transparency_mask, memory_info.num_write_ports),
"COLLISION_X_MASK": _ast.Const(0, memory_info.num_write_ports),
"ARST_VALUE": _ast.Const(0, cell.width),
"SRST_VALUE": _ast.Const(0, cell.width),
"INIT_VALUE": _ast.Const(0, cell.width),
# Horrible hack alert: Yosys has two different Verilog code patterns it can emit for
# transparent synchronous read ports — the old, limitted one and the generic new one.
# The old one essentially consists of a combinational read port with a register added
# on the *address* input. It has several limitations:
#
# - can only express read ports transparent wrt *all* write ports
# - cannot support initial values
# - cannot support reset
# - cannot support clock enable
#
# The new pattern can express any supported read port, but is not widely recognized
# by other toolchains, leading to memory inference failures. Thus, Yosys will use
# the old pattern whenever possible.
#
# In order to enable Yosys to use the old pattern and avoid memory inference regressions
# with non-Yosys synthesis, we need to emit undefined initial value here. This is in
# direct conflict with RFC 54, and will have to be revisited before 0.6, possibly
# requiring a large-scale design change in Amaranth memory support.
"ARST_VALUE": Undef(cell.width),
"SRST_VALUE": Undef(cell.width),
"INIT_VALUE": Undef(cell.width),
"CE_OVER_SRST": False,
}
if isinstance(cell, _nir.AsyncReadPort):
Expand Down
12 changes: 6 additions & 6 deletions tests/test_back_rtlil.py
Original file line number Diff line number Diff line change
Expand Up @@ -1643,9 +1643,9 @@ def test_async_read(self):
parameter \WIDTH 8
parameter \TRANSPARENCY_MASK 1'0
parameter \COLLISION_X_MASK 1'0
parameter \ARST_VALUE 8'00000000
parameter \SRST_VALUE 8'00000000
parameter \INIT_VALUE 8'00000000
parameter \ARST_VALUE 8'xxxxxxxx
parameter \SRST_VALUE 8'xxxxxxxx
parameter \INIT_VALUE 8'xxxxxxxx
parameter \CE_OVER_SRST 0
parameter \CLK_ENABLE 0
parameter \CLK_POLARITY 1
Expand Down Expand Up @@ -1725,9 +1725,9 @@ def test_sync_read(self):
parameter \WIDTH 8
parameter \TRANSPARENCY_MASK 1'1
parameter \COLLISION_X_MASK 1'0
parameter \ARST_VALUE 8'00000000
parameter \SRST_VALUE 8'00000000
parameter \INIT_VALUE 8'00000000
parameter \ARST_VALUE 8'xxxxxxxx
parameter \SRST_VALUE 8'xxxxxxxx
parameter \INIT_VALUE 8'xxxxxxxx
parameter \CE_OVER_SRST 0
parameter \CLK_ENABLE 1
parameter \CLK_POLARITY 1
Expand Down

0 comments on commit 1d2b9c3

Please sign in to comment.