Skip to content

Allow Signals to be privately named using name="" #1234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions amaranth/hdl/_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -1884,7 +1884,8 @@ class Signal(Value, DUID, metaclass=_SignalMeta):
If not specified, ``shape`` defaults to 1-bit and non-signed.
name : str
Name hint for this signal. If ``None`` (default) the name is inferred from the variable
name this ``Signal`` is assigned to.
name this ``Signal`` is assigned to. If the empty string, then this ``Signal`` is treated
as private and is generally hidden from view.
init : int or integral Enum
Reset (synchronous) or default (combinatorial) value.
When this ``Signal`` is assigned to in synchronous context and the corresponding clock
Expand Down Expand Up @@ -1920,7 +1921,10 @@ def __init__(self, shape=None, *, name=None, init=None, reset=None, reset_less=F

if name is not None and not isinstance(name, str):
raise TypeError(f"Name must be a string, not {name!r}")
self.name = name or tracer.get_var_name(depth=2 + src_loc_at, default="$signal")
if name is None:
self.name = tracer.get_var_name(depth=2 + src_loc_at, default="$signal")
else:
self.name = name

orig_shape = shape
if shape is None:
Expand Down Expand Up @@ -2102,7 +2106,10 @@ def _rhs_signals(self):
return SignalSet((self,))

def __repr__(self):
return f"(sig {self.name})"
if self.name != "":
return f"(sig {self.name})"
else:
return "(sig)"


@final
Expand Down
6 changes: 3 additions & 3 deletions amaranth/hdl/_dsl.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def ongoing(self, name):
if name not in self.encoding:
self.encoding[name] = len(self.encoding)
fsm_name = self._data["name"]
self._data["ongoing"][name] = Signal(name=f"{fsm_name}_ongoing_{name}")
self._data["ongoing"][name] = Signal(name="")
return self._data["ongoing"][name]


Expand Down Expand Up @@ -462,7 +462,7 @@ def State(self, name):
if name not in fsm_data["encoding"]:
fsm_name = fsm_data["name"]
fsm_data["encoding"][name] = len(fsm_data["encoding"])
fsm_data["ongoing"][name] = Signal(name=f"{fsm_name}_ongoing_{name}")
fsm_data["ongoing"][name] = Signal(name="")
try:
_outer_case, self._statements = self._statements, {}
self._ctrl_context = None
Expand All @@ -486,7 +486,7 @@ def next(self, name):
if name not in ctrl_data["encoding"]:
fsm_name = ctrl_data["name"]
ctrl_data["encoding"][name] = len(ctrl_data["encoding"])
ctrl_data["ongoing"][name] = Signal(name=f"{fsm_name}_ongoing_{name}")
ctrl_data["ongoing"][name] = Signal(name="")
self._add_statement(
assigns=[FSMNextStatement(ctrl_data, name)],
domain=ctrl_data["domain"],
Expand Down
4 changes: 3 additions & 1 deletion amaranth/hdl/_ir.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,8 @@ def _assign_port_names(self):
assigned_names = {name for name, conn, dir in self.ports if name is not None}
for name, conn, dir in self.ports:
if name is None:
if conn.name == "": # Nothing to name this port!
raise TypeError("Signals with private names cannot be used in unnamed top-level ports")
name = _add_name(assigned_names, conn.name)
assigned_names.add(name)
new_ports.append((name, conn, dir))
Expand Down Expand Up @@ -590,7 +592,7 @@ def _assign_names(self, fragment: Fragment, hierarchy: "tuple[str]"):
frag_info.io_port_names[conn] = name

for signal in frag_info.used_signals:
if signal not in frag_info.signal_names:
if signal not in frag_info.signal_names and signal.name != "": # Private name shouldn't be added.
frag_info.signal_names[signal] = _add_name(frag_info.assigned_names, signal.name)
for port in frag_info.used_io_ports:
if port not in frag_info.io_port_names:
Expand Down
11 changes: 11 additions & 0 deletions amaranth/sim/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from .._utils import deprecated
from ..hdl._cd import *
from ..hdl._ir import *
from ..hdl._ast import Value, ValueLike
from ._base import BaseEngine


Expand Down Expand Up @@ -238,5 +239,15 @@ def write_vcd(self, vcd_file, gtkw_file=None, *, traces=(), fs_per_delta=0):
file.close()
raise ValueError("Cannot start writing waveforms after advancing simulation time")

for trace in traces:
if isinstance(trace, ValueLike):
trace_cast = Value.cast(trace)
for trace_signal in trace_cast._rhs_signals():
if trace_signal.name == "":
if trace_signal is trace:
raise TypeError("Cannot trace signal with private name")
else:
raise TypeError(f"Cannot trace signal with private name (within {trace!r})")

return self._engine.write_vcd(vcd_file=vcd_file, gtkw_file=gtkw_file,
traces=traces, fs_per_delta=fs_per_delta)
4 changes: 4 additions & 0 deletions tests/test_hdl_ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,8 @@ def test_name(self):
self.assertEqual(s1.name, "s1")
s2 = Signal(name="sig")
self.assertEqual(s2.name, "sig")
s3 = Signal(name="")
self.assertEqual(s3.name, "")

def test_init(self):
s1 = Signal(4, init=0b111, reset_less=True)
Expand Down Expand Up @@ -1294,6 +1296,8 @@ def test_attrs(self):
def test_repr(self):
s1 = Signal()
self.assertEqual(repr(s1), "(sig s1)")
s2 = Signal(name="")
self.assertEqual(repr(s2), "(sig)")

def test_like(self):
s1 = Signal.like(Signal(4))
Expand Down
23 changes: 11 additions & 12 deletions tests/test_hdl_dsl.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ def test_FSM_basic(self):
)
(case 1 )
)
(eq (sig fsm_ongoing_FIRST) (== (sig fsm_state) (const 1'd0)))
(eq (sig fsm_ongoing_SECOND) (== (sig fsm_state) (const 1'd1)))
(eq (sig) (== (sig fsm_state) (const 1'd0)))
(eq (sig) (== (sig fsm_state) (const 1'd1)))
)
""")
self.assertRepr(frag.statements["sync"], """
Expand All @@ -627,8 +627,7 @@ def test_FSM_basic(self):
"(sig a)": "comb",
"(sig fsm_state)": "sync",
"(sig b)": "sync",
"(sig fsm_ongoing_FIRST)": "comb",
"(sig fsm_ongoing_SECOND)": "comb",
"(sig)": "comb",
})
fsm = frag.find_generated("fsm")
self.assertIsInstance(fsm.state, Signal)
Expand Down Expand Up @@ -659,8 +658,8 @@ def test_FSM_init(self):
)
(case 1 )
)
(eq (sig fsm_ongoing_FIRST) (== (sig fsm_state) (const 1'd0)))
(eq (sig fsm_ongoing_SECOND) (== (sig fsm_state) (const 1'd1)))
(eq (sig) (== (sig fsm_state) (const 1'd0)))
(eq (sig) (== (sig fsm_state) (const 1'd1)))
)
""")
self.assertRepr(frag.statements["sync"], """
Expand Down Expand Up @@ -697,8 +696,8 @@ def test_FSM_reset(self):
)
(case 1 )
)
(eq (sig fsm_ongoing_FIRST) (== (sig fsm_state) (const 1'd0)))
(eq (sig fsm_ongoing_SECOND) (== (sig fsm_state) (const 1'd1)))
(eq (sig) (== (sig fsm_state) (const 1'd0)))
(eq (sig) (== (sig fsm_state) (const 1'd1)))
)
""")
self.assertRepr(frag.statements["sync"], """
Expand Down Expand Up @@ -743,10 +742,10 @@ def test_FSM_ongoing(self):
self.maxDiff = 10000
self.assertRepr(frag.statements["comb"], """
(
(eq (sig b) (sig fsm_ongoing_SECOND))
(eq (sig a) (sig fsm_ongoing_FIRST))
(eq (sig fsm_ongoing_SECOND) (== (sig fsm_state) (const 1'd0)))
(eq (sig fsm_ongoing_FIRST) (== (sig fsm_state) (const 1'd1)))
(eq (sig b) (sig))
(eq (sig a) (sig))
(eq (sig) (== (sig fsm_state) (const 1'd0)))
(eq (sig) (== (sig fsm_state) (const 1'd1)))
)
""")

Expand Down
14 changes: 12 additions & 2 deletions tests/test_hdl_ir.py
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,7 @@ def test_assign_names_to_signals(self):
o1 = Signal()
o2 = Signal()
o3 = Signal()
o4 = Signal(name="")
i1 = Signal(name="i")

f = Fragment()
Expand All @@ -980,6 +981,7 @@ def test_assign_names_to_signals(self):
"o1": (o1, PortDirection.Output),
"o2": (o2, PortDirection.Output),
"o3": (o3, PortDirection.Output),
"o4": (o4, PortDirection.Output),
}
design = f.prepare(ports)
self.assertEqual(design.fragments[design.fragment].signal_names, SignalDict([
Expand All @@ -988,12 +990,20 @@ def test_assign_names_to_signals(self):
(o1, "o1"),
(o2, "o2"),
(o3, "o3"),
# (o4, "o4"), # Signal has a private name.
(cd_sync.clk, "clk"),
(cd_sync.rst, "rst$6"),
(cd_sync.rst, "rst$7"),
(cd_sync_norst.clk, "sync_norst_clk"),
(i1, "i$7"),
(i1, "i$8"),
]))

def test_wrong_private_unnamed_toplevel_ports(self):
s = Signal(name="")
f = Fragment()
with self.assertRaisesRegex(TypeError,
r"^Signals with private names cannot be used in unnamed top-level ports$"):
Design(f, ports=((None, s, None),), hierarchy=("top",))

def test_assign_names_to_fragments(self):
f = Fragment()
f.add_subfragment(a := Fragment())
Expand Down
16 changes: 16 additions & 0 deletions tests/test_sim.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from amaranth.hdl._ir import *
from amaranth.sim import *
from amaranth.lib.memory import Memory
from amaranth.lib.data import View, StructLayout

from .utils import *
from amaranth._utils import _ignore_deprecated
Expand Down Expand Up @@ -1042,6 +1043,21 @@ def test_vcd_wrong_nonzero_time(self):
with sim.write_vcd(f):
pass

def test_vcd_private_signal(self):
sim = Simulator(Module())
with self.assertRaisesRegex(TypeError,
r"^Cannot trace signal with private name$"):
with open(os.path.devnull, "w") as f:
with sim.write_vcd(f, traces=(Signal(name=""),)):
pass

sim = Simulator(Module())
with self.assertRaisesRegex(TypeError,
r"^Cannot trace signal with private name \(within \(cat \(sig x\) \(sig\)\)\)$"):
with open(os.path.devnull, "w") as f:
with sim.write_vcd(f, traces=(Cat(Signal(name="x"), Signal(name="")),)):
pass

def test_no_negated_boolean_warning(self):
m = Module()
a = Signal()
Expand Down