From: whitequark <whitequark@whitequark.org>
Date: Tue, 2 Jul 2019 18:05:34 +0000 (+0000)
Subject: back.rtlil: do not emit $next wires for comb signals.
X-Git-Tag: locally_working~129
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=dd5e513e4266dfdfbb5c8eca03aad4d2bcd0a745;p=nmigen.git

back.rtlil: do not emit $next wires for comb signals.

According to RTLIL semantics (that was undocumented before today),
the only purpose of `sync always` is to enable inference of latches,
because there is no other way to express them in terms of RTLIL
processes without ending up with a combinatorial loop. But, nMigen
specifically avoids latches, so this is not necessary.

This change results in major improvements in Verilog readability.

See also #98.
---

diff --git a/nmigen/back/rtlil.py b/nmigen/back/rtlil.py
index 602d1c3..f09dda6 100644
--- a/nmigen/back/rtlil.py
+++ b/nmigen/back/rtlil.py
@@ -269,7 +269,7 @@ class _ValueCompilerState:
         wire_curr = self.rtlil.wire(width=signal.nbits, name=wire_name,
                                     port_id=port_id, port_kind=port_kind,
                                     src=src(signal.src_loc))
-        if signal in self.driven:
+        if signal in self.driven and self.driven[signal]:
             wire_next = self.rtlil.wire(width=signal.nbits, name="$next" + wire_curr,
                                         src=src(signal.src_loc))
         else:
@@ -559,10 +559,10 @@ class _LHSValueCompiler(_ValueCompiler):
         return self(value)
 
     def on_Signal(self, value):
-        wire_curr, wire_next = self.s.resolve(value)
-        if wire_next is None:
+        if value not in self.s.driven:
             raise ValueError("No LHS wire for non-driven signal {}".format(repr(value)))
-        return wire_next
+        wire_curr, wire_next = self.s.resolve(value)
+        return wire_next or wire_curr
 
     def _prepare_value_for_Slice(self, value):
         assert isinstance(value, (ast.Signal, ast.Slice, ast.Cat, rec.Record))
@@ -824,23 +824,24 @@ def convert_fragment(builder, fragment, hierarchy):
                         sync.update(verilog_trigger, "1'0")
                         verilog_trigger_sync_emitted = True
 
-                # For every signal in every domain, assign \sig to $next\sig. The sensitivity list,
-                # however, differs between domains: for comb domains, it is `always`, for sync
-                # domains with sync reset, it is `posedge clk`, for sync domains with async reset
-                # it is `posedge clk or posedge rst`.
+                # For every signal in every sync domain, assign \sig to $next\sig. The sensitivity
+                # list, however, differs between domains: for domains with sync reset, it is
+                # `posedge clk`, for sync domains with async reset it is `posedge clk or
+                # posedge rst`.
                 for domain, signals in fragment.drivers.items():
+                    if domain is None:
+                        continue
+
                     signals = signals & group_signals
                     if not signals:
                         continue
 
+                    cd = fragment.domains[domain]
+
                     triggers = []
-                    if domain is None:
-                        triggers.append(("always",))
-                    else:
-                        cd = fragment.domains[domain]
-                        triggers.append(("posedge", compiler_state.resolve_curr(cd.clk)))
-                        if cd.async_reset:
-                            triggers.append(("posedge", compiler_state.resolve_curr(cd.rst)))
+                    triggers.append(("posedge", compiler_state.resolve_curr(cd.clk)))
+                    if cd.async_reset:
+                        triggers.append(("posedge", compiler_state.resolve_curr(cd.rst)))
 
                     for trigger in triggers:
                         with process.sync(*trigger) as sync: