From b02beda5c9ad40e2f9782b4dd82e4a9a8885a1dc Mon Sep 17 00:00:00 2001 From: Max Korbel Date: Wed, 29 Nov 2023 12:21:14 -0800 Subject: [PATCH] More module and signal naming improvements (#440) --- CONTRIBUTING.md | 2 +- lib/src/module.dart | 2 +- lib/src/signals/port.dart | 4 + lib/src/synthesizers/synthesis_result.dart | 6 +- test/counter_wintf_test.dart | 9 ++ test/module_merging_test.dart | 107 +++++++++++++++++++++ test/tree_test.dart | 1 - tool/gh_actions/generate_documentation.sh | 5 +- 8 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 test/module_merging_test.dart 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: -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: +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: ### 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 _internalSignals = HashSet(); + final Set _internalSignals = {}; /// An internal list of inputs to this [Module]. final Map _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/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 + +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.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({ 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}"