More module and signal naming improvements (#440)

Author: mkorbel1

CONTRIBUTING.md

@@ -42,7 +42,7 @@ The [ROHD Forum](https://intel.github.io/rohd-website/forum/rohd-forum/) is a pe
 
 You must have [Dart](https://dart.dev/) installed on your system to use ROHD. You can find detailed instructions for how to install Dart here: <https://dart.dev/get-dart>
 
-To run the complete ROHD test suite for development, you need to install [Icarus Verilog](http://iverilog.icarus.com/). It is used to compare SystemVerilog functionality with the ROHD simulator functionality. Installation instructions are available here: <https://iverilog.fandom.com/wiki/Installation_Guide>
+To run the complete ROHD test suite for development, you need to install [Icarus Verilog](https://steveicarus.github.io/iverilog/). It is used to compare SystemVerilog functionality with the ROHD simulator functionality. Installation instructions are available here: <https://iverilog.fandom.com/wiki/Installation_Guide>
 
 ### Setup Recommendations
 

lib/src/module.dart

@@ -38,7 +38,7 @@ abstract class Module {
   /// An internal list of internal-signals.
   ///
   /// Used for waveform dump efficiency.
-  final Set<Logic> _internalSignals = HashSet<Logic>();
+  final Set<Logic> _internalSignals = {};
 
   /// An internal list of inputs to this [Module].
   final Map<String, Logic> _inputs = {};

lib/src/signals/port.dart

@@ -21,6 +21,10 @@ class Port extends Logic {
       : super(
           name: name,
           width: width,
+
+          // make port names mergeable so we don't duplicate the ports
+          // when calling connectIO
+          naming: Naming.mergeable,
         ) {
     if (!Sanitizer.isSanitary(name)) {
       throw InvalidPortNameException(name);

lib/src/synthesizers/synthesis_result.dart

@@ -40,7 +40,11 @@ abstract class SynthesisResult {
 
   @override
   bool operator ==(Object other) =>
-      other is SynthesisResult && matchesImplementation(other);
+      other is SynthesisResult &&
+      matchesImplementation(other) &&
+      // if they are both reserved defs but different def names, not equal
+      !((module.reserveDefinitionName && other.module.reserveDefinitionName) &&
+          module.definitionName != other.module.definitionName);
 
   @override
   int get hashCode => matchHashCode;

test/counter_wintf_test.dart

@@ -106,6 +106,7 @@ void main() {
       await moduleTest(mod);
     });
   });
+
   test('resetFlipflop from root w/o resetVal', () async {
     final mod = Counter(CounterInterface(8), useBuiltInSequentialReset: true);
     await moduleTest(mod);
@@ -133,4 +134,12 @@ void main() {
     final simResult = SimCompare.iverilogVector(mod, vectors);
     expect(simResult, equals(true));
   });
+
+  test('interface ports dont get doubled up', () async {
+    final mod = Counter(CounterInterface(8));
+    await mod.build();
+    final sv = mod.generateSynth();
+
+    expect(!sv.contains('en_0'), true);
+  });
 }

test/module_merging_test.dart

@@ -0,0 +1,107 @@
+// Copyright (C) 2023 Intel Corporation
+// SPDX-License-Identifier: BSD-3-Clause
+//
+// module_merging_test.dart
+// Unit tests for deduplication of module definitions in generated verilog
+//
+// 2023 November 28
+// Author: Max Korbel <[email protected]>
+
+import 'package:rohd/rohd.dart';
+import 'package:test/test.dart';
+
+class ComplicatedLeaf extends Module {
+  Logic get d => output('d');
+  ComplicatedLeaf(
+    Logic clk,
+    Logic reset, {
+    required Logic a,
+    required Logic b,
+    required Logic c,
+  }) {
+    clk = addInput('clk', clk);
+    reset = addInput('reset', reset);
+    a = addInput('a', a);
+    b = addInput('b', b);
+    c = addInput('c', c);
+
+    final internal1 = Logic(name: 'internal1');
+    final internal2 = Logic(name: 'internal2');
+
+    addOutput('d');
+
+    Combinational.ssa((s) => [
+          s(internal1) < internal2,
+          If(a, then: [
+            internal1.incr(s: s),
+          ]),
+          If(b, then: [
+            internal1.decr(s: s),
+          ]),
+          If(c, then: [
+            s(internal1) < 0,
+          ])
+        ]);
+
+    Sequential(clk, reset: reset, [
+      internal2 < internal1,
+    ]);
+  }
+}
+
+class TrunkWithLeaves extends Module {
+  TrunkWithLeaves(
+    Logic clk,
+    Logic reset,
+  ) {
+    clk = addInput('clk', clk);
+    reset = addInput('reset', reset);
+
+    final abc = [Logic(name: 'a'), Logic(name: 'b'), Logic(name: 'c')];
+    for (var i = 0; i < 50; i++) {
+      ComplicatedLeaf(
+        clk,
+        reset,
+        a: abc[i % 3],
+        b: abc[(i + 1) % 3],
+        c: abc[(i + 2) % 3],
+      );
+    }
+  }
+}
+
+class SpecificallyDefinedNameModule extends Module {
+  SpecificallyDefinedNameModule(Logic a, {required super.definitionName})
+      : super(reserveDefinitionName: true) {
+    a = addInput('a', a);
+    addOutput('b') <= ~a;
+  }
+}
+
+class ParentOfDifferentModuleDefNames extends Module {
+  ParentOfDifferentModuleDefNames(Logic a) {
+    a = addInput('a', a);
+    SpecificallyDefinedNameModule(a, definitionName: 'def1');
+    SpecificallyDefinedNameModule(a, definitionName: 'def2');
+  }
+}
+
+void main() async {
+  test('complex trunk with leaves doesnt duplicate identical modules',
+      () async {
+    final dut = TrunkWithLeaves(Logic(), Logic());
+    await dut.build();
+    final sv = dut.generateSynth();
+
+    expect('module ComplicatedLeaf'.allMatches(sv).length, 1);
+  });
+
+  test('different reserved definition name modules stay separate', () async {
+    final dut = ParentOfDifferentModuleDefNames(Logic());
+    await dut.build();
+    final sv = dut.generateSynth();
+
+    expect(sv, contains('module def1'));
+    expect(sv, contains('module def2'));
+  });
+}

test/tree_test.dart

@@ -53,7 +53,6 @@ void main() {
           List<Logic>.generate(16, (index) => Logic(width: 8)),
           (a, b) => mux(a > b, a, b));
       await mod.build();
-      // File('tmp_tree.sv').writeAsStringSync(mod.generateSynth());
 
       final vectors = [
         Vector({

tool/gh_actions/generate_documentation.sh

@@ -14,7 +14,10 @@ set -euo pipefail
 # See script "check_documentation.sh" for a note on processing "dart doc" output.
 
 # The documentation will be placed in the "doc/api" folder.
-output=$(dart doc --validate-links 2>&1 | tee)
+
+# Disabling --validate-links due to https://github.com/dart-lang/dartdoc/issues/3584
+# output=$(dart doc --validate-links 2>&1 | tee)
+output=$(dart doc 2>&1 | tee)
 
 echo "${output}"