The output of the module is `X` when it shouldn't be

[chykon]

Describe the bug

The output of the module is X when it shouldn't be.

To Reproduce

The failure occurs when testing DLatch outputs.

Example on DartPad: https://dartpad.dev/?id=c84f7d3b50b1418a8f7741eb47ba63ff

File for local check:

test.dart

import 'package:rohd/rohd.dart';
import 'package:test/test.dart';

/// A two-input NAND gate.
class Nand2Gate extends Module {
  /// Calculates the NAND of `in0` and `in1`.
  Nand2Gate(Logic in0, Logic in1) {
    in0 = addInput(in0.name, in0);
    in1 = addInput(in1.name, in1);

    _out.gets(NotGate(And2Gate(in0, in1).out).out);
  }

  /// The output of this gate.
  Logic get out => output(_out.name);

  late final _out = addOutput(Logic().name);
}

/// Represents a single D-Latch with no reset.
class DLatch extends Module {
  /// Constructs a D-Latch.
  DLatch(Logic en, Logic d) {
    en = addInput(en.name, en);
    d = addInput(d.name, d);

    final nand2gate0 = Nand2Gate(en, d);
    final nand2gate1 = Nand2Gate(en, nand2gate0.out);
    final nand2gate2 = Nand2Gate(nand2gate0.out, _outB);
    final nand2gate3 = Nand2Gate(nand2gate1.out, _out);

    Combinational([
      ConditionalAssign(_out, nand2gate2.out),
      ConditionalAssign(_outB, nand2gate3.out)
    ]);
  }

  /// The direct output of the latch.
  Logic get out => output(_out.name);

  /// The inverse output of the latch.
  Logic get outB => output(_outB.name);

  late final _out = addOutput('out');
  late final _outB = addOutput('outB');
}

void main() async {
  final en = Logic();
  final d = Logic();

  final dLatch = DLatch(en, d);
  await dLatch.build();

  group('DLatch', () {
    test('0. en = 0, d = 0 : out = X', () {
      en.put(0);
      d.put(0);
      expect(dLatch.out.value, equals(LogicValue.x));
      expect(dLatch.outB.value, equals(LogicValue.x));
    });
    test('1. en = 0, d = 1 : out = X', () {
      en.put(0);
      d.put(1);
      expect(dLatch.out.value, equals(LogicValue.x));
      expect(dLatch.outB.value, equals(LogicValue.x));
    });
    test('2. en = 1, d = 1 : out = 1', () {
      en.put(1);
      d.put(1);
      expect(dLatch.out.value, equals(LogicValue.one));
      expect(dLatch.outB.value, equals(LogicValue.zero));
    });
    test('3. en = 0, d = 1 : out = 1', () {
      en.put(0);
      d.put(1);
      expect(dLatch.out.value, equals(LogicValue.one));
      expect(dLatch.outB.value, equals(LogicValue.zero));
    });
    test('4. en = 0, d = 0 : out = 1', () {
      en.put(0);
      d.put(0);
      expect(dLatch.out.value, equals(LogicValue.one));
      expect(dLatch.outB.value, equals(LogicValue.zero));
    });
    test('5. en = 1, d = 0 : out = 0', () {
      en.put(1);
      d.put(0);
      expect(dLatch.out.value, equals(LogicValue.zero));
      expect(dLatch.outB.value, equals(LogicValue.one));
    });
    test('6. en = 1, d = 1 : out = 1', () {
      en.put(1);
      d.put(1);
      expect(dLatch.out.value, equals(LogicValue.one));
      expect(dLatch.outB.value, equals(LogicValue.zero));
    });
    test('7. en = 1, d = 0 : out = 0', () {
      en.put(1);
      d.put(0);
      expect(dLatch.out.value, equals(LogicValue.zero));
      expect(dLatch.outB.value, equals(LogicValue.one));
    });
    test('8. en = 1, d = 1 : out = 1', () {
      en.put(1);
      d.put(1);
      expect(dLatch.out.value, equals(LogicValue.one));
      expect(dLatch.outB.value, equals(LogicValue.zero));
    });
    test('9. en = 0, d = 0 : out = 1', () {
      en.put(0);
      d.put(0);
      expect(dLatch.out.value, equals(LogicValue.one));
      expect(dLatch.outB.value, equals(LogicValue.zero));
    });
    test('10. en = 1, d = 1 : out = 1', () {
      en.put(1);
      d.put(1);
      expect(dLatch.out.value, equals(LogicValue.one));
      expect(dLatch.outB.value, equals(LogicValue.zero));
    });
  });
}

Expected behavior

X does not appear and everything works like this: https://circuitverse.org/users/165714/projects/d-latch-18f3ff83-3cbc-447a-8f40-8f0d3491848e

Actual behavior

An X value occurs, causing the module to work incorrectly.

Additional: Dart SDK info

Dart SDK version: 2.19.1 (stable) (Tue Jan 31 12:25:35 2023 +0000) on "linux_x64"

Additional: pubspec.yaml

ROHD v0.4.2

Additional: Context

Also checked with Codespaces on the main branch of ROHD.

[chykon]

I'm not sure if Combinational should even allow a latch to be generated, since it matches the behavior of always_latch, not always_comb. In such a case, it may be necessary to throw an exception when a latch is encountered.

[mkorbel1]

This circuit (being a latch) has a combinational loop in it. Specifically, it looks like you're building this (a bit easier to see than the CircuitVerse link).

The way that this circuit saves state for a latch is exactly because of that loop. A Combinational or any other combinational circuitry in ROHD are specifically intended not to represent state-saving circuits like latches and flops. An always_comb block in SystemVerilog may simulate the way you expected, but a synthesis tool will (should) warn you that a latch has been inferred even though you specifically asked for a purely combinational circuit via always_comb. ROHD intentionally generates an x here instead of silently inferring a latch to help developers identify bugs earlier.

The ROHD simulator detects combinational loops via reentrance checks in the propagation logic (here is an example, if you're curious). I think it's not that easy to statically identify a combinational loop or inferred latch without doing a lot of the work that a synthesis tool does, so this check is a simulation-time check in ROHD. I think it's also probably not that easy to tell the difference between a "latch" and some other unintentional combinational loop, except for maybe in Combinational.

To summarize, I think the x generation you're observing is probably the intended behavior unless I'm misunderstanding your bug.

ROHD doesn't currently have an equivalent to always_latch. There are some less safe things you can do with latches that are harder to do with flops. Not having latches in ROHD helps encourage designers to stay in safer territory. However, it would be possible to add latches to ROHD if there was reason and desire for them.

This other issue (https://github.com/intel/rohd/issues/230) tracks adding of some additional logging to reveal the source of x generation during ROHD simulation. This probably would be helpful in this case and just reinforces that the enhancement is worth implementing.

Please let me know if this answers your questions or if there's more to this that I didn't fully appreciate. And thank you for filing!

[chykon]

Thanks for the detailed answer! I just wanted to try creating a latch in ROHD and didn't know if what I got was the expected behavior or a bug. It seems logical that Combinational does not allow them to be created.

I would also like to clarify the differences between gets (<=) and Combinational (linked from https://github.com/intel/rohd/issues/286). It turns out that gets completely detects loops and emits x on all outputs, and Combinational sometimes allows the correct value to be output on one of the outputs. Shouldn't they work the same way?

[mkorbel1]

This bug will be closed by #344 which now makes a combinational circuit representing a latch always either drive X or a fatal exception due to "write after read", depending on the details of the implementation.