Make conditional assign a little more optimistic with invalid values (#…

…459)

CHANGELOG.md

@@ -1,6 +1,6 @@
 ## 0.5.2
 
-- Added APIs for accessing indices of a `List<Logic>` using another `Logic`: `Logic.selectFrom` and `List<Logic>.selectIndex`.
+- Added APIs for accessing indices of a `List<Logic>` using another `Logic`: `Logic.selectFrom` and `List<Logic>.selectIndex` (<https://github.com/intel/rohd/pull/438>).
 - Added/fixed support for compiling ROHD to JavaScript via bug fixes, compile-time arithmetic precision consideration, and testing (<https://github.com/intel/rohd/pull/445>).
 - Added `isZero` to `LogicValue`.
 - Improved `Pipeline` abstraction via bug fixes, better error checking, improved documentation, and new APIs (<https://github.com/intel/rohd/pull/447>).

lib/src/module.dart

@@ -201,13 +201,13 @@ abstract class Module {
     return hierarchyQueue;
   }
 
-  /// Indicates whether this [Module] has had the [build()] method called on it.
+  /// Indicates whether this [Module] has had the [build] method called on it.
   bool get hasBuilt => _hasBuilt;
   bool _hasBuilt = false;
 
   /// Builds the [Module] and all [subModules] within it.
   ///
-  /// It is recommended not to override [build()] nor put logic in [build()]
+  /// It is recommended not to override [build] nor put logic in [build]
   /// unless you have good reason to do so.  Aim to build up relevant logic in
   /// the constructor.
   ///

lib/src/modules/conditional.dart

@@ -909,7 +909,9 @@ class ConditionalAssign extends Conditional {
 
     final currentValue = driverValue(driver);
     if (!currentValue.isValid) {
-      _receiverOutput.put(LogicValue.x);
+      // Use bitwise & to turn Z's into X's, but keep valid signals as-is.
+      // It's too pessimistic to convert the whole bus to X.
+      _receiverOutput.put(currentValue & currentValue);
     } else {
       _receiverOutput.put(currentValue);
     }

lib/src/synthesizers/synth_builder.dart

@@ -9,6 +9,7 @@
 
 import 'package:collection/collection.dart';
 import 'package:rohd/rohd.dart';
+import 'package:rohd/src/utilities/sanitizer.dart';
 import 'package:rohd/src/utilities/uniquifier.dart';
 
 /// A generic class which can convert a module into a generated output using
@@ -88,6 +89,9 @@ class SynthBuilder {
           initialName: newName, reserved: module.reserveDefinitionName);
     }
 
+    assert(Sanitizer.isSanitary(newName),
+        'Module definition names should be sanitary.');
+
     _moduleToInstanceTypeMap[module] = newName;
     return newName;
   }

lib/src/synthesizers/systemverilog.dart

@@ -11,6 +11,7 @@ import 'package:collection/collection.dart';
 import 'package:meta/meta.dart';
 import 'package:rohd/rohd.dart';
 import 'package:rohd/src/collections/traverseable_collection.dart';
+import 'package:rohd/src/utilities/sanitizer.dart';
 import 'package:rohd/src/utilities/uniquifier.dart';
 
 /// A [Synthesizer] which generates equivalent SystemVerilog as the
@@ -762,7 +763,15 @@ class _SynthLogicArrayElement extends _SynthLogic {
   bool get needsDeclaration => false;
 
   @override
-  String get name => '${parentArray.name}[${logic.arrayIndex!}]';
+  String get name {
+    final n = '${parentArray.name}[${logic.arrayIndex!}]';
+    assert(
+      Sanitizer.isSanitary(
+          n.substring(0, n.contains('[') ? n.indexOf('[') : null)),
+      'Array name should be sanitary, but found $n',
+    );
+    return n;
+  }
 
   /// The element of the [parentArray].
   final Logic logic;
@@ -840,7 +849,13 @@ class _SynthLogic {
   /// The chosen name of this.
   ///
   /// Must call [pickName] before this is accessible.
-  String get name => _name!;
+  String get name {
+    assert(isConstant || Sanitizer.isSanitary(_name!),
+        'Signal names should be sanitary, but found $_name.');
+
+    return _name!;
+  }
+
   String? _name;
 
   /// Picks a [name].

test/conditionals_test.dart

@@ -167,8 +167,8 @@ class ConditionalAssignModule extends Module {
   ConditionalAssignModule(
     Logic a,
   ) : super(name: 'ConditionalAssignModule') {
-    a = addInput('a', a);
-    final c = addOutput('c');
+    a = addInput('a', a, width: a.width);
+    final c = addOutput('c', width: a.width);
     Combinational([c < a]);
   }
 }
@@ -614,6 +614,22 @@ void main() {
         Vector({'a': LogicValue.x}, {'c': LogicValue.x}),
       ];
       await SimCompare.checkFunctionalVector(mod, vectors);
+      // no SV run here, ROHD converts Z to X
+    });
+
+    test('Conditional assign module with invalid inputs wider than 1 bit',
+        () async {
+      final mod = ConditionalAssignModule(Logic(width: 8));
+      await mod.build();
+      final vectors = [
+        Vector({'a': 0xa5}, {'c': 0xa5}),
+        Vector({'a': LogicValue.z}, {'c': LogicValue.x}),
+        Vector({'a': LogicValue.x}, {'c': LogicValue.x}),
+        Vector({'a': LogicValue.ofString('01zzxx10')},
+            {'c': LogicValue.ofString('01xxxx10')}),
+      ];
+      await SimCompare.checkFunctionalVector(mod, vectors);
+      // no SV run here, ROHD converts Z to X
     });
 
     test('single elseifblock comb', () async {

test/sv_gen_test.dart

@@ -93,6 +93,29 @@ class ModuleWithFloatingSignals extends Module {
   }
 }
 
+class TopCustomSvWrap extends Module {
+  TopCustomSvWrap(Logic a, Logic b) {
+    a = addInput('a', a);
+    b = addInput('b', b);
+
+    SubCustomSv([a, b]);
+  }
+}
+
+class SubCustomSv extends Module with CustomSystemVerilog {
+  SubCustomSv(List<Logic> toSwizzle) {
+    addInput('fer_swizzle', toSwizzle.swizzle(), width: toSwizzle.length);
+  }
+
+  @override
+  String instantiationVerilog(String instanceType, String instanceName,
+          Map<String, String> inputs, Map<String, String> outputs) =>
+      '''
+logic my_fancy_new_signal; // $instanceName (of type $instanceType)
+assign my_fancy_new_signal <= ^${inputs['fer_swizzle']};
+''';
+}
+
 void main() {
   group('signal declaration order', () {
     void checkSignalDeclarationOrder(String sv, List<String> signalNames) {
@@ -173,4 +196,11 @@ void main() {
     expect('assign'.allMatches(sv).length, 1);
     expect('assign xylophone'.allMatches(sv).length, 1);
   });
+
+  test('properly drops in custom systemveri;og', () async {
+    final mod = TopCustomSvWrap(Logic(), Logic());
+    await mod.build();
+    final sv = mod.generateSynth();
+    expect(sv, contains('assign my_fancy_new_signal <= ^({a,b});'));
+  });
 }