Signal naming improvements (#439)

Author: mkorbel1

doc/architecture.md

@@ -16,7 +16,7 @@ A `LogicValue` represents a multi-bit (including 0-bit and 1-bit) 4-value (`1`,
 
 The `Module` is the fundamental building block of hardware designs in ROHD. They have clearly defined inputs and outputs, and all logic contained within the module should connect either/both from inputs and to outputs. The ROHD framework will determine at `build()` time which logic sits within which `Module`. Any functional operation, whether a simple gate or a large module, is implemented as a `Module`.
 
-Every `Module` defines its own functionality. This could be through composition of other `Module`s, or through custom functional definition. For a custom functionality to be convertable to an output (e.g. SystemVerilog), it has to explicitly define how to convert it (via `CustomVerilog` or `InlineVerilog`). Any time the input of a custom functionality `Module` toggles, the outputs should correspondingly change, if necessary.
+Every `Module` defines its own functionality. This could be through composition of other `Module`s, or through custom functional definition. For a custom functionality to be convertable to an output (e.g. SystemVerilog), it has to explicitly define how to convert it (via `CustomVerilog` or `InlineSystemVerilog`). Any time the input of a custom functionality `Module` toggles, the outputs should correspondingly change, if necessary.
 
 ### Simulator
 

doc/user_guide/_docs/A21-generation.md

@@ -0,0 +1,59 @@
+---
+title: "Generating Outputs"
+permalink: /docs/generation/
+last_modified_at: 2023-11-13
+toc: true
+---
+
+Hardware in ROHD is convertible to an output format via `Synthesizer`s, the most popular of which is SystemVerilog. Hardware in ROHD can be converted to logically equivalent, human readable SystemVerilog with structure, hierarchy, ports, and names maintained.
+
+The simplest way to generate SystemVerilog is with the helper method `generateSynth` in `Module`:
+
+```dart
+void main() async {
+    final myModule = MyModule();
+    
+    // remember that `build` returns a `Future`, hence the `await` here
+    await myModule.build();
+
+    final generatedSv = myModule.generateSynth();
+
+    // you can print it out...
+    print(generatedSv);
+
+    // or write it to a file
+    File('myHardware.sv').writeAsStringSync(generatedSv);
+}
+```
+
+The `generateSynth` function will return a `String` with the SystemVerilog `module` definitions for the top-level it is called on, as well as any sub-modules (recursively).  You can dump the entire contents to a file and use it anywhere you would any other SystemVerilog.
+
+## Controlling naming
+
+### Modules
+
+Port names are always maintained exactly in generated SystemVerilog, so they must always be unique and sanitary (valid) SystemVerilog.
+
+`Module`s have two names:
+
+- The `definitionName`, which maps to the name of the module declaration in SystemVerilog.
+  - If you want to ensure this does not change (e.g. uniquified because multiple different declarations have the same `definitionname`), set `reserveDefinitionName` to `true`.
+- The `name`, which maps to the instance name when that instance is instanitated as a sub-module of another module.
+  - If you want to ensure this does not change (e.g. uniquified because other signals or sub-modules would have the same name), then set `reserveName` to `true`.
+
+### Internal signals
+
+Internal signals, unlike ports, don't need to always have the same exact name as in the original hardware definition.
+
+- If you do not name a signal, it will get a default name.  Generated code will attempt to avoid keeping that intermediate signal around (declared) if possible.
+- If you do name a signal, by default it will be characterized as `renameable`.  This means it will try to keep that name in generated output, but may rename it for uniqification purposes.
+- If you want to make sure an internal signal maintains exactly the name you want, you can mark it explicitly with `reserved`.
+- You can downgrade a named signal as well to `mergeable` or even `unnamed`, if you care less about it's name in generated outputs and prefer that others will take over.
+
+### Unpreferred names
+
+The `Naming.unpreferredName` function will modify a signal name to indicate to downstream flows that the name is preferably omitted from the output, but preferable to an unnamed signal. This is generally most useful for things like output ports of `InlineSystemVerilog` modules.
+
+## More advanced generation
+
+Under the hood of `generateSynth`, it's actually using a [`SynthBuilder`](https://intel.github.io/rohd/rohd/SynthBuilder-class.html) which accepts a `Module` and a `Synthesizer` (usually a `SystemVerilogSynthesizer`) as arguments.  This `SynthBuilder` can provide a collection of `String` file contents via `getFileContents`, or you can ask for the full set of `synthesisResults`, which contains `SynthesisResult`s which can each be converted `toFileContents` but also has context about the `module` it refers to, the `instanceTypeName`, etc. With these APIs, you can easily generate named files, add file headers, ignore generation of some modules, generate file lists for other tools, etc.

lib/rohd.dart

@@ -11,5 +11,6 @@ export 'src/signals/signals.dart';
 export 'src/simulator.dart';
 export 'src/swizzle.dart';
 export 'src/synthesizers/synthesizers.dart';
+export 'src/utilities/naming.dart';
 export 'src/values/values.dart';
 export 'src/wave_dumper.dart';

lib/src/exceptions/name/name_exceptions.dart

@@ -5,3 +5,4 @@ export 'empty_reserved_name_exception.dart';
 export 'invalid_portname_exceptions.dart';
 export 'invalid_reserved_name_exception.dart';
 export 'null_reserved_name_exception.dart';
+export 'unavailable_reserved_name_exception.dart';

lib/src/exceptions/name/unavailable_reserved_name_exception.dart

@@ -0,0 +1,23 @@
+// Copyright (C) 2023 Intel Corporation
+// SPDX-License-Identifier: BSD-3-Clause
+//
+// unavailable_reserved_name_exception.dart
+// An exception that thrown when a reserved name can't be acquired.
+//
+// 2023 November 3
+// Author: Max Korbel <[email protected]>
+
+import 'package:rohd/rohd.dart';
+
+/// An exception that thrown when a reserved name cannot be acquired.
+class UnavailableReservedNameException extends RohdException {
+  /// Constructs an error indicating that the reserved [name] could not be
+  /// acquired.
+  UnavailableReservedNameException(String name)
+      : this.withMessage('Unable to use reserved name "$name" '
+            'because something else already has this name.');
+
+  /// Constructs an error indicating that the reserved `name` could not be
+  /// acquired with a [message] explaining why.
+  UnavailableReservedNameException.withMessage(super.message);
+}

lib/src/module.dart

@@ -41,10 +41,10 @@ abstract class Module {
   final Set<Logic> _internalSignals = HashSet<Logic>();
 
   /// An internal list of inputs to this [Module].
-  final Map<String, Logic> _inputs = HashMap<String, Logic>();
+  final Map<String, Logic> _inputs = {};
 
   /// An internal list of outputs to this [Module].
-  final Map<String, Logic> _outputs = HashMap<String, Logic>();
+  final Map<String, Logic> _outputs = {};
 
   /// The parent [Module] of this [Module].
   ///
@@ -141,23 +141,6 @@ abstract class Module {
           '  Call build() before accessing this.');
   String _uniqueInstanceName;
 
-  /// Return string type definition name if validation passed
-  /// else throw exception.
-  ///
-  /// This validation method ensure that [name] is valid if
-  /// [reserveName] set to `true`.
-  static String? _nameValidation(String? name, bool reserveName) {
-    if (reserveName && name == null) {
-      throw NullReservedNameException();
-    } else if (reserveName && name!.isEmpty) {
-      throw EmptyReservedNameException();
-    } else if (reserveName && !Sanitizer.isSanitary(name!)) {
-      throw InvalidReservedNameException();
-    } else {
-      return name;
-    }
-  }
-
   /// If true, guarantees [uniqueInstanceName] matches [name] or else the
   /// [build] will fail.
   final bool reserveName;
@@ -192,9 +175,10 @@ abstract class Module {
       this.reserveName = false,
       String? definitionName,
       this.reserveDefinitionName = false})
-      : _uniqueInstanceName = _nameValidation(name, reserveName) ?? name,
-        _definitionName =
-            _nameValidation(definitionName, reserveDefinitionName);
+      : _uniqueInstanceName =
+            Naming.validatedName(name, reserveName: reserveName) ?? name,
+        _definitionName = Naming.validatedName(definitionName,
+            reserveName: reserveDefinitionName);
 
   /// Returns an [Iterable] of [Module]s representing the hierarchical path to
   /// this [Module].
@@ -286,9 +270,6 @@ abstract class Module {
     await module.build();
   }
 
-  /// A prefix to add to the beginning of any port name that is "unpreferred".
-  static String get _unpreferredPrefix => '_';
-
   /// Makes a signal name "unpreferred" when considering between multiple
   /// possible signal names.
   ///
@@ -299,13 +280,15 @@ abstract class Module {
   /// choose the other one for the final signal name.  Marking signals as
   /// "unpreferred" can have the effect of making generated output easier to
   /// read.
+  @Deprecated('Use `Naming.unpreferredName` or `Logic.naming` instead.')
   @protected
-  static String unpreferredName(String name) => _unpreferredPrefix + name;
+  static String unpreferredName(String name) => Naming.unpreferredName(name);
 
   /// Returns true iff the signal name is "unpreferred".
   ///
   /// See documentation for [unpreferredName] for more details.
-  static bool isUnpreferred(String name) => name.startsWith(_unpreferredPrefix);
+  @Deprecated('Use `Naming.isUnpreferred` or `Logic.naming` instead.')
+  static bool isUnpreferred(String name) => Naming.isUnpreferred(name);
 
   /// Searches for [Logic]s and [Module]s within this [Module] from its inputs.
   Future<void> _traceInputForModuleContents(Logic signal,
@@ -455,12 +438,11 @@ abstract class Module {
 
   /// Checks whether a port name is safe to add (e.g. no duplicates).
   void _checkForSafePortName(String name) {
-    if (!Sanitizer.isSanitary(name)) {
-      throw InvalidPortNameException(name);
-    }
+    Naming.validatedName(name, reserveName: true);
 
     if (outputs.containsKey(name) || inputs.containsKey(name)) {
-      throw Exception('Already defined a port with name "$name".');
+      throw UnavailableReservedNameException.withMessage(
+          'Already defined a port with name "$name".');
     }
   }
 
@@ -480,7 +462,7 @@ abstract class Module {
       x = x.packed;
     }
 
-    final inPort = Port(name, width)
+    final inPort = Logic(name: name, width: width, naming: Naming.reserved)
       ..gets(x)
       // ignore: invalid_use_of_protected_member
       ..parentModule = this;
@@ -508,8 +490,13 @@ abstract class Module {
   }) {
     _checkForSafePortName(name);
 
-    final inArr = LogicArray(dimensions, elementWidth,
-        name: name, numUnpackedDimensions: numUnpackedDimensions)
+    final inArr = LogicArray(
+      name: name,
+      dimensions,
+      elementWidth,
+      numUnpackedDimensions: numUnpackedDimensions,
+      naming: Naming.reserved,
+    )
       ..gets(x)
       // ignore: invalid_use_of_protected_member
       ..parentModule = this;
@@ -527,7 +514,7 @@ abstract class Module {
   Logic addOutput(String name, {int width = 1}) {
     _checkForSafePortName(name);
 
-    final outPort = Port(name, width)
+    final outPort = Logic(name: name, width: width, naming: Naming.reserved)
       // ignore: invalid_use_of_protected_member
       ..parentModule = this;
 
@@ -550,8 +537,13 @@ abstract class Module {
   }) {
     _checkForSafePortName(name);
 
-    final outArr = LogicArray(dimensions, elementWidth,
-        name: name, numUnpackedDimensions: numUnpackedDimensions)
+    final outArr = LogicArray(
+      name: name,
+      dimensions,
+      elementWidth,
+      numUnpackedDimensions: numUnpackedDimensions,
+      naming: Naming.reserved,
+    )
       // ignore: invalid_use_of_protected_member
       ..parentModule = this;
 

lib/src/modules/bus.dart

@@ -32,6 +32,9 @@ class BusSubset extends Module with InlineSystemVerilog {
   /// End index of the subset.
   final int endIndex;
 
+  @override
+  List<String> get expressionlessInputs => [_original];
+
   /// Constructs a [Module] that accesses a subset from [bus] which ranges
   /// from [startIndex] to [endIndex] (inclusive of both).
   /// When, [bus] has a width of '1', [startIndex] and [endIndex] are ignored
@@ -52,13 +55,9 @@ class BusSubset extends Module with InlineSystemVerilog {
           ' than ${bus.width}');
     }
 
-    // original name can't be unpreferred because you cannot do a bit slice
-    // on expressions in SystemVerilog, and other expressions could have
-    // been in-lined
-    _original = 'original_${bus.name}';
-
+    _original = Naming.unpreferredName('original_${bus.name}');
     _subset =
-        Module.unpreferredName('subset_${endIndex}_${startIndex}_${bus.name}');
+        Naming.unpreferredName('subset_${endIndex}_${startIndex}_${bus.name}');
 
     addInput(_original, bus, width: bus.width);
     final newWidth = (endIndex - startIndex).abs() + 1;
@@ -92,13 +91,19 @@ class BusSubset extends Module with InlineSystemVerilog {
     }
   }
 
+  /// A regular expression that will have matches if an expression is included.
+  static final RegExp _expressionRegex = RegExp("[()']");
+
   @override
   String inlineVerilog(Map<String, String> inputs) {
     if (inputs.length != 1) {
       throw Exception('BusSubset has exactly one input, but saw $inputs.');
     }
     final a = inputs[_original]!;
 
+    assert(!a.contains(_expressionRegex),
+        'Inputs to bus swizzle cannot contain any expressions.');
+
     // When, input width is 1, ignore startIndex and endIndex
     if (original.width == 1) {
       return a;
@@ -127,7 +132,7 @@ class BusSubset extends Module with InlineSystemVerilog {
 /// You can use convenience functions [swizzle()] or [rswizzle()] to more easily
 /// use this [Module].
 class Swizzle extends Module with InlineSystemVerilog {
-  final String _out = Module.unpreferredName('swizzled');
+  final String _out = Naming.unpreferredName('swizzled');
 
   /// The output port containing concatenated signals.
   late final Logic out = output(_out);
@@ -140,7 +145,7 @@ class Swizzle extends Module with InlineSystemVerilog {
     var outputWidth = 0;
     for (final signal in signals.reversed) {
       //reverse so bit 0 is the last thing in the input list
-      final inputName = Module.unpreferredName('in${idx++}');
+      final inputName = Naming.unpreferredName('in${idx++}');
       _swizzleInputs.add(
         addInput(inputName, signal, width: signal.width),
       );

lib/src/modules/conditional.dart

@@ -112,7 +112,7 @@ abstract class _Always extends Module with CustomSystemVerilog {
       for (final driver in conditional.drivers) {
         if (!_assignedDriverToInputMap.containsKey(driver)) {
           final inputName = _portUniquifier.getUniqueName(
-              initialName: Module.unpreferredName(
+              initialName: Naming.unpreferredName(
                   Sanitizer.sanitizeSV('in${idx}_${driver.name}')));
           addInput(inputName, driver, width: driver.width);
           _assignedDriverToInputMap[driver] = input(inputName);
@@ -122,7 +122,7 @@ abstract class _Always extends Module with CustomSystemVerilog {
       for (final receiver in conditional.receivers) {
         if (!_assignedReceiverToOutputMap.containsKey(receiver)) {
           final outputName = _portUniquifier.getUniqueName(
-              initialName: Module.unpreferredName(
+              initialName: Naming.unpreferredName(
                   Sanitizer.sanitizeSV('out${idx}_${receiver.name}')));
           addOutput(outputName, width: receiver.width);
           _assignedReceiverToOutputMap[receiver] = output(outputName);
@@ -437,7 +437,7 @@ class Sequential extends _Always {
       _clks.add(addInput(
           _portUniquifier.getUniqueName(
               initialName: Sanitizer.sanitizeSV(
-                  Module.unpreferredName('clk${i}_${clk.name}'))),
+                  Naming.unpreferredName('clk${i}_${clk.name}'))),
           clk));
       _preTickClkValues.add(null);
     }
@@ -1612,22 +1612,22 @@ Logic flop(
 /// Represents a single flip-flop with no reset.
 class FlipFlop extends Module with CustomSystemVerilog {
   /// Name for the enable input of this flop
-  final String _enName = Module.unpreferredName('en');
+  final String _enName = Naming.unpreferredName('en');
 
   /// Name for the clk of this flop.
-  final String _clkName = Module.unpreferredName('clk');
+  final String _clkName = Naming.unpreferredName('clk');
 
   /// Name for the input of this flop.
-  final String _dName = Module.unpreferredName('d');
+  final String _dName = Naming.unpreferredName('d');
 
   /// Name for the output of this flop.
-  final String _qName = Module.unpreferredName('q');
+  final String _qName = Naming.unpreferredName('q');
 
   /// Name for the reset of this flop.
-  final String _resetName = Module.unpreferredName('reset');
+  final String _resetName = Naming.unpreferredName('reset');
 
   /// Name for the reset value of this flop.
-  final String _resetValueName = Module.unpreferredName('resetValue');
+  final String _resetValueName = Naming.unpreferredName('resetValue');
 
   /// The clock, posedge triggered.
   late final Logic _clk = input(_clkName);