Repository: intel/rohd (dart)

Issue:
A mechanism to prevent merging of certain module definitions
### Motivation

Sometimes it is desirable to create a variety of modules with specific definition names, even if some of those definitions have exactly the same contents.  Currently, if you use `reserveDefinitionName` ROHD will flag an error if it's unable to keep them unique.  It would be nice to have a way to instead just duplicate the module definition.

### Desired solution

Some sort of flag (perhaps reusing `reserveDefinitionName`?) that tells the synthesizer not to de-duplicate that module definition in output code.

### Alternatives considered

Current workarounds involve adding some sort of thing to make the code unique such as:
- a uniquely named interface port
- a uniquely named module instance

### Additional details

_No response_

Pull request:
<!-- Please make sure you check out the contribution guidelines before submitting a pull request! -->

## Description & Motivation

This PR fixes some more naming issues discovered during customer testing after #439.
- Make internal signals use an ordered Set to avoid improper duplication of generated SV module definitions.
- Make `Port` mergeable to avoid calls to `connectIO` from duplicating signal names in generated SV unnecessarily.
- Prevent `reserveDefinitionName` Modules from being merged if they have different definitionNames.
- Disable link validation in dart doc for documentation deployment due to dartdoc bug (https://github.com/dart-lang/dartdoc/issues/3584).
- Update doc link to icarus verilog

## Related Issue(s)

Fix #345

## Testing

Added tests to fix bugs and cover new functionality.

## Backwards-compatibility

> Is this a breaking change that will not be backwards-compatible? If yes, how so?

No, though generated SV will change

## Documentation

> Does the change require any updates to documentation? If so, where? Are they included?

No

Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 1cfb3b6f0..cb333f2a0 100644
--- a/CONTRIBUTING.md
+++ b/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
 
diff --git a/lib/src/module.dart b/lib/src/module.dart
index 3e2c13d5e..f8f480406 100644
--- a/lib/src/module.dart
+++ b/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 = {};
diff --git a/lib/src/signals/port.dart b/lib/src/signals/port.dart
index 1f76978ae..2cd9d8d3a 100644
--- a/lib/src/signals/port.dart
+++ b/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);
diff --git a/lib/src/synthesizers/synthesis_result.dart b/lib/src/synthesizers/synthesis_result.dart
index c13450e68..70bc48dfb 100644
--- a/lib/src/synthesizers/synthesis_result.dart
+++ b/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;
diff --git a/tool/gh_actions/generate_documentation.sh b/tool/gh_actions/generate_documentation.sh
index 9cecffa74..94cbe6f52 100755
--- a/tool/gh_actions/generate_documentation.sh
+++ b/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}"
 


Test patch:
diff --git a/test/counter_wintf_test.dart b/test/counter_wintf_test.dart
index 0823c9494..bf0f80c52 100644
--- a/test/counter_wintf_test.dart
+++ b/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);
+  });
 }
diff --git a/test/module_merging_test.dart b/test/module_merging_test.dart
new file mode 100644
index 000000000..5a5590ce9
--- /dev/null
+++ b/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 <max.korbel@intel.com>
+
+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'));
+  });
+}
diff --git a/test/tree_test.dart b/test/tree_test.dart
index 4105cee67..2bfd392a5 100644
--- a/test/tree_test.dart
+++ b/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({