Various features and testing for Logic and LogicValue (#121)

Author: mkorbel1

CHANGELOG.md

@@ -8,6 +8,12 @@
 - Fixed a bug related to zero-width construction of `LogicValue`s (https://github.com/intel/rohd/issues/90).
 - Fixed a bug where generated constants in SystemVerilog had no width, which can cause issues in some cases (e.g. swizzles) (https://github.com/intel/rohd/issues/89)
 - Added capability to convert binary strings to ints with underscore separators using `bin` (https://github.com/intel/rohd/issues/56).
+- Added `getRange` and `reversed` on `Logic` and `slice` on `LogicValue` to improve consistency.
+- Using `slice` in reverse-index order now reverses the order.
+- Added the ability to extend signals (e.g. `zeroExtend` and `signExtend`) on both `Logic` and `LogicValue` (https://github.com/intel/rohd/issues/101).
+- Improved flexibility of `IfBlock`.
+- Added `withSet` on `LogicValue` and `Logic` to make it easier to assign subsets of signals and values (https://github.com/intel/rohd/issues/101).
+- Fixed a bug where 0-bit signals would sometimes improperly generate 0-bit constants in generated SystemVerilog (https://github.com/intel/rohd/issues/122).
 
 ## 0.2.0
 - Updated implementation to avoid `Iterable.forEach` to make debug easier.

README.md

@@ -175,6 +175,12 @@ x.value.toInt()
 
 // a BigInt
 x.value.toBigInt()
+
+// constructing a LogicValue a handful of different ways
+LogicValue.ofString('0101xz01');                      // 0b0101xz01
+LogicValue.of([LogicValue.one, LogicValue.zero]);     // 0b10
+[LogicValue.z, LogicValue.x].swizzle();               // 0bzx
+LogicValue.ofInt(15, 4);                              // 0xf
 ```
 
 You can create `LogicValue`s using a variety of constructors including `ofInt`, `ofBigInt`, `filled` (like '0, '1, 'x, etc. in SystemVerilog), and `of` (which takes any `Iterable<LogicValue>`).
@@ -205,7 +211,8 @@ var x = Const(5, width:16);
 There is a convenience function for converting binary to an integer:
 ```dart
 // this is equvialent to and shorter than int.parse('010101', radix:2)
-bin('010101')
+// you can put underscores to help with readability, they are ignored
+bin('01_0101')
 ```
 
 ### Assignment
@@ -238,7 +245,7 @@ a_gte_b   <=  (a >= b) // greater than or equal NOTE: careful with order of oper
 ```
 
 ### Shift Operations
-Dart has [implemented the triple shift](https://github.com/dart-lang/language/blob/master/accepted/future-releases/triple-shift-operator/feature-specification.md) operator (>>>) in the opposite way as is [implemented in SystemVerilog](https://www.nandland.com/verilog/examples/example-shift-operator-verilog.html).  That is to say in Dart, >>> means *logical* shift right (fill with 0's), and >> means *arithmetic* shift right (maintaining sign).  ROHD keeps consistency with Dart's implementation to avoid introducing confusion within Dart code you write (whether ROHD or plain Dart).
+Dart has [implemented the triple shift](https://github.com/dart-lang/language/blob/master/accepted/2.14/triple-shift-operator/feature-specification.md) operator (>>>) in the opposite way as is [implemented in SystemVerilog](https://www.nandland.com/verilog/examples/example-shift-operator-verilog.html).  That is to say in Dart, >>> means *logical* shift right (fill with 0's), and >> means *arithmetic* shift right (maintaining sign).  ROHD keeps consistency with Dart's implementation to avoid introducing confusion within Dart code you write (whether ROHD or plain Dart).
 
 ```dart
 a << b    // logical shift left
@@ -247,7 +254,7 @@ a >>> b   // logical shift right
 ```
 
 ### Bus ranges and swizzling
-Multi-bit busses can be accessed by single bits and ranges or composed from multiple other signals.
+Multi-bit busses can be accessed by single bits and ranges or composed from multiple other signals.  Slicing, swizzling, etc. are also accessible on `LogicValue`s.
 ```dart
 var a = Logic(width:8),
     b = Logic(width:3),
@@ -271,7 +278,11 @@ e <= [d, c, b].swizzle();
 e <= [b, c, d].rswizzle();
 ```
 
-ROHD does not (currently) support assignment to a subset of a bus.  That is, you *cannot* do something like `e[3] <= d`.  If you need to build a bus from a collection of other signals, use swizzling.
+ROHD does not support assignment to a subset of a bus.  That is, you *cannot* do something like `e[3] <= d`.  Instead, you can use the `withSet` function to get a copy with that subset of the bus assigned to something else.  This applies for both `Logic` and `LogicValue`.  For example:
+```dart
+// reassign the variable `e` to a new `Logic` where bit 3 is set to `d`
+e = e.withSet(3, d);
+```
 
 ### Modules
 [`Module`](https://intel.github.io/rohd/rohd/Module-class.html)s are similar to modules in SystemVerilog.  They have inputs and outputs and logic that connects them.  There are a handful of rules that *must* be followed when implementing a module.
@@ -373,7 +384,7 @@ ROHD supports a variety of [`Conditional`](https://intel.github.io/rohd/rohd/Con
 
 Conditional statements are executed imperatively and in order, just like the contents of `always` blocks in SystemVerilog.  `_Always` blocks in ROHD map 1-to-1 with SystemVerilog `always` statements when converted.
 
-Assignments within an `_Always` should be executed conditionally, so use the `<` operator which creates a [`ConditionalAssign`](https://intel.github.io/rohd/rohd/ConditionalAssign-class.html) object instead of `<=`.
+Assignments within an `_Always` should be executed conditionally, so use the `<` operator which creates a [`ConditionalAssign`](https://intel.github.io/rohd/rohd/ConditionalAssign-class.html) object instead of `<=`.  The right hand side a `ConditionalAssign` can be anything that can be `put` onto a `Logic`, which includes `int`s.  If you're looking to fill the width of something, use `Const` with the `fill = true`.
 
 #### `If`
 Below is an example of an [`If`](https://intel.github.io/rohd/rohd/If-class.html) statement in ROHD:
@@ -390,7 +401,7 @@ Combinational([
       q < 13,
   ], orElse: [
       y < 0,
-      z < 1,
+      z < Const(1, width: 4, fill: true),
   ])])
 ]);
 ```

lib/src/logic.dart

@@ -174,7 +174,7 @@ class Logic {
   Module? get parentModule => _parentModule;
   Module? _parentModule;
 
-  /// Sets the value of [parentModule] to [newParent].
+  /// Sets the value of [parentModule] to [newParentModule].
   ///
   /// This should *only* be called by [Module.build()].  It is used to optimize search.
   @protected
@@ -426,7 +426,92 @@ class Logic {
   }
 
   /// Accesses a subset of this signal from [startIndex] to [endIndex], both inclusive.
+  ///
+  /// If [endIndex] is less than [startIndex], the returned value will be reversed relative
+  /// to the original signal.
   Logic slice(int endIndex, int startIndex) {
     return BusSubset(this, startIndex, endIndex).subset;
   }
+
+  /// Returns a version of this [Logic] with the bit order reversed.
+  Logic get reversed => slice(0, width - 1);
+
+  /// Returns a subset [Logic].  It is inclusive of [startIndex], exclusive of [endIndex].
+  ///
+  /// [startIndex] must be less than [endIndex]. If [startIndex] and [endIndex] are equal, then a
+  /// zero-width signal is returned.
+  Logic getRange(int startIndex, int endIndex) {
+    if (endIndex < startIndex) {
+      throw Exception(
+          'End ($endIndex) cannot be less than start ($startIndex).');
+    }
+    if (endIndex > width) {
+      throw Exception('End ($endIndex) must be less than width ($width).');
+    }
+    if (startIndex < 0) {
+      throw Exception(
+          'Start ($startIndex) must be greater than or equal to 0.');
+    }
+    if (endIndex == startIndex) {
+      return Const(0, width: 0);
+    }
+    return slice(endIndex - 1, startIndex);
+  }
+
+  /// Returns a new [Logic] with width [newWidth] where new bits added are zeros
+  /// as the most significant bits.
+  ///
+  /// The [newWidth] must be greater than or equal to the current width or an exception
+  /// will be thrown.
+  Logic zeroExtend(int newWidth) {
+    if (newWidth < width) {
+      throw Exception(
+          'New width $newWidth must be greater than or equal to width $width.');
+    }
+    return [
+      Const(0, width: newWidth - width),
+      this,
+    ].swizzle();
+  }
+
+  /// Returns a new [Logic] with width [newWidth] where new bits added are sign bits
+  /// as the most significant bits.  The sign is determined using two's complement, so
+  /// it takes the most significant bit of the original signal and extends with that.
+  ///
+  /// The [newWidth] must be greater than or equal to the current width or an exception
+  /// will be thrown.
+  Logic signExtend(int newWidth) {
+    if (newWidth < width) {
+      throw Exception(
+          'New width $newWidth must be greater than or equal to width $width.');
+    }
+    return [
+      Mux(
+        this[width - 1],
+        Const(1, width: newWidth - width, fill: true),
+        Const(0, width: newWidth - width),
+      ).y,
+      this,
+    ].swizzle();
+  }
+
+  /// Returns a copy of this [Logic] with the bits starting from [startIndex]
+  /// up until [startIndex] + [update]`.width` set to [update] instead
+  /// of their original value.
+  ///
+  /// The return signal will be the same [width].  An exception will be thrown if
+  /// the position of the [update] would cause an overrun past the [width].
+  Logic withSet(int startIndex, Logic update) {
+    if (startIndex + update.width > width) {
+      throw Exception(
+          'Width of updatedValue $update at startIndex $startIndex would'
+          'overrun the width of the original ($width).');
+    }
+
+    return [
+      getRange(startIndex + update.width, width),
+      update,
+      getRange(0, startIndex),
+    ].swizzle();
+  }
 }

lib/src/modules/bus.dart

@@ -75,6 +75,15 @@ class BusSubset extends Module with InlineSystemVerilog {
       throw Exception('BusSubset has exactly one input, but saw $inputs.');
     }
     var a = inputs[_original]!;
+
+    // SystemVerilog doesn't allow reverse-order select to reverse a bus, so do it manually
+    if (startIndex > endIndex) {
+      return '{' +
+          List.generate(startIndex - endIndex + 1, (i) => '$a[${endIndex + i}]')
+              .join(',') +
+          '}';
+    }
+
     var sliceString =
         startIndex == endIndex ? '[$startIndex]' : '[$endIndex:$startIndex]';
     return '$a$sliceString';
@@ -122,9 +131,8 @@ class Swizzle extends Module with InlineSystemVerilog {
 
   /// Executes the functional behavior of this gate.
   void _execute(int startIdx, Logic swizzleInput, LogicValueChanged? args) {
-    var updatedVal = out.value.toList();
-    updatedVal.setAll(startIdx, swizzleInput.value.toList());
-    out.put(LogicValue.of(updatedVal));
+    var updatedVal = out.value.withSet(startIdx, swizzleInput.value);
+    out.put(updatedVal);
   }
 
   @override
@@ -133,7 +141,10 @@ class Swizzle extends Module with InlineSystemVerilog {
       throw Exception('This swizzle has ${_swizzleInputs.length} inputs,'
           ' but saw $inputs with ${inputs.length} values.');
     }
-    var inputStr = _swizzleInputs.reversed.map((e) => inputs[e.name]).join(',');
+    var inputStr = _swizzleInputs.reversed
+        .where((e) => e.width > 0)
+        .map((e) => inputs[e.name])
+        .join(',');
     return '{$inputStr}';
   }
 }

lib/src/modules/conditional.dart

@@ -650,22 +650,20 @@ class CaseZ extends Case {
 /// A conditional block to execute only if [condition] is satisified.
 ///
 /// Intended for use with [IfBlock].
-class Iff {
+class ElseIf {
   /// A condition to match against to determine if [then] should be executed.
   final Logic condition;
 
   /// The [Conditional]s to execute if [condition] is satisfied.
   final List<Conditional> then;
 
-  Iff(this.condition, this.then);
+  ElseIf(this.condition, this.then);
 }
 
 /// A conditional block to execute only if [condition] is satisified.
 ///
 /// Intended for use with [IfBlock].
-class ElseIf extends Iff {
-  ElseIf(Logic condition, List<Conditional> then) : super(condition, then);
-}
+typedef Iff = ElseIf;
 
 /// A conditional block to execute only if [condition] is satisified.
 ///
@@ -681,8 +679,9 @@ class Else extends Iff {
 class IfBlock extends Conditional {
   /// A set of conditional items to check against for execution, in order.
   ///
-  /// The first item *must* be an [Iff], and if an [Else] is included it must
-  /// be the last item.  Any other items should be [ElseIf].
+  /// The first item should be an [Iff], and if an [Else] is included it must
+  /// be the last item.  Any other items should be [ElseIf].  It is okay to make the
+  /// first item an [ElseIf], it will act just like an [Iff].
   final List<Iff> iffs;
 
   IfBlock(this.iffs);
@@ -753,12 +752,9 @@ class IfBlock extends Conditional {
       }
       var header = iff == iffs.first
           ? 'if'
-          : iff is ElseIf
-              ? 'else if'
-              : iff is Else
-                  ? 'else'
-                  : throw Exception(
-                      'Unsupported Iff type: ${iff.runtimeType}.');
+          : iff is Else
+              ? 'else'
+              : 'else if';
 
       var conditionName = inputsNameMap[driverInput(iff.condition).name];
       var ifContents = iff.then

lib/src/values/logic_value.dart

@@ -282,20 +282,23 @@ abstract class LogicValue {
   /// Returns a new [LogicValue] with the order of all bits in the reverse order of this [LogicValue]
   LogicValue get reversed;
 
-  /// Returns a subset [LogicValue].  It is inclusive of [start], exclusive of [end].
+  /// Returns a subset [LogicValue].  It is inclusive of [startIndex], exclusive of [endIndex].
   ///
-  /// If [start] and [end] are equal, then a zero-width signal is returned.
-  LogicValue getRange(int start, int end) {
-    if (end < start) {
-      throw Exception('End ($end) cannot be greater than start ($start).');
+  /// [startIndex] must be less than [endIndex]. If [startIndex] and [endIndex] are equal, then a
+  /// zero-width value is returned.
+  LogicValue getRange(int startIndex, int endIndex) {
+    if (endIndex < startIndex) {
+      throw Exception(
+          'End ($endIndex) cannot be less than start ($startIndex).');
     }
-    if (end > width) {
-      throw Exception('End ($end) must be less than width ($width).');
+    if (endIndex > width) {
+      throw Exception('End ($endIndex) must be less than width ($width).');
     }
-    if (start < 0) {
-      throw Exception('Start ($start) must be greater than or equal to 0.');
+    if (startIndex < 0) {
+      throw Exception(
+          'Start ($startIndex) must be greater than or equal to 0.');
     }
-    return _getRange(start, end);
+    return _getRange(startIndex, endIndex);
   }
 
   /// Returns a subset [LogicValue].  It is inclusive of [start], exclusive of [end].
@@ -304,6 +307,18 @@ abstract class LogicValue {
   /// If [start] and [end] are equal, then a zero-width signal is returned.
   LogicValue _getRange(int start, int end);
 
+  /// Accesses a subset of this [LogicValue] from [startIndex] to [endIndex], both inclusive.
+  ///
+  /// If [endIndex] is less than [startIndex], the returned value will be reversed relative
+  /// to the original value.
+  LogicValue slice(int endIndex, int startIndex) {
+    if (startIndex <= endIndex) {
+      return getRange(startIndex, endIndex + 1);
+    } else {
+      return getRange(endIndex, startIndex + 1).reversed;
+    }
+  }
+
   /// Converts a pair of `_value` and `_invalid` into a [LogicValue].
   LogicValue _bitsToLogicValue(bool bitValue, bool bitInvalid) => bitInvalid
       ? (bitValue ? LogicValue.z : LogicValue.x)
@@ -595,6 +610,64 @@ abstract class LogicValue {
     }
     return previousValue == LogicValue.one && newValue == LogicValue.zero;
   }
+
+  /// Returns a new [LogicValue] with width [newWidth] where the most significant
+  /// bits for indices beyond the original [width] are set to [fill].
+  ///
+  /// The [newWidth] must be greater than or equal to the current width or an exception
+  /// will be thrown. [fill] must be a single bit ([width]=1).
+  LogicValue extend(int newWidth, LogicValue fill) {
+    if (newWidth < width) {
+      throw Exception(
+          'New width $newWidth must be greater than or equal to width $width.');
+    }
+    if (fill.width != 1) {
+      throw Exception('The fill must be 1 bit, but got $fill.');
+    }
+    return [
+      LogicValue.filled(newWidth - width, fill),
+      this,
+    ].swizzle();
+  }
+
+  /// Returns a new [LogicValue] with width [newWidth] where new bits added are zeros
+  /// as the most significant bits.
+  ///
+  /// The [newWidth] must be greater than or equal to the current width or an exception
+  /// will be thrown.
+  LogicValue zeroExtend(int newWidth) {
+    return extend(newWidth, LogicValue.zero);
+  }
+
+  /// Returns a new [LogicValue] with width [newWidth] where new bits added are sign bits
+  /// as the most significant bits.  The sign is determined using two's complement, so
+  /// it takes the most significant bit of the original value and extends with that.
+  ///
+  /// The [newWidth] must be greater than or equal to the current width or an exception
+  /// will be thrown.
+  LogicValue signExtend(int newWidth) {
+    return extend(newWidth, this[width - 1]);
+  }
+
+  /// Returns a copy of this [LogicValue] with the bits starting from [startIndex]
+  /// up until [startIndex] + [update]`.width` set to [update] instead
+  /// of their original value.
+  ///
+  /// The return value will be the same [width].  An exception will be thrown if
+  /// the position of the [update] would cause an overrun past the [width].
+  LogicValue withSet(int startIndex, LogicValue update) {
+    if (startIndex + update.width > width) {
+      throw Exception(
+          'Width of updatedValue $update at startIndex $startIndex would'
+          'overrun the width of the original ($width).');
+    }
+
+    return [
+      getRange(startIndex + update.width, width),
+      update,
+      getRange(0, startIndex),
+    ].swizzle();
+  }
 }
 
 /// Enum for direction of shift