Skip to content

Commit

Permalink
Fixing bugs with largest operand division. (#129)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kimmeljo
kimmeljo authored Nov 15, 2024
1 parent f59dfee commit 3530022
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 32 deletions.
2 changes: 2 additions & 0 deletions doc/components/divider.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ For the division, implicit rounding towards 0 is always performed. I.e., a negat

For the remainder, the following equation will always precisely hold true: `dividend = divisor * quotient + remainder`. Note that this differs from the Euclidean modulo operator where the sign of the remainder is always positive.

Overflow can only occur when `dividend=<max negative number>`, `divisor=-1` and `isSigned=1`. In this case, the hardware will return `quotient=<max negative number>` and `remainder=0`. This is by design as the mathematically correct quotient cannot be represented in the fixed number of bits available.

## Code Example

```dart
Expand Down
59 changes: 32 additions & 27 deletions lib/src/arithmetic/divider.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ class MultiCycleDivider extends Module {
void _build() {
// to capture current inputs
// as this operation takes multiple cycles
final aBuf = Logic(name: 'aBuf', width: dataWidth);
final rBuf = Logic(name: 'rBuf', width: dataWidth);
final bBuf = Logic(name: 'bBuf', width: dataWidth);
final aBuf = Logic(name: 'aBuf', width: dataWidth + 1);
final rBuf = Logic(name: 'rBuf', width: dataWidth + 1);
final bBuf = Logic(name: 'bBuf', width: dataWidth + 1);
final signOut = Logic(name: 'signOut');
final signNum = Logic(name: 'signNum');

Expand All @@ -183,27 +183,30 @@ class MultiCycleDivider extends Module {

// internal buffers for computation
// accumulator that contains dividend
final outBuffer = Logic(name: 'outBuffer', width: dataWidth);
final outBuffer = Logic(name: 'outBuffer', width: dataWidth + 1);
// capture last successful power of 2
final lastSuccess = Logic(name: 'lastSuccess', width: dataWidth);
final lastSuccess = Logic(name: 'lastSuccess', width: dataWidth + 1);
// combinational logic signal to compute current (a-b*2^i)
final tmpDifference = Logic(name: 'tmpDifference', width: dataWidth);
final tmpDifference = Logic(name: 'tmpDifference', width: dataWidth + 1);
// store last diff
final lastDifference = Logic(name: 'lastDifference', width: dataWidth);
final lastDifference = Logic(name: 'lastDifference', width: dataWidth + 1);
// combinational logic signal to check for overflow when shifting
final tmpShift = Logic(name: 'tmpShift', width: dataWidth);
final tmpShift = Logic(name: 'tmpShift', width: dataWidth + 1);

// current value of i to try
// need log(dataWidth) bits
final currIndex = Logic(name: 'currIndex', width: logDataWidth);

intf.quotient <= outBuffer; // result is ultimately stored in out_buffer
intf.quotient <=
outBuffer.getRange(
0, dataWidth); // result is ultimately stored in out_buffer
intf.divZero <= ~bBuf.or(); // divide-by-0 if b==0 (NOR)
intf.remainder <= rBuf; // synonymous with the remainder
intf.remainder <=
rBuf.getRange(0, dataWidth); // synonymous with the remainder

// ready/busy signals are based on internal state
intf.validOut <= currentState.eq(_MultiCycleDividerState.done);
intf.readyIn <= ~currentState.eq(_MultiCycleDividerState.ready);
intf.readyIn <= currentState.eq(_MultiCycleDividerState.ready);

// update current_state with next_state once per cycle
Sequential(intf.clk, [
Expand Down Expand Up @@ -249,13 +252,7 @@ class MultiCycleDivider extends Module {
~bBuf.getRange(0, dataWidth - 2).or() &
(signOut ^ signNum),
then: [
// special logic for when a is also most negative #
tmpDifference <
mux(
aBuf[dataWidth - 1] &
~aBuf.getRange(0, dataWidth - 2).or(),
~Const(0, width: dataWidth), // -1
Const(0, width: dataWidth)),
tmpDifference < ~Const(0, width: dataWidth + 1), // -1
nextState < _MultiCycleDividerState.accumulate
],
orElse: [
Expand Down Expand Up @@ -284,6 +281,12 @@ class MultiCycleDivider extends Module {

// capture input arguments a, b into internal buffers
// so the consumer doesn't have to continually assert them
final extDividendIn = Logic(name: 'extDividendIn', width: dataWidth + 1)
..gets(mux(intf.isSigned, intf.dividend.signExtend(dataWidth + 1),
intf.dividend.zeroExtend(dataWidth + 1)));
final extDivisorIn = Logic(name: 'extDivisorIn', width: dataWidth + 1)
..gets(mux(intf.isSigned, intf.divisor.signExtend(dataWidth + 1),
intf.divisor.zeroExtend(dataWidth + 1)));
Sequential(intf.clk, [
If.block([
// only when READY and new inputs are available
Expand All @@ -297,11 +300,11 @@ class MultiCycleDivider extends Module {
// conditionally convert negative inputs to positive
// and compute the output sign
aBuf <
mux(intf.dividend[dataWidth - 1] & intf.isSigned,
~intf.dividend + 1, intf.dividend),
mux(extDividendIn[dataWidth - 1] & intf.isSigned,
~extDividendIn + 1, extDividendIn),
bBuf <
mux(intf.divisor[dataWidth - 1] & intf.isSigned,
~intf.divisor + 1, intf.divisor),
mux(extDivisorIn[dataWidth - 1] & intf.isSigned,
~extDivisorIn + 1, extDivisorIn),
signOut <
(intf.dividend[dataWidth - 1] ^ intf.divisor[dataWidth - 1]) &
intf.isSigned,
Expand All @@ -328,7 +331,7 @@ class MultiCycleDivider extends Module {
final aBufConv = mux(signNum, ~aBuf + 1, aBuf);
Sequential(intf.clk, [
If.block([
Iff(intf.reset, [rBuf < Const(0, width: dataWidth)]),
Iff(intf.reset, [rBuf < Const(0, width: dataWidth + 1)]),
ElseIf(
currentState.eq(_MultiCycleDividerState
.convert), // adjust positive remainder for signs
Expand Down Expand Up @@ -365,16 +368,17 @@ class MultiCycleDivider extends Module {
ElseIf(currentState.eq(_MultiCycleDividerState.ready) & intf.validIn, [
lastSuccess < 0,
lastDifference <
mux(intf.dividend[dataWidth - 1] & intf.isSigned,
~intf.dividend + 1, intf.dividend), // start by matching aBuf
mux(extDividendIn[dataWidth - 1] & intf.isSigned,
~extDividendIn + 1, extDividendIn), // start by matching aBuf
]),
ElseIf(
currentState.eq(_MultiCycleDividerState
.process), // didn't exceed a_buf, so count as success
[
If(~tmpDifference[dataWidth - 1], then: [
lastSuccess <
(Const(1, width: dataWidth) << currIndex), // capture 2^i
(Const(1, width: dataWidth + 1) <<
currIndex), // capture 2^i
lastDifference < tmpDifference
], orElse: [
// failure so maintain
Expand All @@ -397,7 +401,8 @@ class MultiCycleDivider extends Module {
If.block([
Iff(intf.reset, [outBuffer < 0]), // reset buffer
ElseIf(currentState.eq(_MultiCycleDividerState.done), [
outBuffer < mux(intf.readyOut, Const(0, width: dataWidth), outBuffer),
outBuffer <
mux(intf.readyOut, Const(0, width: dataWidth + 1), outBuffer),
]), // reset buffer if consumed result
ElseIf(currentState.eq(_MultiCycleDividerState.convert), [
outBuffer < mux(signOut, ~outBuffer + 1, outBuffer),
Expand Down
100 changes: 95 additions & 5 deletions test/arithmetic/divider_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class MultiCycleDividerDriver extends Driver<MultiCycleDividerInputSeqItem> {
// Every clock negative edge, drive the next pending item if it exists
// but only when the DUT isn't busy
intf.clk.negedge.listen((args) {
if (_pendingItems.isNotEmpty && intf.readyIn.value == LogicValue.zero) {
if (_pendingItems.isNotEmpty && intf.readyIn.value == LogicValue.one) {
final nextItem = _pendingItems.removeFirst();
drive(nextItem);
if (_pendingItems.isEmpty) {
Expand Down Expand Up @@ -167,7 +167,7 @@ class MultiCycleDividerInputMonitor
// Every positive edge of the clock
intf.clk.posedge.listen((event) {
if (intf.validIn.value == LogicValue.one &&
intf.readyIn.value == LogicValue.zero) {
intf.readyIn.value == LogicValue.one) {
add(MultiCycleDividerInputSeqItem(
mDividend: intf.isSigned.value.toBool()
? _twosComp(intf.dividend.value.toInt(), intf.dividend.width)
Expand Down Expand Up @@ -266,11 +266,22 @@ class MultiCycleDividerScoreboard extends Component {
inSign ? _twosComp(currResult, intf.quotient.width) : currResult;
final tCurrRemain =
inSign ? _twosComp(currRemain, intf.remainder.width) : currRemain;
final overflow = inSign &&
in1 ==
_twosComp(
1 << (intf.quotient.width - 1), intf.quotient.width) &&
in2 == -1;
if (triggerCheck) {
final check1 = (in2 == 0) ? divZero : ((in1 ~/ in2) == tCurrResult);
final check2 = (in2 == 0)
var check1 = (in2 == 0) ? divZero : ((in1 ~/ in2) == tCurrResult);
var check2 = (in2 == 0)
? divZero
: ((in1 - (in2 * tCurrResult)) == tCurrRemain);
if (overflow) {
check1 = tCurrResult ==
_twosComp(1 << (intf.quotient.width - 1), intf.quotient.width);
check2 = tCurrRemain == 0;
}

if (check1 && check2) {
final msg = (in2 == 0)
? '''
Expand Down Expand Up @@ -399,6 +410,84 @@ class MultiCycleDividerBasicSequence extends Sequence {
}
}

class MultiCycleDividerEvilSequence extends Sequence {
late final int numBits;

MultiCycleDividerEvilSequence(
{required this.numBits, String name = 'MultiCycleDividerEvilSequence'})
: super(name);

@override
Future<void> body(Sequencer sequencer) async {
sequencer as MultiCycleDividerSequencer
// largest positive divided by largest positive
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)) - 1,
mDivisor: (1 << (numBits - 1)) - 1,
mValidIn: true,
mIsSigned: true,
mReadyOut: true))
// largest positive divided by largest negative
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)) - 1,
mDivisor: (1 << (numBits - 1)),
mValidIn: true,
mIsSigned: true,
mReadyOut: true))
// largest negative divided by largest positive
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)),
mDivisor: (1 << (numBits - 1)) - 1,
mValidIn: true,
mIsSigned: false,
mReadyOut: true))
// largest negative divided by largest negative
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)),
mDivisor: (1 << (numBits - 1)),
mValidIn: true,
mIsSigned: false,
mReadyOut: true))
// largest positive divided by negative 1
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)) - 1,
mDivisor: -1,
mValidIn: true,
mIsSigned: true,
mReadyOut: true))
// largest negative divided by negative 1
// this is the only true overflow case...
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)),
mDivisor: -1,
mValidIn: true,
mIsSigned: true,
mReadyOut: true))
// largest positive divided by 1
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)) - 1,
mDivisor: 1,
mValidIn: true,
mIsSigned: true,
mReadyOut: true))
// largest negative divided by 1
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)),
mDivisor: 1,
mValidIn: true,
mIsSigned: true,
mReadyOut: true))
// unsigned version of overflow case
// which should not result in overflow
..add(MultiCycleDividerInputSeqItem(
mDividend: (1 << (numBits - 1)),
mDivisor: -1,
mValidIn: true,
mIsSigned: false,
mReadyOut: true));
}
}

class MultiCycleDividerVolumeSequence extends Sequence {
final int numReps;
final rng = Random(0xdeadbeef); // fixed seed
Expand Down Expand Up @@ -479,6 +568,7 @@ class MultiCycleDividerTest extends Test {

// Kick off a sequence on the sequencer
await _divSequencer.start(MultiCycleDividerBasicSequence());
await _divSequencer.start(MultiCycleDividerEvilSequence(numBits: 32));
await _divSequencer.start(MultiCycleDividerVolumeSequence(1000));

logger.info('Done adding stimulus to the sequencer');
Expand Down Expand Up @@ -512,7 +602,7 @@ void main() {
group('divider tests', () {
test('VF tests', () async {
// Set the logger level
Logger.root.level = Level.OFF;
Logger.root.level = Level.SEVERE;

// Create the testbench
final intf = MultiCycleDividerInterface();
Expand Down

0 comments on commit 3530022

Please sign in to comment.