From 2ef187eb6eea4c168a3aa62974ed75c3e639f5d4 Mon Sep 17 00:00:00 2001 From: ganewto <75503794+ganewto@users.noreply.github.com> Date: Mon, 18 Sep 2023 08:18:22 -0700 Subject: [PATCH] Fixes for LogicValue operation bugs related to size, sign, math, and comparison (#319) Co-authored-by: Kirkpatrick, Desmond A --- lib/src/values/big_logic_value.dart | 7 +- lib/src/values/logic_value.dart | 35 +++--- lib/src/values/small_logic_value.dart | 2 +- test/logic_value_width_test.dart | 166 ++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 19 deletions(-) create mode 100644 test/logic_value_width_test.dart diff --git a/lib/src/values/big_logic_value.dart b/lib/src/values/big_logic_value.dart index 24258369e..f76aab2f9 100644 --- a/lib/src/values/big_logic_value.dart +++ b/lib/src/values/big_logic_value.dart @@ -45,7 +45,8 @@ class _BigLogicValue extends LogicValue { static final Map _masksOfWidth = {}; static BigInt _maskOfWidth(int width) { if (!_masksOfWidth.containsKey(width)) { - _masksOfWidth[width] = (BigInt.one << width) - BigInt.one; + _masksOfWidth[width] = + ((BigInt.one << width) - BigInt.one).toUnsigned(width); } return _masksOfWidth[width]!; } @@ -60,8 +61,8 @@ class _BigLogicValue extends LogicValue { : assert(width > LogicValue._INT_BITS, '_BigLogicValue should only be used for large values'), super._(width) { - _value = _mask & value; - _invalid = _mask & invalid; + _value = (_mask & value).toUnsigned(width); + _invalid = (_mask & invalid).toUnsigned(width); assert( allowInefficientRepresentation || diff --git a/lib/src/values/logic_value.dart b/lib/src/values/logic_value.dart index 5c17c7eb7..c845feeec 100644 --- a/lib/src/values/logic_value.dart +++ b/lib/src/values/logic_value.dart @@ -18,6 +18,8 @@ typedef LogicValues = LogicValue; /// /// Each bit of [LogicValue] can be represented as a [LogicValue] /// of `0`, `1`, `x` (contention), or `z` (floating). +/// +/// [LogicValue] is unsigned. @immutable abstract class LogicValue implements Comparable { /// The number of bits in an int. @@ -841,25 +843,21 @@ abstract class LogicValue implements Comparable { /// Addition operation. /// - /// WARNING: Signed math is not fully tested. // ignore: avoid_dynamic_calls LogicValue operator +(dynamic other) => _doMath(other, (a, b) => a + b); /// Subtraction operation. /// - /// WARNING: Signed math is not fully tested. // ignore: avoid_dynamic_calls LogicValue operator -(dynamic other) => _doMath(other, (a, b) => a - b); /// Multiplication operation. /// - /// WARNING: Signed math is not fully tested. // ignore: avoid_dynamic_calls LogicValue operator *(dynamic other) => _doMath(other, (a, b) => a * b); /// Division operation. /// - /// WARNING: Signed math is not fully tested. // ignore: avoid_dynamic_calls LogicValue operator /(dynamic other) => _doMath(other, (a, b) => a ~/ b); @@ -883,7 +881,7 @@ abstract class LogicValue implements Comparable { } if (this is BigInt || this is _BigLogicValue || - (this is _FilledLogicValue && width >= _INT_BITS)) { + (this is _FilledLogicValue && width > _INT_BITS)) { final a = toBigInt(); return LogicValue.ofBigInt(op(a, width) as BigInt, width); } else { @@ -927,12 +925,12 @@ abstract class LogicValue implements Comparable { return LogicValue.filled(other.width, LogicValue.x); } - if (this is _BigLogicValue || other is BigInt || other is _BigLogicValue) { + if (width > _INT_BITS || (other is LogicValue && other.width > _INT_BITS)) { final a = toBigInt(); final b = other is BigInt ? other : other is int - ? BigInt.from(other) + ? BigInt.from(other).toUnsigned(_INT_BITS) : other is LogicValue ? other.toBigInt() : throw Exception( @@ -970,28 +968,24 @@ abstract class LogicValue implements Comparable { /// Less-than operation. /// - /// WARNING: Signed math is not fully tested. LogicValue operator <(dynamic other) => // ignore: avoid_dynamic_calls _doCompare(other, (a, b) => (a < b) as bool); /// Greater-than operation. /// - /// WARNING: Signed math is not fully tested. LogicValue operator >(dynamic other) => // ignore: avoid_dynamic_calls _doCompare(other, (a, b) => (a > b) as bool); /// Less-than-or-equal operation. /// - /// WARNING: Signed math is not fully tested. LogicValue operator <=(dynamic other) => // ignore: avoid_dynamic_calls _doCompare(other, (a, b) => (a <= b) as bool); /// Greater-than-or-equal operation. /// - /// WARNING: Signed math is not fully tested. LogicValue operator >=(dynamic other) => // ignore: avoid_dynamic_calls _doCompare(other, (a, b) => (a >= b) as bool); @@ -1042,19 +1036,30 @@ abstract class LogicValue implements Comparable { dynamic a; dynamic b; - if (this is _BigLogicValue || other is BigInt || other is _BigLogicValue) { + if (width > _INT_BITS || (other is LogicValue && other.width > _INT_BITS)) { a = toBigInt(); b = other is BigInt ? other : other is int - ? BigInt.from(other) + ? BigInt.from(other).toUnsigned(_INT_BITS) : other is LogicValue ? other.toBigInt() : throw Exception( 'Unexpected big type: ${other.runtimeType}.'); } else { - a = toInt(); - b = other is int ? other : (other as LogicValue).toInt(); + if (width < _INT_BITS) { + a = toInt(); + b = other is int ? other : (other as LogicValue).toInt(); + } else { + // Here we now know: width == _INT_BITS + final ai = toInt(); + final bi = other is int ? other : (other as LogicValue).toInt(); + if ((ai < 0) || (bi < 0)) { + final abig = LogicValue.ofBigInt(BigInt.from(ai), _INT_BITS + 1); + final bbig = LogicValue.ofBigInt(BigInt.from(bi), _INT_BITS + 1); + return abig._doCompare(bbig, op); + } + } } return op(a, b) ? LogicValue.one : LogicValue.zero; } diff --git a/lib/src/values/small_logic_value.dart b/lib/src/values/small_logic_value.dart index c2939e88d..aaaba70dc 100644 --- a/lib/src/values/small_logic_value.dart +++ b/lib/src/values/small_logic_value.dart @@ -102,7 +102,7 @@ class _SmallLogicValue extends LogicValue { bool get isFloating => (_invalid == _mask) && (_value == _mask); @override - BigInt toBigInt() => BigInt.from(toInt()); + BigInt toBigInt() => BigInt.from(toInt()).toUnsigned(width); @override int toInt() { diff --git a/test/logic_value_width_test.dart b/test/logic_value_width_test.dart new file mode 100644 index 000000000..efa3123e1 --- /dev/null +++ b/test/logic_value_width_test.dart @@ -0,0 +1,166 @@ +// Copyright (C) 2021-2023 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// logic_value_width_test.dart +// Unit tests for width issues (64-bit boundary) in [LogicValue]. +// +// 2023 September 17 +// Author: Desmond Kirkpatrick + +import 'package:rohd/rohd.dart'; +import 'package:test/test.dart'; + +void main() { + test('crash compare', () { + final input = Const(BigInt.from(2).pow(128), width: 129); + final output = Logic(); + Combinational([ + If.block([ + Iff(input.getRange(0, 128) > BigInt.from(0), + [output < Const(1, width: 1)]), + Else([output < Const(0, width: 1)]), + ]) + ]); + }); + + test('bad compare', () { + const i = 64; + final input = Const(BigInt.from(1) << (i - 1), width: i); + final output = Logic(); + Combinational([ + If.block([ + Iff(input > BigInt.from(0), [output < Const(1, width: 1)]), + Else([output < Const(0, width: 1)]), + ]) + ]); + final b = ~input.eq(0); + expect(output.value, equals(b.value)); + }); + + test('big value test', () { + expect( + LogicValue.ofBigInt(BigInt.zero, 128) + + LogicValue.ofBigInt(BigInt.zero, 128), + LogicValue.ofInt(0, 128)); + }); + + group('values test', () { + for (final len in [63, 64, 65, 66, 67]) { + final sslv = LogicValue.ofInt(4, len); // small Int hold Big + final bslv = LogicValue.ofInt(-0xFFFF, len); // 18446744073709486081 + final fslv = LogicValue.ofInt(-2, len); // 18446744073709551614 + + final sblv = LogicValue.ofBigInt(BigInt.from(4), len); + final bblv = LogicValue.ofBigInt(BigInt.from(-0xFFFF), len); + final fblv = LogicValue.ofBigInt(BigInt.from(-2), len); + + test('small Int storage len=$len', () { + expect(sslv < bslv, LogicValue.one); + expect(bslv < sslv, LogicValue.zero); + expect(sslv > bslv, LogicValue.zero); + expect(bslv > sslv, LogicValue.one); + + expect(sslv < fslv, LogicValue.one); + expect(fslv < sslv, LogicValue.zero); + expect(sslv > fslv, LogicValue.zero); + expect(fslv > sslv, LogicValue.one); + + expect(bslv < fslv, LogicValue.one); + expect(fslv < bslv, LogicValue.zero); + expect(bslv > fslv, LogicValue.zero); + expect(fslv > bslv, LogicValue.one); + }); + + test('big Int storage len=$len', () { + expect(sblv < bblv, LogicValue.one); + expect(bblv < sblv, LogicValue.zero); + expect(sblv > bblv, LogicValue.zero); + expect(bblv > sblv, LogicValue.one); + + expect(sblv < fblv, LogicValue.one); + expect(fblv < sblv, LogicValue.zero); + expect(sblv > fblv, LogicValue.zero); + expect(fblv > sblv, LogicValue.one); + + expect(bblv < fblv, LogicValue.one); + expect(fblv < bblv, LogicValue.zero); + expect(bblv > fblv, LogicValue.zero); + expect(fblv > bblv, LogicValue.one); + }); + + test('big math len=$len', () { + expect(sslv + fslv, LogicValue.ofInt(2, len)); + expect(sslv - fslv, LogicValue.ofInt(6, len)); + expect(fslv - sslv, LogicValue.ofInt(-6, len)); + + expect(sslv * fslv, LogicValue.ofInt(-8, len)); + + expect(sslv + fslv, LogicValue.ofBigInt(BigInt.from(2), len)); + expect(sslv - fslv, LogicValue.ofBigInt(BigInt.from(6), len)); + expect(fslv - sslv, LogicValue.ofBigInt(BigInt.from(-6), len)); + + expect(fslv * sslv, LogicValue.ofBigInt(BigInt.from(-8), len)); + + expect(sblv + fblv, LogicValue.ofInt(2, len)); + expect(sblv - fblv, LogicValue.ofInt(6, len)); + expect(fblv - sblv, LogicValue.ofInt(-6, len)); + + expect(sblv * fblv, LogicValue.ofInt(-8, len)); + + expect(sblv + fblv, LogicValue.ofBigInt(BigInt.from(2), len)); + expect(sblv - fblv, LogicValue.ofBigInt(BigInt.from(6), len)); + expect(fblv - sblv, LogicValue.ofBigInt(BigInt.from(-6), len)); + + expect(fblv * sblv, LogicValue.ofBigInt(BigInt.from(-8), len)); + }); + + test('division test len=$len', () { + final negsfour = LogicValue.ofInt(-4, len); + final negbfour = LogicValue.ofBigInt(BigInt.from(-4), len); + final two = LogicValue.ofBigInt(BigInt.from(2), len); + expect(negsfour / two, LogicValue.ofInt(-4, len) >>> 1); + expect(negbfour / two, LogicValue.ofBigInt(BigInt.from(-4), len) >>> 1); + }); + + test('modulo test len=$len', () { + final negsfive = LogicValue.ofInt(-5, len); + final negbfive = LogicValue.ofBigInt(BigInt.from(-5), len); + final two = LogicValue.ofBigInt(BigInt.from(2), len); + expect(negsfive % two, LogicValue.ofInt(1, len)); + expect(negbfive % two, LogicValue.ofBigInt(BigInt.from(1), len)); + }); + + test('clog test len=$len', () { + final negnum = LogicValue.ofInt(-1, len); + expect(negnum.clog2(), LogicValue.ofInt(len, len)); + for (final l in [1, 2, 3]) { + expect((negnum >>> l).clog2(), LogicValue.ofInt(len - l, len)); + } + for (final l in [len - 5, len - 4, len - 3, len - 2]) { + final bignum = LogicValue.ofBigInt(BigInt.from(1) << l, len); + expect(bignum.clog2(), LogicValue.ofInt(l, len)); + if (len < 64) { + final smallnum = LogicValue.ofInt(1 << l, len); + expect(smallnum.clog2(), LogicValue.ofInt(l, len)); + } + } + for (final l in [len - 5, len - 4, len - 3]) { + final bignum = LogicValue.ofBigInt(BigInt.from(2) << l, len); + expect(bignum.clog2().toBigInt(), BigInt.from(l + 1)); + if (len < 64) { + final smallnum = LogicValue.ofInt(2 << l, len); + expect(smallnum.clog2(), LogicValue.ofInt(l + 1, len)); + } + } + for (final l in [len - 5, len - 4, len - 3]) { + final bignum = LogicValue.ofBigInt(BigInt.from(3) << l, len); + expect(bignum.clog2(), LogicValue.ofInt(l + 2, len)); + if (len < 64) { + final smallnum = LogicValue.ofInt(3 << l, len); + expect(smallnum.clog2(), LogicValue.ofInt(l + 2, len)); + } + } + }); + } + }); +}