`gets` is not equivalent to `Combinational`

[chykon]

Describe the bug

Related to https://github.com/intel/rohd/issues/285

If you replace Combinational with gets in DLatch, then the module will return only the X value.

To Reproduce

Modified example from https://github.com/intel/rohd/issues/285

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);

    _out.gets(nand2gate2.out);
    _outB.gets(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

The behavior of gets is equivalent to Combinational.

Actual behavior

The behavior of gets is not equivalent to Combinational.

Additional: Dart SDK info

No response

Additional: pubspec.yaml

No response

Additional: Context

No response

[mkorbel1]

My initial feeling is that you're right that this is a bug. I would have expected Combinational and gets to operate pretty much identically here. I'm able to reproduce your observations, thank you for filing and for the detailed reproduction instructions!

[mkorbel1]

One difference between a Combinational and gets is that the Combinational will execute procedurally, so actually it is possible for them to meaningfully be different.

As an experiment, I tried moving them into independent Combinationals:

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

Unexpectedly with these changes, your test now passes! There's definitely something wrong going on here.

[chykon]

Indeed, it works. It's not a bug, it's a feature :D

In that case, please close the https://github.com/intel/rohd/issues/285 issue when you need it. I just don't know if it is related to this problem or not.

[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.