Logic operation fails on LogicValue wider than 127

[ganewto]

https://github.com/intel/rohd/blob/344db0e18fa702eda056ed1ddbcda7717e154c17/lib/src/values/logic_value.dart#L783

The conversion to Int rather than BigInt means that logic operations can fail when LogicValues are wide. The limit appears to be 128.

[mkorbel1]

I think the issue is not that we should have toBigInt() here, but rather that the check directly above it is insufficient for all possible scenarios where it is not convertible to an int. For example, it looks like if we have _FilledLogicValue of width > 64 we may still hit toInt() in the else clause.

The check should probably check the width of this and other if they are LogicValues instead of the type.

[ganewto]

Here I have a testcase that replicates the problem:

  test('crash_compare', () {
    final input = Const(BigInt.from(2).pow(128), width: 129);
    final output = Logic();
    Combinational([
      IfBlock([
        Iff(input.getRange(0, 128) > BigInt.from(0),
            [output < Const(1, width: 1)]),
        Else([output < Const(0, width: 1)]),
      ])
    ]);
  });```

[ganewto]

I posted my first pull request. Let me know if there are better ways to commit these changes as well as if the changes I am suggesting are sufficient.

There is still an outstanding issue on sign-extension when converting to BigInt which causes a problem.

On Fri, Mar 3, 2023 at 3:48 PM Max Korbel @.***> wrote:

I think the issue is not that we should have toBigInt() here, but rather that the check directly above it is insufficient for all possible scenarios where it is not convertible to an int. For example, it looks like if we have _FilledLogicValue of width > 64 we may still hit toInt() in the else clause.

The check should probably check the width of this and other if they are LogicValues instead of the type.

— Reply to this email directly, view it on GitHub https://github.com/intel/rohd/issues/299#issuecomment-1454272972, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASABRMTTPD3JO6KYMC5B2Y3W2J7MTANCNFSM6AAAAAAVPFBIR4 . You are receiving this because you authored the thread.Message ID: @.***>

[mkorbel1]

@Sanchit-kumar found a simple test case that reproduces this bug in #336:

LogicValue.ofBigInt(BigInt.zero, 128) + LogicValue.ofBigInt(BigInt.zero, 128);