Skip to content

Commit

Permalink
Inherit from Record and OpaqueType rather than Bundle
Browse files Browse the repository at this point in the history
  • Loading branch information
konda-x1 authored and milovanovic committed Jul 17, 2023
1 parent 1c411db commit ece2788
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 24 deletions.
20 changes: 10 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,29 +37,29 @@ This outputs the following SystemVerilog code:
module Example(
input clock,
input reset,
input [7:0] in_data,
output [7:0] out1_data,
output [10:0] out2_data
input [7:0] in,
output [7:0] out1,
output [10:0] out2
);
wire [8:0] _out1_T = {$signed(in_data), 1'h0}; // @[FixedPoint.scala 295:34]
assign out1_data = _out1_T[7:0]; // @[ExampleApp.scala 9:8]
assign out2_data = 11'sh324; // @[FixedPoint.scala 295:34]
wire [8:0] _out1_T = {$signed(in), 1'h0}; // @[FixedPoint.scala 301:34]
assign out1 = _out1_T[7:0]; // @[Example.scala 9:8]
assign out2 = 11'sh324; // @[FixedPoint.scala 301:34]
endmodule
```

## How It Works

FixedPoint is implemented as an extension of `Bundle`, which has one field named `data` of type `SInt`. Most of the arithmetic involving FixedPoints has been delegated to the `SInt` arithmetic of the `data` field, where shift operations are first used to align the data of FixedPoints that have different binary points. Connect methods have also been overridden to account for data alignment of FixedPoints with different binary points, and to implement binary point inference.
FixedPoint is implemented as an extension of `Record`, which has one *anonymous* data field of type `SInt`; it is also an [opaque type](https://github.com/chipsalliance/chisel/blob/v3.5.6/core/src/main/scala/chisel3/experimental/OpaqueType.scala). Most of the arithmetic involving FixedPoints has been delegated to the `SInt` arithmetic of the underlying data field, where shift operations are first used to align the data of FixedPoints that have different binary points. Connect methods have also been overridden to account for data alignment of FixedPoints with different binary points, and to implement binary point inference.

## Limitations

It was challenging to implement FixedPoint using Chisel's public API as some of the needed functionality for FixedPoints was originally implemented in Chisel's package-private objects, which cannot be accessed or altered from a user-level library. Due to this issue, some of the original FixedPoint functionality could not be implemented without limited workarounds. Here is the current list of limitations of this implementation of FixedPoint:
* FixedPoints with different binary points are not aligned properly when used inside Chisel's Muxes (Mux, Mux1H, PriorityMux, MuxLookup, MuxCase). To that end, these objects have been redefined in the package `fixedpoint.shadow` to align FixedPoints by width and binary point before calling Chisel's corresponding Mux objects. In order to make FixedPoint work properly with Muxes, you have to import the new Mux definitions as follows:
* FixedPoints with different binary points are not aligned properly when used inside Chisel's Muxes (`Mux`, `Mux1H`, `PriorityMux`, `MuxLookup`, `MuxCase`). To that end, these objects have been redefined in the package `fixedpoint.shadow` to align FixedPoints by width and binary point before calling Chisel's corresponding Mux objects. In order to make FixedPoint work properly with Muxes, you have to import the new Mux definitions as follows:
```scala
import fixedpoint.shadow.{Mux, Mux1H, PriorityMux, MuxLookup, MuxCase}
```
* Bundles with inferred widths cannot be used inside Mux1H. If you want to use FixedPoints inside a Mux1H, make sure that both width and binary point are specified in advance.
* FixedPoints do not connect properly if they are nested inside a Bundle. If you have a bundle that has a FixedPoint field, you will have to extend it with the `ForceElementwiseConnect` trait. If you have a bundle `Foo` defined as:
* Records with inferred widths cannot be used inside `Mux1H`. If you want to use FixedPoints inside a `Mux1H`, make sure that both width and binary point are specified in advance.
* FixedPoints do not connect properly if they are nested inside a `Bundle` or `Record`. If you have a bundle/record that has a FixedPoint field, you will have to extend it with the `ForceElementwiseConnect` trait. If you have a bundle `Foo` defined as:
```scala
class Foo extends Bundle {
val data = FixedPoint(8.W, 4.BP)
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/fixedpoint/BinaryPoint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

package fixedpoint

import chisel3.{Bundle, ChiselException, Num}
import chisel3.{ChiselException, Num, Record}

/** Chisel types that have binary points support retrieving
* literal values as `Double` or `BigDecimal`
Expand All @@ -35,7 +35,7 @@ object NumBP {
}

trait HasBinaryPoint {
self: Bundle =>
self: Record =>

def binaryPoint: BinaryPoint

Expand Down
27 changes: 16 additions & 11 deletions src/main/scala/fixedpoint/FixedPoint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@

package fixedpoint

import chisel3.{fromIntToBinaryPoint => _, fromDoubleToLiteral => _, _}
import chisel3.{fromDoubleToLiteral => _, fromIntToBinaryPoint => _, _}
import chisel3.experimental.BundleLiterals.AddBundleLiteralConstructor
import chisel3.experimental.{AutoCloneType, OpaqueType}
import chisel3.internal.firrtl.Width
import chisel3.internal.sourceinfo.SourceInfo
import chisel3.stage.ChiselStage
import fixedpoint.shadow.{Mux, Mux1H, MuxCase, MuxLookup, PriorityMux}

import scala.collection.immutable.SeqMap

object FixedPoint extends NumObject {

/** Create a FixedPoint type with inferred width. */
Expand Down Expand Up @@ -156,24 +159,27 @@ object FixedPoint extends NumObject {
}

sealed class FixedPoint private[fixedpoint] (width: Width, private var _inferredBinaryPoint: BinaryPoint)
extends Bundle
extends Record
with AutoCloneType
with OpaqueType
with Num[FixedPoint]
with HasBinaryPoint {
val data: SInt = SInt(width)
def binaryPoint: BinaryPoint = _inferredBinaryPoint
private val data: SInt = SInt(width)
val elements: SeqMap[String, SInt] = SeqMap("" -> data)
def binaryPoint: BinaryPoint = _inferredBinaryPoint

private def requireKnownBP(message: => Any = "Unknown binary point is not supported in this operation"): Unit = {
require(_inferredBinaryPoint.isInstanceOf[KnownBinaryPoint], message)
}

private def additiveOp(that: FixedPoint, f: (SInt, SInt) => SInt, width: Width = Width()): FixedPoint = {
val Seq(dis, dat) = FixedPoint.dataAligned(this, that)
FixedPoint.fromData(width, _inferredBinaryPoint.max(that._inferredBinaryPoint), f(dis.data, dat.data))
val Seq(_this, _that) = FixedPoint.dataAligned(this, that).map(WireDefault(_))
FixedPoint.fromData(width, _inferredBinaryPoint.max(that._inferredBinaryPoint), f(_this.data, _that.data))
}

private def comparativeOp(that: FixedPoint, f: (SInt, SInt) => Bool): Bool = {
val Seq(dis, dat) = FixedPoint.dataAligned(this, that)
WireDefault(f(dis.data, dat.data))
val Seq(_this, _that) = FixedPoint.dataAligned(this, that).map(WireDefault(_))
f(_this.data, _that.data)
}

private def connectOp(
Expand Down Expand Up @@ -315,13 +321,12 @@ sealed class FixedPoint private[fixedpoint] (width: Width, private var _inferred
litToDoubleOption match {
case Some(value) => s"FixedPoint$width$binaryPoint($value)"
case _ =>
// Can't use stringAccessor so will have to extract from data field's toString()...
val suffix = ".*?([(].*[)])".r.findFirstMatchIn(data.toString()) match {
// Can't use stringAccessor so will have to extract from data field's toString...
val suffix = ".*?([(].*[)])".r.findFirstMatchIn(data.toString) match {
case Some(m) => m.group(1)
case None => ""
}
s"FixedPoint$width$binaryPoint$suffix"
}
}

}
2 changes: 1 addition & 1 deletion src/test/scala/FixedPointSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class FixedPointSpec extends ChiselPropSpec with Utils {
}
property("Negative shift amounts are invalid") {
a[ChiselException] should be thrownBy extractCause[ChiselException] {
ChiselStage.elaborate(new NegativeShift(FixedPoint(1.W, 0.BP).data))
ChiselStage.elaborate(new NegativeShift(FixedPoint(1.W, 0.BP).asSInt))
}
}
property("Bit extraction on literals should work for all non-negative indices") {
Expand Down

0 comments on commit ece2788

Please sign in to comment.