Repository: intel/rohd (dart)

Issue:
Generated SystemVerilog includes index access on 1-bit signals
### Describe the bug

If a signal is defined as 1 bit in SystemVerilog:
```verilog
logic x;
```
Then you should not access the 0'th bit of it like this:
```verilog
x[0]
```
However, ROHD can generate this.

### To Reproduce

Access the 0'th bit of a `Logic` and generate SystemVerilog for it.

### Expected behavior

When a signal is 1 bit, treat a 0-index into it as a nop and generate accesses as just the raw signal.  It's probably even better to just short-circuit this and return the signal itself immediately to save the extra work of a nop module.

### Actual behavior

Generated verilog has `x[0]`-like behavior for 1-bit signals

### Additional: Dart SDK info

_No response_

### Additional: pubspec.yaml

_No response_

### Additional: Context

This bug likely applies to both `slice`/`BusSubset` via constant indexing and `IndexGate`.

Pull request:
<!-- Please make sure you check out the contribution guidelines before submitting a pull request! -->

## Description & Motivation
- Fixed: slice on a 1 bit signal should generate $x into SystemVerilog, instead of a $x[0], for single bit signals
- Fixed: test/config_test.dart: ROHD version in wavedumper test fails on Windows, since delete temporary directory is called before the simulator run returns.
- Fixed: IndexGate and BusSubset to generate $x into SystemVerilog, instead of $x[0] for single bit signals
- Updated: doc comments
<!-- Description of changes, and motivation for adding them. -->

## Related Issue(s)

<!-- If there are any issues related to this PR, please link to the issues here -->
Fixes: [204](https://github.com/intel/rohd/issues/204)
## Testing

<!-- Please describe how you tested your changes -->
- Added: additional tests on 1 bit signals using slice, IndexGate and BusSubset

## Backwards-compatibility

> Is this a breaking change that will not be backwards-compatible?  If yes, how so?
No.
<!--Answer here-->

## Documentation

> Does the change require any updates to documentation?  If so, where?  Are they included?

<!--Answer here-->
Yes.

Patch:
diff --git a/lib/src/logic.dart b/lib/src/logic.dart
index f5b004367..ea83852a6 100644
--- a/lib/src/logic.dart
+++ b/lib/src/logic.dart
@@ -614,7 +614,10 @@ class Logic {
   /// nextVal <= val[-9]; // Error!: allowed values [-8, 7]
   /// nextVal <= val[8]; // Error!: allowed values [-8, 7]
   /// ```
-  ///
+  /// Note: When, indexed by a Logic value, out-of-bounds will always return an
+  /// invalid (LogicValue.x) value. This behavior is differs in simulation as
+  /// compared to the generated SystemVerilog. In the generated SystemVerilog,
+  /// [index] will be ignored, and the logic is returned as-is.
   Logic operator [](dynamic index) {
     if (index is Logic) {
       return IndexGate(this, index).selection;
@@ -631,6 +634,7 @@ class Logic {
   /// value will be reversed relative to the original signal.
   /// Negative/Positive index values are allowed. (The negative indexing starts from where the array ends)
   ///
+  ///
   /// ```dart
   /// Logic nextVal = addOutput('nextVal', width: width);
   /// // Example: val = 0xce, val.width = 8, bin(0xce) = "0b11001110"
@@ -648,6 +652,12 @@ class Logic {
         (startIndex < 0) ? width + startIndex : startIndex;
     final modifiedEndIndex = (endIndex < 0) ? width + endIndex : endIndex;
 
+    if (width == 1 &&
+        modifiedEndIndex == 0 &&
+        modifiedEndIndex == modifiedStartIndex) {
+      return this;
+    }
+
     // Create a new bus subset
     return BusSubset(this, modifiedStartIndex, modifiedEndIndex).subset;
   }
diff --git a/lib/src/modules/bus.dart b/lib/src/modules/bus.dart
index 8daee20da..cfab75a8e 100644
--- a/lib/src/modules/bus.dart
+++ b/lib/src/modules/bus.dart
@@ -35,6 +35,8 @@ class BusSubset extends Module with InlineSystemVerilog {
 
   /// 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
+  /// in the generated SystemVerilog.
   BusSubset(Logic bus, this.startIndex, this.endIndex,
       {super.name = 'bussubset'}) {
     // If a converted index value is still -ve then it's an Index out of bounds
@@ -79,6 +81,11 @@ class BusSubset extends Module with InlineSystemVerilog {
 
   /// Executes the functional behavior of this gate.
   void _execute() {
+    if (original.width == 1) {
+      subset.put(original.value);
+      return;
+    }
+
     if (endIndex < startIndex) {
       subset.put(original.value.getRange(endIndex, startIndex + 1).reversed);
     } else {
@@ -93,6 +100,11 @@ class BusSubset extends Module with InlineSystemVerilog {
     }
     final a = inputs[_original]!;
 
+    // When, input width is 1, ignore startIndex and endIndex
+    if (original.width == 1) {
+      return a;
+    }
+
     // SystemVerilog doesn't allow reverse-order select to reverse a bus,
     // so do it manually
     if (startIndex > endIndex) {
diff --git a/lib/src/modules/gates.dart b/lib/src/modules/gates.dart
index e01154b43..52ff95f0a 100644
--- a/lib/src/modules/gates.dart
+++ b/lib/src/modules/gates.dart
@@ -676,9 +676,11 @@ class IndexGate extends Module with InlineSystemVerilog {
   /// Constructs a two-input bit index gate for an abitrary custom functional
   /// implementation.
   ///
-  /// The bit [index] will be indexed as an output.
+  /// The signal will be indexed by [index] as an output.
   /// [Module] is in-lined as SystemVerilog, it will use original[index], where
   /// target is index's int value
+  /// When, the [original] has width '1', [index] is ignored in the generated
+  /// SystemVerilog.
   IndexGate(Logic original, Logic index) : super() {
     _originalName = 'original_${original.name}';
     _indexName = Module.unpreferredName('index_${index.name}');
@@ -705,9 +707,10 @@ class IndexGate extends Module with InlineSystemVerilog {
 
   /// Executes the functional behavior of this gate.
   void _execute() {
-    if (_index.hasValidValue()) {
+    if (_index.hasValidValue() && _index.value.toInt() < _original.width) {
       final indexVal = _index.value.toInt();
-      selection.put(_original.value.getRange(indexVal, indexVal + 1));
+      final outputValue = _original.value.getRange(indexVal, indexVal + 1);
+      selection.put(outputValue);
     } else {
       selection.put(LogicValue.x);
     }
@@ -720,6 +723,11 @@ class IndexGate extends Module with InlineSystemVerilog {
     }
 
     final target = inputs[_originalName]!;
+
+    if (_original.width == 1) {
+      return target;
+    }
+
     final idx = inputs[_indexName]!;
     return '$target[$idx]';
   }


Test patch:
diff --git a/test/config_test.dart b/test/config_test.dart
index 7b103f326..fc63e4ccb 100644
--- a/test/config_test.dart
+++ b/test/config_test.dart
@@ -63,6 +63,7 @@ void main() async {
     final vcdContents = await File(temporaryDumpPath(dumpName)).readAsString();
     expect(vcdContents, contains(version));
 
+    await Simulator.run();
     deleteTemporaryDump(dumpName);
   });
 }
diff --git a/test/gate_test.dart b/test/gate_test.dart
index c26bc7789..976355829 100644
--- a/test/gate_test.dart
+++ b/test/gate_test.dart
@@ -430,7 +430,7 @@ void main() {
       expect(simResult, equals(true));
     });
 
-    test('Index Logic by Logic test', () async {
+    test('Index Logic(8bit) by Logic test', () async {
       final gtm = IndexGateTestModule(Logic(width: 8), Logic(width: 8));
       await gtm.build();
       final vectors = [
@@ -438,23 +438,81 @@ void main() {
         Vector({'original': 14, 'index': 2}, {'index_output': 1}),
         Vector({'original': 14, 'index': LogicValue.x},
             {'index_output': LogicValue.x}),
+        Vector({'original': 14, 'index': 9}, {'index_output': LogicValue.x})
+      ];
+      await SimCompare.checkFunctionalVector(gtm, vectors);
+      final simResult = SimCompare.iverilogVector(
+          gtm.generateSynth(), gtm.runtimeType.toString(), vectors,
+          signalToWidthMap: {'original': 8, 'index': 8});
+      expect(simResult, equals(true));
+    });
+
+    test('Index Logic(1bit) by Logic test', () async {
+      final gtm = IndexGateTestModule(Logic(), Logic(width: 8));
+      await gtm.build();
+      final vectors = [
         Vector({'original': LogicValue.x, 'index': LogicValue.x},
             {'index_output': LogicValue.x}),
         Vector({'original': LogicValue.x, 'index': 0},
-            {'index_output': LogicValue.x})
+            {'index_output': LogicValue.x}),
+        Vector({'original': LogicValue.one, 'index': 0},
+            {'index_output': LogicValue.one}),
+        Vector({'original': LogicValue.zero, 'index': 0},
+            {'index_output': LogicValue.zero})
       ];
       await SimCompare.checkFunctionalVector(gtm, vectors);
       final simResult = SimCompare.iverilogVector(
           gtm.generateSynth(), gtm.runtimeType.toString(), vectors,
-          signalToWidthMap: {'original': 8, 'index': 8});
+          signalToWidthMap: {'original': 1, 'index': 8, 'index_output': 1});
       expect(simResult, equals(true));
     });
 
     test('Index Logic by an Integer test', () {
       final testLogic = Logic(width: 8)..put(14);
+      final testLogicOne = Logic()..put(LogicValue.one);
+      final testLogicZero = Logic()..put(LogicValue.zero);
+      final testLogicInvalid = Logic()..put(LogicValue.x);
+
       expect(testLogic[0].value.toInt(), 0);
       expect(testLogic[2].value.toInt(), 1);
+
+      expect(testLogicOne[0].value.toInt(), 1);
+      expect(testLogicZero[0].value.toInt(), 0);
+      expect(testLogicInvalid[0].value, LogicValue.x);
+
+      expect(testLogicOne[-1].value.toInt(), 1);
+      expect(testLogicZero[-1].value.toInt(), 0);
+      expect(testLogicInvalid[-1].value, LogicValue.x);
+
       expect(() => testLogic[10], throwsException);
+      expect(() => testLogicOne[1], throwsException);
+      expect(() => testLogicZero[1], throwsException);
+      expect(() => testLogicInvalid[1], throwsException);
+    });
+
+    test('index Logic(1bit) by Logic index out of bounds test', () {
+      final testLogicOne = Logic()..put(LogicValue.one);
+      final testLogicZero = Logic()..put(LogicValue.zero);
+      final invalidIndex = Logic(width: 8)..put(1);
+
+      expect(testLogicOne[invalidIndex].value, equals(LogicValue.x));
+      expect(testLogicZero[invalidIndex].value, equals(LogicValue.x));
+    });
+
+    test('slice 1 bit wide Logic test', () {
+      final testLogic = Logic(width: 8)..put(14);
+      final testLogicOne = Logic()..put(LogicValue.one);
+      final testLogicZero = Logic()..put(LogicValue.zero);
+      final testLogicInvalid = Logic()..put(LogicValue.x);
+
+      expect(testLogicOne.slice(0, 0), equals(testLogicOne));
+      expect(testLogicZero.slice(0, 0), equals(testLogicZero));
+      expect(testLogicInvalid.slice(0, 0), equals(testLogicInvalid));
+
+      expect(() => testLogic.slice(0, 10), throwsException);
+      expect(() => testLogicOne.slice(0, 1), throwsException);
+      expect(() => testLogicZero.slice(0, 1), throwsException);
+      expect(() => testLogicInvalid.slice(0, 1), throwsException);
     });
 
     test('Index Logic by does not accept input other than int or Logic', () {